7951 anchors are not centered on grid cells#8475
Conversation
Signed-off-by: hachoj <hjchojnowski@gmail.com>
Signed-off-by: hachoj <hjchojnowski@gmail.com>
Signed-off-by: hachoj <hjchojnowski@gmail.com>
|
Hi @hachoj thanks for the contribution, it looks fine to me as far as I can tell. Could you please check that this works with the luna16 tutorial example? If so I think we're good. @Can-Zhao I think you were the original author of this file, any thoughts? |
|
Hi @ericspod I ran a smoke test of the LUNA16 detection tutorial pipeline with this patch applied (MONAI commit 8f6e2c7, PyTorch What was tested:
The full pipeline (data loading → ATSS matching → training → sliding window inference → box post-processing) runs Note: since real LUNA16 data was not used, detection quality metrics are not meaningful here. The test only |
📝 WalkthroughWalkthroughThis PR addresses anchor center placement by offsetting anchor centers by half the stride (integer division) in Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
monai/apps/detection/utils/anchor_utils.py (1)
220-235: Document the new centered-anchor behavior ingrid_anchorsdocstring.This method’s behavior changed (breaking), but the docstring still describes spacing only. Please add explicit center-offset semantics and the raised exceptions.
As per coding guidelines, "Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@monai/apps/detection/utils/anchor_utils.py` around lines 220 - 235, The docstring for grid_anchors needs to be updated to document the new centered-anchor behavior and the exceptions it can raise: update the description of grid_anchors (and mention interaction with self.cell_anchors and the grid_sizes/strides inputs) to state that anchors are generated centered in each grid cell with an explicit center-offset of stride/2 in each spatial dimension, describe the shape and meaning of the returned list[Tensor] (each tensor contains N x D x ... anchor boxes in image coordinates or the same dims as cell_anchors), list and describe all arguments (grid_sizes, strides) and the meaning of their element types, and add a Raises section that documents any ValueError or TypeError thrown by grid_anchors (e.g., mismatched lengths between grid_sizes, strides, invalid tensor dtypes or non-positive strides); reference the grid_anchors method name and self.cell_anchors in the docstring so implementers can locate the relevant behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@monai/apps/detection/utils/anchor_utils.py`:
- Line 256: The center calculation uses integer division and floors odd strides:
replace the expression torch.arange(0, size[axis], dtype=torch.int32,
device=device) * stride[axis] + stride[axis] // 2 with a floating-point
computation so centers are true geometric midpoints — e.g. create the arange
with a floating dtype (torch.float32) on the same device and use stride[axis] /
2 (floating division) when adding the half-stride; ensure variables stride,
size, axis and device are used but the arithmetic is done in float so the
produced centers are not truncated.
---
Nitpick comments:
In `@monai/apps/detection/utils/anchor_utils.py`:
- Around line 220-235: The docstring for grid_anchors needs to be updated to
document the new centered-anchor behavior and the exceptions it can raise:
update the description of grid_anchors (and mention interaction with
self.cell_anchors and the grid_sizes/strides inputs) to state that anchors are
generated centered in each grid cell with an explicit center-offset of stride/2
in each spatial dimension, describe the shape and meaning of the returned
list[Tensor] (each tensor contains N x D x ... anchor boxes in image coordinates
or the same dims as cell_anchors), list and describe all arguments (grid_sizes,
strides) and the meaning of their element types, and add a Raises section that
documents any ValueError or TypeError thrown by grid_anchors (e.g., mismatched
lengths between grid_sizes, strides, invalid tensor dtypes or non-positive
strides); reference the grid_anchors method name and self.cell_anchors in the
docstring so implementers can locate the relevant behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a9b4e36-ec5c-4a4c-889d-0f53e2c0efab
📒 Files selected for processing (2)
monai/apps/detection/utils/anchor_utils.pytests/apps/detection/utils/test_anchor_box.py
|
Hi @Cado87 thanks for the help here. I think the change is fine with a sanity check like what you've done so we're good to merge. We can return to this later of course, the Coderabbit comment didn't need necessary to me but it might be worth looking at later if anyone has issues with this change. |
This pull request resolves issue #7951 by correctly centering anchor boxes within their grid cells.
Description
Changes Made:
monai/apps/detection/utils/anchor_utils.pyto add astride // 2offset.tests/apps/detection/utils/test_anchor_box.pyto validate this new, correct behavior against the torchvision baseline by accounting for the offset.Fixes #7951
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.