Perf: skip redundant full-image mask on StdShiftIntensity nonzero=False path#8975
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
Estimated code review effort: 1 (Trivial) | ~3 minutes Related issues: Suggested labels: performance, transforms 🚥 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 |
a9f6888 to
f72c648
Compare
On the default nonzero=False path the boolean mask is all-True, so img[slices] just gathers and scatters the whole image. Shift the image directly instead, avoiding the mask allocation, .any(), gather and scatter. Output is unchanged. Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
f72c648 to
2e4fafe
Compare
Address review: keep a single unified code path and preserve in-place semantics by selecting the whole tensor with an empty-tuple index (a view) instead of allocating a full boolean mask. Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
Fixes #8972 .
Description
On the default
nonzero=FalsepathStdShiftIntensity._stdshiftbuilt an all-True boolean mask the size of the image and shifted through it (img[slices] = img[slices] + offset), forcing a full advanced-index gather and scatter plus the mask allocation and.any()scan even though every voxel is selected. That is equivalent to shifting the image directly, so thenonzero=Falsebranch now doesimg + factor * std(img). Output is bit-for-bit identical and thenonzero=Truepath is untouched.RandStdShiftIntensitybenefits too, since it also defaults tononzero=False. Measured across 2D/3D, single and multi channel, float32 and float64 on both backends (best-of-3, CPU); output verified equal in every configuration:numpy ranges 1.55x to 2.65x and torch 8.5x to 66x across the full sweep; the largest gains are on torch, where all-True boolean-mask indexing is especially costly.
Types of changes