-
Notifications
You must be signed in to change notification settings - Fork 1.4k
docs: clarify backward mapping logic for RandAffine and RandAffined #8730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdded explanatory Note blocks about MONAI's backward-mapping (image-to-grid) semantics for translation, scaling, and rotation to the RandAffine docstrings in monai/transforms/spatial/array.py and monai/transforms/spatial/dictionary.py. No public signatures or functional behavior were changed. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In `@monai/transforms/spatial/dictionary.py`:
- Around line 1103-1106: Update the Note in the docstring that references
RandAffine to include rotation semantics: state that, like translation and
scaling, rotation uses backward-mapping so positive rotation values rotate the
sampled image in the opposite direction (i.e., the image is rotated negatively
relative to the transform parameter) and add that the apparent direction (CW vs
CCW) may vary depending on the axis, mirroring RandAffine's wording; modify the
Note near the existing RandAffine reference in
monai/transforms/spatial/dictionary.py to include this rotation sentence.
🧹 Nitpick comments (1)
monai/transforms/spatial/array.py (1)
2439-2445: Good documentation addition addressing backward mapping confusion.The Note effectively explains the counterintuitive behavior reported in issue
#8726. The translation and scaling explanations are accurate for backward mapping semantics.Optional: Consider clarifying the scaling explanation.
Line 2443 states "Values > 0 decrease the image size" but doesn't reference that these are
scale_rangeinput values (before the 1.0 offset mentioned at line 2408 is applied). The current wording is correct but could be clearer for users.📝 Optional clarification for scale explanation
- - Scaling: Values > 0 decrease the image size; values in [-1, 0) increase it. + - Scaling: scale_range values > 0 (resulting in scale factors > 1.0 after the 1.0 offset) decrease the image size; values in [-1, 0) increase it.Or more concisely:
- - Scaling: Values > 0 decrease the image size; values in [-1, 0) increase it. + - Scaling: Positive scale_range values decrease the image size; values in [-1, 0) increase it.
Signed-off-by: ytl0623 <david89062388@gmail.com>
b7c3f2d to
f9f311c
Compare
Fixes #8726
Description
monai/transforms/spatial/array.py: Added a Note toRandAffineexplaining the inverse relationship for translation and scaling.monai/transforms/spatial/dictionary.py: Added a corresponding Note toRandAffinedto ensure consistency for dictionary-based workflows.Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.