perf(test): cache SwinUNETR SSL weights instead of re-downloading - #8987
perf(test): cache SwinUNETR SSL weights instead of re-downloading#8987aymuos15 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe test replaces Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This localized test-only change reuses cached pretrained weights without changing product behavior, and no actionable merge-blocking risk remains beyond normal checks and review. 🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/networks/nets/test_swin_unetr.py (1)
38-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
Path/operator consistently.
Pathis imported butos.path.joinis still used for path construction. Prefer the/operator for consistency.♻️ Proposed refactor
- weight_path = os.path.join(TESTS_PATH, "testing_data", f"temp_{file_name}") + weight_path = TESTS_PATH / "testing_data" / f"temp_{file_name}"Also applies to: 118-118
🤖 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/networks/nets/test_swin_unetr.py` around lines 38 - 39, The path construction in test_swin_unetr.py is inconsistent because Path is imported but os.path.join is still used in the test setup. Update the affected path-building code to use the Path "/" operator consistently, using TESTS_PATH and any related path variables/helpers in this file instead of os.path.join so the tests follow one style throughout.
🤖 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.
Nitpick comments:
In `@tests/networks/nets/test_swin_unetr.py`:
- Around line 38-39: The path construction in test_swin_unetr.py is inconsistent
because Path is imported but os.path.join is still used in the test setup.
Update the affected path-building code to use the Path "/" operator
consistently, using TESTS_PATH and any related path variables/helpers in this
file instead of os.path.join so the tests follow one style throughout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b17c3e53-2745-4f30-b342-84bb44ad3b2f
📒 Files selected for processing (1)
tests/networks/nets/test_swin_unetr.py
Description
Cache the SwinUNETR SSL pretrained weights (~750 MB) under
tests/testing_data/instead of a per-runtempfile.TemporaryDirectory(), sotest_filter_swinunetrreuses them across runs instead of re-downloading every time.download_urlskips the download when the cached file's hash matches, so repeated runs replace a ~750 MB download with a local hash-check (~5x faster). The cache path is gitignored and behavior/coverage are unchanged.Changes
tests/networks/nets/test_swin_unetr.py: download the weights totests/testing_data/temp_ssl_pretrained_weights.pth(newTESTS_PATHconstant) instead of a temporary directory; drop thetempfileimport and addfrom pathlib import Path.Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests.