Skip to content

7951 anchors are not centered on grid cells#8475

Merged
ericspod merged 5 commits intoProject-MONAI:devfrom
hachoj:7951-anchors-are-not-centered-on-grid-cells
Apr 29, 2026
Merged

7951 anchors are not centered on grid cells#8475
ericspod merged 5 commits intoProject-MONAI:devfrom
hachoj:7951-anchors-are-not-centered-on-grid-cells

Conversation

@hachoj
Copy link
Copy Markdown
Contributor

@hachoj hachoj commented Jun 9, 2025

This pull request resolves issue #7951 by correctly centering anchor boxes within their grid cells.

Description

Changes Made:

  • Modified the anchor generation logic in monai/apps/detection/utils/anchor_utils.py to add a stride // 2 offset.
  • Refactored the corresponding unit test in tests/apps/detection/utils/test_anchor_box.py to validate this new, correct behavior against the torchvision baseline by accounting for the offset.

Fixes #7951

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.
  • [ ]

hachoj added 3 commits June 8, 2025 17:49
Signed-off-by: hachoj <hjchojnowski@gmail.com>
Signed-off-by: hachoj <hjchojnowski@gmail.com>
Signed-off-by: hachoj <hjchojnowski@gmail.com>
@hachoj hachoj marked this pull request as ready for review June 9, 2025 23:26
@ericspod
Copy link
Copy Markdown
Member

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?

@Cado87
Copy link
Copy Markdown
Contributor

Cado87 commented Apr 23, 2026

Hi @ericspod

I ran a smoke test of the LUNA16 detection tutorial pipeline with this patch applied (MONAI commit 8f6e2c7, PyTorch
2.10.0, Tesla T4 GPU).

What was tested:

  • Verified the anchor fix is active: with a 2×2×2 grid and stride=4, the first anchor center is now at (2.0, 2.0,
    2.0) instead of (0.0, 0.0, 0.0)
  • Ran luna16_training.py (fold 0, 3 epochs, synthetic volumes matching the fold-0 datasplit paths) — training
    completed without errors, loss was finite and decreasing
  • Ran luna16_testing.py on the saved checkpoint — inference processed 67 images, produced 94 predicted boxes with
    scores in a valid range

The full pipeline (data loading → ATSS matching → training → sliding window inference → box post-processing) runs
without issues with the centered anchors.

Note: since real LUNA16 data was not used, detection quality metrics are not meaningful here. The test only
validates that the fix does not break the tutorial pipeline. With real data, a full training run would be needed to
confirm the expected improvement in detection accuracy from properly centered anchors.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

📝 Walkthrough

Walkthrough

This PR addresses anchor center placement by offsetting anchor centers by half the stride (integer division) in AnchorGenerator.grid_anchors, placing anchors at grid cell centers rather than grid multiples. Test assertions are modified to validate grid_anchors outputs against torchvision anchors adjusted by the same half-stride offset, removing full forward-pass comparison logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly and concisely describes the main change: centering anchors on grid cells.
Description check ✅ Passed Description covers the issue fixed, changes made, and test validation. Marks breaking change and confirms testing completion.
Linked Issues check ✅ Passed Changes directly address #7951 requirement: stride//2 offset added to anchor generation, matching the suggested fix in the issue.
Out of Scope Changes check ✅ Passed All changes are scoped to anchor centering: modified anchor generation logic and updated corresponding unit tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
monai/apps/detection/utils/anchor_utils.py (1)

220-235: Document the new centered-anchor behavior in grid_anchors docstring.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3bb9d8e and 5333449.

📒 Files selected for processing (2)
  • monai/apps/detection/utils/anchor_utils.py
  • tests/apps/detection/utils/test_anchor_box.py

Comment thread monai/apps/detection/utils/anchor_utils.py
@ericspod
Copy link
Copy Markdown
Member

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.

@ericspod ericspod merged commit 71f16ed into Project-MONAI:dev Apr 29, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Anchors are not centered on grid cells

3 participants