Fix Invertd confusion with postprocessing transforms #8526
Conversation
|
Warning Rate limit exceeded@eclipse0922 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/transforms/post/dictionary.py(1 hunks)tests/transforms/inverse/test_invertd.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit Configuration File
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. 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. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/post/dictionary.pytests/transforms/inverse/test_invertd.py
🔇 Additional comments (1)
tests/transforms/inverse/test_invertd.py (1)
140-182: Excellent test coverage for the fix.Test directly validates the issue #8396 scenario - Lambdad in postprocessing before Invertd. Proper setup, assertions, and deterministic seeding.
eclipse0922
force-pushed
the
fix-invertd-postproc-transforms
branch
2 times, most recently
from
9c0b530 to
8e34cc0
Compare
- Modified Invertd to only invert preprocessing transforms - When orig_key == key, limits transform_info to preprocessing transforms only - Counts invertible transforms in preprocessing and uses only that many from applied_operations - Fixes issue Project-MONAI#8396 where Lambdad before Invertd caused errors - Added test case to verify the fix The issue occurred because Invertd would try to invert all transforms in applied_operations, including those from postprocessing. When Lambdad was applied in postprocessing before Invertd, it would be at the top of the stack, causing ID mismatch errors when preprocessing transforms tried to pop themselves. Fix Invertd confusion with postprocessing transforms - Modified Invertd to only invert preprocessing transforms - When orig_key == key, limits transform_info to preprocessing transforms only - Counts invertible transforms in preprocessing and uses only that many from applied_operations - Fixes issue Project-MONAI#8396 where Lambdad before Invertd caused errors - Added test case to verify the fix The issue occurred because Invertd would try to invert all transforms in applied_operations, including those from postprocessing. When Lambdad was applied in postprocessing before Invertd, it would be at the top of the stack, causing ID mismatch errors when preprocessing transforms tried to pop themselves. Signed-off-by: sewon.jeon <[email protected]>
eclipse0922
force-pushed
the
fix-invertd-postproc-transforms
branch
from
91b69ed to
7d8e0b7
Compare
Fixes #8396
Description
The issue occurred because Invertd would try to invert all transforms in applied_operations, including those from postprocessing. When Lambdad was applied in postprocessing before Invertd, it would be at the top of the stack, causing ID mismatch errors when preprocessing transforms tried to pop themselves.
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.