Perf: single-pass remap_instance_id (unique + bincount + LUT gather)#9009
Perf: single-pass remap_instance_id (unique + bincount + LUT gather)#9009aymuos15 wants to merge 3 commits into
Conversation
Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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. Comment |
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/metrics/test_remap_instance_id.py (2)
78-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a descriptive generator name.
Rename
gtogenerator.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/metrics/test_remap_instance_id.py` around lines 78 - 79, Rename the local generator variable g to generator in the test setup, and update its use in the torch.randint call accordingly.Source: Path instructions
36-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd required Google-style docstrings.
Document
_reference_remap,TestRemapInstanceId, and each test method, withArgs,Returns, andRaiseswhere applicable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/metrics/test_remap_instance_id.py` around lines 36 - 84, Add Google-style docstrings to `_reference_remap`, `TestRemapInstanceId`, and every test method in that class. Document parameters with `Args`, return values with `Returns` where applicable, and expected exceptions with `Raises` where applicable; keep the existing test behavior unchanged.Source: Path instructions
monai/metrics/utils.py (1)
422-433: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the return contract.
Add a Google-style
Returns:section, including thetorch.intoutput for remapped inputs and passthrough dtype for empty/all-background inputs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@monai/metrics/utils.py` around lines 422 - 433, Update the documentation for the function containing the torch.unique remapping logic with a Google-style Returns: section. Specify that remapped inputs return a torch.int tensor, while empty or all-background inputs are returned unchanged with their original dtype.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/metrics/test_remap_instance_id.py`:
- Line 43: Update the zip call used to build pair_list so it passes strict=True,
ensuring pred_id and instance_size have matching lengths while preserving the
existing sorting and reverse-order behavior.
---
Nitpick comments:
In `@monai/metrics/utils.py`:
- Around line 422-433: Update the documentation for the function containing the
torch.unique remapping logic with a Google-style Returns: section. Specify that
remapped inputs return a torch.int tensor, while empty or all-background inputs
are returned unchanged with their original dtype.
In `@tests/metrics/test_remap_instance_id.py`:
- Around line 78-79: Rename the local generator variable g to generator in the
test setup, and update its use in the torch.randint call accordingly.
- Around line 36-84: Add Google-style docstrings to `_reference_remap`,
`TestRemapInstanceId`, and every test method in that class. Document parameters
with `Args`, return values with `Returns` where applicable, and expected
exceptions with `Raises` where applicable; keep the existing test behavior
unchanged.
🪄 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 Plus
Run ID: 7e01ca0f-c313-4f7e-bce5-f5ff3e022504
📒 Files selected for processing (2)
monai/metrics/utils.pytests/metrics/test_remap_instance_id.py
Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
1 participant
Description
Replaces the per-instance loops in
remap_instance_id(which scanned the full volume once per instance id — twice withby_size=True— forO(instances * volume)work) with a singletorch.unique(return_inverse=True)+bincount+ LUT-gather pass, computing the same relabeling inO(volume). Same approach as #8978;by_sizetie-breaking (stable, ascending-id) and the int32 output dtype are preserved exactly.Benchmark
Synthetic instance masks (random sparse ids,
by_size=True), median of repeated runs,OMP_NUM_THREADS=8.2D
3D
Numerics are bit-identical (
torch.equal) at every size. The old implementation was slowest on GPU (e.g. 2720 ms for 1024×1024 with 500 instances, vs 262 ms on CPU) because the per-instance masked scans serialize; that inversion is gone (49.8 ms).System: 12th Gen Intel Core i7-12800H (20 threads,
OMP_NUM_THREADS=8); NVIDIA RTX A1000 Laptop GPU (4 GB); Linux 6.8.0-124-generic x86_64; Python 3.10.12; PyTorch 2.12.1+cu130.Types of changes
black/isort/ruff/mypy.