fix(apps/nuclick): replace np.random.choice with self.R.choice in Add…#8836
Conversation
…PointGuidanceSignald In AddPointGuidanceSignald.exclusion_map, two calls to np.random.choice were using the global NumPy random state instead of the instance's self.R (RandomState). Since AddPointGuidanceSignald already inherits from Randomizable, replacing with self.R.choice ensures reproducibility when set_determinism() or manual seeding is used. Ref Project-MONAI#6888 Signed-off-by: Zeeshan Modi <92383127+Zeesejo@users.noreply.github.com> Signed-off-by: Zeeshan Modi <92383127+Zeesejo@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Pull request overview
Updates NuClick’s AddPointGuidanceSignald transform to use its local MONAI Randomizable RNG (self.R) instead of NumPy’s global RNG for drop decisions, improving reproducibility when seeding is used via set_random_state() / Compose.set_random_state().
Changes:
- Replaced two
np.random.choice(...)calls withself.R.choice(...)insideAddPointGuidanceSignald.exclusion_map.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Project-MONAI#8836) Fixes Project-MONAI#6888 (partial) - follow-up to Project-MONAI#8798 ### Description In `AddPointGuidanceSignald.exclusion_map` (`monai/apps/nuclick/transforms.py`), two calls used `np.random.choice` (global NumPy random state) instead of `self.R.choice`. Since `AddPointGuidanceSignald` already inherits from `Randomizable`, this is a straightforward replacement that ensures reproducibility when `set_determinism()` or manual seeding is used. **Before:** ```python if np.random.choice([True, False], p=[drop_rate, 1 - drop_rate]): ``` **After:** ```python if self.R.choice([True, False], p=[drop_rate, 1 - drop_rate]): ``` PR Project-MONAI#8798 already fixed `np.random.*` calls in transforms and data utilities. This PR continues that work for the two remaining calls in `apps/nuclick/transforms.py` mentioned in Project-MONAI#6888. ### Types of changes - [x] 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. Signed-off-by: Zeeshan Modi <92383127+Zeesejo@users.noreply.github.com>
Fixes #6888 (partial) - follow-up to #8798
Description
In
AddPointGuidanceSignald.exclusion_map(monai/apps/nuclick/transforms.py), two calls usednp.random.choice(global NumPy random state) instead ofself.R.choice. SinceAddPointGuidanceSignaldalready inherits fromRandomizable, this is a straightforward replacement that ensures reproducibility whenset_determinism()or manual seeding is used.Before:
After:
PR #8798 already fixed
np.random.*calls in transforms and data utilities. This PR continues that work for the two remaining calls inapps/nuclick/transforms.pymentioned in #6888.Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.