Skip to content

Fix RandGridPatch whole-dimension patch sizes - #9048

Open
Rajioba1 wants to merge 2 commits into
Project-MONAI:devfrom
Rajioba1:9046-fix-rand-grid-patch-whole-dim
Open

Fix RandGridPatch whole-dimension patch sizes#9048
Rajioba1 wants to merge 2 commits into
Project-MONAI:devfrom
Rajioba1:9046-fix-rand-grid-patch-whole-dim

Conversation

@Rajioba1

@Rajioba1 Rajioba1 commented Aug 3, 2026

Copy link
Copy Markdown

Fixes #9046.

Description

RandGridPatch documents that patch_size entries of 0 or None select the whole dimension. When max_offset is omitted, the transform currently computes the default offset range with s % p, which raises for 0 and None patch dimensions.

This PR treats whole-dimension patch entries as having zero random offset for that dimension, matching the documented behavior and GridPatch behavior. It adds regression coverage for both RandGridPatch and RandGridPatchd with patch_size=(0, 2) and patch_size=(None, 2).

Types of changes

  • 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.

Local validation:

  • python -m tests.transforms.spatial.test_rand_grid_patch
  • python -m tests.transforms.spatial.test_rand_grid_patchd
  • git diff --check

Notes:

  • black and isort were not installed in my local Python environment, so I could not run those checks directly.
  • python -m ruff check monai/transforms/spatial/array.py tests/transforms/spatial/test_rand_grid_patch.py tests/transforms/spatial/test_rand_grid_patchd.py reports an unrelated pre-existing UP038 warning in array.py outside this PR's changed lines.

Signed-off-by: Rajioba1 <raji.lukmon@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8181fe72-9912-472c-9a68-21d23e4b9c12

📥 Commits

Reviewing files that changed from the base of the PR and between 7e9303e and 2ce2116.

📒 Files selected for processing (2)
  • tests/transforms/spatial/test_rand_grid_patch.py
  • tests/transforms/spatial/test_rand_grid_patchd.py
💤 Files with no reviewable changes (2)
  • tests/transforms/spatial/test_rand_grid_patchd.py
  • tests/transforms/spatial/test_rand_grid_patch.py

📝 Walkthrough

Walkthrough

RandGridPatch.randomize now handles zero patch dimensions without modulo-by-zero errors. New parameterized tests verify zero-dimension patches for RandGridPatch and RandGridPatchd across supported input types.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changes address zero-sized dimensions, but the summary does not confirm support or regression coverage for the required None dimensions in issue #9046. Confirm that default max_offset handles patch_size=None and add or verify regression tests for both RandGridPatch and RandGridPatchd.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: fixing whole-dimension patch sizes in RandGridPatch.
Description check ✅ Passed The description explains the bug, solution, scope, tests, and local validation, with only optional checks left unreported.
Out of Scope Changes check ✅ Passed All reported code and test changes relate directly to the RandGridPatch whole-dimension behavior described in issue #9046.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
monai/transforms/spatial/array.py (1)

3570-3570: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Make the dimension contract explicit.

zip silently truncates when array.shape[1:] and self.patch_size have different lengths. This can create an incomplete max_offset and an invalid self.offset. Pass strict=True.

Proposed fix
-            max_offset = tuple(s % p if p else 0 for s, p in zip(array.shape[1:], self.patch_size))
+            max_offset = tuple(
+                s % p if p else 0
+                for s, p in zip(array.shape[1:], self.patch_size, strict=True)
+            )

Ruff B905 identifies this call because it omits strict=.

🤖 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 `@monai/transforms/spatial/array.py` at line 3570, Update the max_offset
computation in the relevant transform method to call zip with strict=True,
ensuring array.shape[1:] and self.patch_size must have matching dimensions
instead of being silently truncated.

Source: Linters/SAST tools

🤖 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 `@monai/transforms/spatial/array.py`:
- Line 3570: Update the max_offset computation in the relevant transform method
to call zip with strict=True, ensuring array.shape[1:] and self.patch_size must
have matching dimensions instead of being silently truncated.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1087d4fe-91b6-4634-bbc4-e4bffa33fc5a

📥 Commits

Reviewing files that changed from the base of the PR and between 8690ae7 and 7e9303e.

📒 Files selected for processing (3)
  • monai/transforms/spatial/array.py
  • tests/transforms/spatial/test_rand_grid_patch.py
  • tests/transforms/spatial/test_rand_grid_patchd.py

Signed-off-by: Rajioba1 <raji.lukmon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RandGridPatch crashes for documented 0 or None patch_size entries

1 participant