Skip to content

Fix LocalNormalizedCrossCorrelationLoss exceeding its expected range in low-variance regions - #9180

Open
ousamabenyounes wants to merge 1 commit into
Project-MONAI:devfrom
ousamabenyounes:fix/issue-9178
Open

ousamabenyounes wants to merge 1 commit into
Project-MONAI:devfrom
ousamabenyounes:fix/issue-9178

Conversation

@ousamabenyounes

@ousamabenyounes ousamabenyounes commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #9178 .

Description

LocalNormalizedCrossCorrelationLoss could return values outside the [-1, 1]
range it documents. In a near-constant window the variance terms are computed by
cancellation (E[x²] - E[x]²) in float32, which can land near zero or slightly
negative; dividing by that denominator makes the squared NCC blow up instead of
saturating at 1.

Reproduced on dev with the existing test data, before the change:

AssertionError: -5.23868989944458 not greater than or equal to -1.0
AssertionError: -523.8689575195312 not greater than or equal to -1.0
Ran 21 tests in 0.021s
FAILED (failures=8)

After the change the same suite passes:

Ran 21 tests in 0.021s
OK

Windows whose variance falls below the smooth_dr threshold are now treated as
having no correlation to measure: the result is zeroed and no gradient flows
through them, rather than propagating an out-of-range ratio. smooth_dr must be
strictly positive, which is now validated in __init__ instead of silently
producing a division by zero.

Added coverage for the low-variance case (both input orders), the exact-threshold
boundary, gradient behaviour, and the new smooth_dr validation.

Verified locally with python -m unittest tests.losses.image_dissimilarity.test_local_normalized_cross_correlation_loss, plus black --check, ruff check and isort --check-only on the two touched files. The full runtests.sh suite was not run locally.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Project-MONAI/MONAI/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3bc820f9-1301-4d53-9c2c-ddd010254839
📥 Commits

Reviewing files that changed from the base of the PR and between 6aa4abf and c11a120.

📒 Files selected for processing: 2
  • monai/losses/image_dissimilarity.py
  • tests/losses/image_dissimilarity/test_local_normalized_cross_correlation_loss.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

LocalNormalizedCrossCorrelationLoss now requires a positive smooth_dr. Windows where either input variance is at or below that threshold contribute zero. Valid squared NCC values are capped at 1. New tests cover low-variance inputs, threshold boundaries, invalid smooth_dr values, and prediction gradients.

Priority: ➖ Normal

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

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c11a1

No material behavior or deployment issue is established; the change is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Issue #9178 requires positive smooth_dr, raw variance checks, zero contribution for windows with either variance at or below the threshold, and an upper bound of 1. `LocalNormalizedCrossCorrelationL…
Out of Scope Changes check Passed The reviewed changes are limited to monai/losses/image_dissimilarity.py and its focused local NCC test file. The implementation, docstring updates, and regression tests directly support Issue #9178.…
Title check Passed The title clearly and concisely describes the primary fix: preventing LocalNormalizedCrossCorrelationLoss from exceeding its expected range in low-variance regions.
Description check Passed The description explains the defect, implementation, tests, validation, and change types. It omits some template checkboxes, but it clearly states that the full suite was not run and remains sufficien…
Full details: Linked Issues check

Explanation

Issue #9178 requires positive smooth_dr, raw variance checks, zero contribution for windows with either variance at or below the threshold, and an upper bound of 1. LocalNormalizedCrossCorrelationLoss implements all four rules. The constructor rejects nonpositive and NaN smooth_dr. The forward path masks invalid windows, uses zero contribution, and clamps valid squared NCC to 1. Regression tests cover low variance, threshold equality, invalid smooth_dr, finite gradients, and zero gradients.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to monai/losses/image_dissimilarity.py and its focused local NCC test file. The implementation, docstring updates, and regression tests directly support Issue #9178. No unrelated product behavior or unrelated files are evidenced.

Full details: Description check

Explanation

The description explains the defect, implementation, tests, validation, and change types. It omits some template checkboxes, but it clearly states that the full suite was not run and remains sufficiently complete.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

Signed-off-by: Ousama Ben Younes <benyounes.ousama@gmail.com>

This branch has not been deployed

No deployments
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.

LocalNormalizedCrossCorrelationLoss can exceed its expected range in low-variance regions

1 participant