Skip to content

Fix make_gaussian_kernel truncated parameter for kernel_size > 3#8811

Open
atharvajoshi01 wants to merge 1 commit intoProject-MONAI:devfrom
atharvajoshi01:fix/8780-gaussian-kernel-truncated
Open

Fix make_gaussian_kernel truncated parameter for kernel_size > 3#8811
atharvajoshi01 wants to merge 1 commit intoProject-MONAI:devfrom
atharvajoshi01:fix/8780-gaussian-kernel-truncated

Conversation

@atharvajoshi01
Copy link
Copy Markdown

Fixes #8780.

make_gaussian_kernel passed kernel_size // 2 as the truncated parameter to gaussian_1d. But gaussian_1d interprets truncated as the number of standard deviations, not the half-width in samples. It computes tail = int(sigma * truncated).

With kernel_size=15, sigma=5.0, truncated=7:

  • tail = 5.0 * 7 = 35 → kernel has 71 elements
  • Slicing to [:15] keeps only the far-left tail where values are near zero
  • LNCC of identical images returns ~0.0 instead of -1.0

Fix: set truncated = (kernel_size // 2) / sigma so that tail equals kernel_size // 2, producing a correctly-sized kernel.

The truncated parameter was set to kernel_size // 2, but gaussian_1d
interprets it as the number of standard deviations. With kernel_size=15
and sigma=5.0, this produced tail=35 (a 71-element kernel), which when
sliced to kernel_size gave near-zero values.

Fix: compute truncated as (kernel_size // 2) / sigma so that the
resulting tail equals kernel_size // 2, producing a correctly-sized
kernel with meaningful values.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b622c50a-8aec-49f4-a320-6658dd7c1d7f

📥 Commits

Reviewing files that changed from the base of the PR and between 8d39519 and 0a097c0.

📒 Files selected for processing (1)
  • monai/losses/image_dissimilarity.py

📝 Walkthrough

Walkthrough

Modified make_gaussian_kernel in the image dissimilarity module to compute the truncated parameter correctly. The function now normalizes the pixel radius by dividing by sigma, converting from absolute pixel units to standard deviation units before passing to gaussian_1d. The scaling and slicing logic remains unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly and concisely summarizes the main change: fixing the truncated parameter calculation in make_gaussian_kernel for larger kernel sizes.
Description check ✅ Passed Description includes issue reference, clear explanation of the bug, and the fix with concrete examples; however, it does not address the PR template sections (Types of changes, testing).
Linked Issues check ✅ Passed Changes directly address issue #8780: correcting truncated calculation from kernel_size // 2 to (kernel_size // 2) / sigma produces properly sized Gaussian kernels.
Out of Scope Changes check ✅ Passed Changes are tightly scoped: only the truncated parameter computation in make_gaussian_kernel is modified; no extraneous alterations present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@ericspod
Copy link
Copy Markdown
Member

ericspod commented Apr 9, 2026

Hi @atharvajoshi01 thanks for this fix. We had PR #8781 already open for it but it initially didn't mention the original issue so you wouldn't have seen it. I think it's equivalent to your solution, if you wouldn't mind please check that you agree with that. I think we would merge the other PR as it also has tests included. Sorry you spent time on this, we should have gotten to the other PR sooner.

@atharvajoshi01
Copy link
Copy Markdown
Author

Checked #8781 — yes, both fixes address the same root cause. Their approach is actually cleaner since it inlines the Gaussian computation directly instead of going through gaussian_1d, which avoids the truncated parameter issue entirely. Happy to close this one in favor of #8781.

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.

Bug in make_gaussian_kernel causes incorrect LocalNormalizedCrossCorrelationLoss when kernel_size > 3

2 participants