MONAI icon indicating copy to clipboard operation
MONAI copied to clipboard

Fix #8366: Add strict shape validation to sliding_window_inference

Open mattlin1124 opened this issue 1 month ago • 2 comments

Description

This PR addresses issue #8366 by implementing strict shape validation in sliding_window_inference.

Per the feedback from maintainers (@ericspod), implicit guessing (heuristics) for channel-last data has been avoided. Instead, this PR ensures that:

  1. The input tensor explicitly matches the expected dimensions based on roi_size (e.g., must be 5D for 3D roi_size).
  2. Validation is skipped if roi_size is an integer (broadcasting), preventing regressions in existing 1D/broadcasting tests.
  3. A clear ValueError is raised if dimensions do not match, guiding users to handle channel-last data upstream using EnsureChannelFirst or EnsureChannelFirstd.

Status

  • [x] Code changes implemented in monai/inferers/utils.py
  • [x] New unit tests added in tests/inferers/test_sliding_window_inference.py
  • [x] No changes to .gitignore

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)

mattlin1124 avatar Nov 30 '25 06:11 mattlin1124

Walkthrough

Docstring in sliding_window_inference clarified that roi_size can be a scalar or per-dimension sequence, and that inputs must be channel-first with a batch dimension (NCHW / NCDHW); it advises converting NHWC/NDHWC upstream. A Raises entry was added documenting a ValueError when input spatial dimensions do not match the roi_size length. The implementation now performs strict shape validation when roi_size is a Sequence: it checks that the input tensor's number of spatial dimensions equals len(roi_size) and raises a descriptive ValueError (including guidance about channel-first data) if not. The buffered flag initialization was moved to occur after this validation. A new test test_strict_shape_validation was added to tests/inferers/test_sliding_window_inference.py; the diff shows this test insertion duplicated. Public API and exported entities are unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the dimensionality check and exact ValueError text match expectations.
  • Confirm moving buffer initialization after validation has no unintended side effects.
  • Remove duplicate test_strict_shape_validation insertion in the test file if present.
  • Check for linting or import issues introduced by the changes.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the main change: adding strict shape validation to sliding_window_inference to fix issue #8366.
Description check ✅ Passed Description covers objectives, implementation details, status checklist, and change type; however, missing 'Fixes #' section header and incomplete types-of-changes checkboxes.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 30 '25 06:11 coderabbitai[bot]

Hi @Nic-Ma @KumoLiu @ericspod,

Just a quick update: per @ericspod 's suggestion, I have closed the old PR #8532 and updated the docstrings in this PR accordingly.

However, I noticed that the CI checks (build-docs and packaging) are now failing with the same error: OSError: [Errno 28] No space left on device.

It seems the runner ran out of disk space during the installation of heavy dependencies (like NVIDIA libraries and PyTorch). This appears to be an infrastructure issue unrelated to my recent changes.

Since I don't have permission to re-run the failed jobs, could I ask for your help to trigger a re-run?

Thanks for your help!

mattlin1124 avatar Dec 06 '25 15:12 mattlin1124