Fix division by zero in clDice loss harmonic mean

[Mr-Neutr0n]

Summary

• Fix division by zero in SoftclDiceLoss and SoftDiceclDiceLoss when computing the harmonic mean of topology precision and sensitivity
• Add a small epsilon (1e-7) to the denominator (tprec + tsens) to prevent NaN when both values are zero
• Add test cases for zero-input and non-overlapping edge cases with smooth=0

Details

The clDice loss computes cl_dice = 1.0 - 2.0 * (tprec * tsens) / (tprec + tsens). When both tprec and tsens are zero (e.g., empty inputs, non-overlapping predictions/targets, or smooth=0), this results in 0/0 = NaN, which propagates through the loss and crashes training.

While the default smooth=1.0 prevents tprec and tsens from being exactly zero in most cases, setting smooth=0 (a valid configuration) exposes this bug whenever skeleton overlap is zero. The fix adds 1e-7 to the harmonic mean denominator, which:

• Has negligible impact on normal computation (tprec, tsens are bounded in [0, 1])
• Returns cl_dice = 1.0 (maximum loss) when both precision and sensitivity are zero, which is the correct semantic result
• Is consistent with epsilon-based denominator guards used elsewhere in MONAI (e.g., smooth_dr in DiceLoss)

Test plan

• Existing test_cldice_loss.py tests still pass (perfect overlap cases)
• New test_zero_input_no_nan: verifies zero-valued inputs with smooth=0 do not produce NaN
• New test_no_overlap_no_nan: verifies non-overlapping predictions/targets with smooth=0 do not produce NaN

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds a new small regularizer parameter, smooth_dr (default 1e-7), to soft_dice and to the constructors/state of SoftclDiceLoss and SoftDiceclDiceLoss. All forward computations now include smooth_dr in Dice/clDice denominators (e.g., tprec, tsens, and combined denominators). Two unit tests were added to ensure no NaNs for zero inputs and non-overlapping predictions when smooth=0.0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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	The title accurately describes the main change: fixing division by zero in clDice loss harmonic mean computation.
Description check	✅ Passed	The description covers all required sections: summary, detailed explanation of the fix, test plan, and rationale for the approach.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

⚔️ Resolve merge conflicts

• Resolve merge conflict in branch fix/cldice-division-by-zero

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[ericspod]

Hi @Mr-Neutr0n thanks for this change, there should be a value to prevent divide-by-zero but I would suggest following the pattern found with the other Dice losses. I would add a smooth_dr constructor argument as a new last argument with a value of 1e-7 and use that value in your forward methods.

[Mr-Neutr0n]

Thanks @ericspod — the smooth_dr pattern makes much more sense. Restructured the fix to follow it:

• Added smooth_dr: float = 1e-7 as a constructor argument to SoftclDiceLoss, SoftDiceclDiceLoss, and the soft_dice helper (matching monai.losses.dice.DiceLoss).
• Used self.smooth_dr in the tprec, tsens, soft_dice, and harmonic-mean denominators — not just the harmonic mean.

The original commit only patched the harmonic-mean denominator, but the same 0/0 bug existed in the individual tprec and tsens ratios and in soft_dice. With smooth=0 and an all-zero skeleton, each ratio resolved to 0/0 = NaN before the harmonic mean was even computed.

Verified locally — all 5 tests in test_cldice_loss.py pass, including the new test_zero_input_no_nan and test_no_overlap_no_nan (which were failing on the previous commit). The 1e-7 epsilon has negligible impact on normal computation since tprec and tsens are bounded in [0, 1].

Latest push: 746afac.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

monai/losses/cldice.py (1)

95-106: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add one explicit smooth_dr regression test.

tests/losses/test_cldice_loss.py only exercises the default smooth_dr, so a wiring regression in these new public parameters would still pass. Please add one case that sets a non-default smooth_dr and asserts the forward path stays finite and uses that value.

As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."

Also applies to: 131-143, 169-182

🤖 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/losses/cldice.py` around lines 95 - 106, Add a regression unit test in
tests/losses/test_cldice_loss.py that calls the public soft_dice API with an
explicit non-default smooth_dr (e.g., 1e-3) and verifies the forward output is
finite (no NaN/inf) and that the smooth_dr value actually affects the result by
comparing outputs for two different smooth_dr values (they should differ). Do
the same for the other public clDice-related forward functions referenced in the
diff (the functions around lines 131-143 and 169-182) so each new/modified
parameter is exercised and asserted to produce finite outputs and to change when
smooth_dr changes.

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

Inline comments:
In `@monai/losses/cldice.py`:
- Around line 175-176: Update the docstring for the parameter smooth_dr in
clDice (referencing smooth_dr and the harmonic mean computation using tprec and
tsens) to explicitly state that smooth_dr is added not only to each individual
ratio denominator but also to the harmonic-mean denominator (the final tprec +
tsens + self.smooth_dr term) so the docs match the implementation; locate the
parameter description in the clDice/clDiceLoss docstring and reword it to
mention both the per-ratio denominators and the combined harmonic denominator.

---

Outside diff comments:
In `@monai/losses/cldice.py`:
- Around line 95-106: Add a regression unit test in
tests/losses/test_cldice_loss.py that calls the public soft_dice API with an
explicit non-default smooth_dr (e.g., 1e-3) and verifies the forward output is
finite (no NaN/inf) and that the smooth_dr value actually affects the result by
comparing outputs for two different smooth_dr values (they should differ). Do
the same for the other public clDice-related forward functions referenced in the
diff (the functions around lines 131-143 and 169-182) so each new/modified
parameter is exercised and asserted to produce finite outputs and to change when
smooth_dr changes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: da0bf21b-1698-4174-88d1-fd388e9fb0d2

📥 Commits

Reviewing files that changed from the base of the PR and between 0e408ee and 746afac.

📒 Files selected for processing (1)

• monai/losses/cldice.py

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Document the harmonic-mean denominator here too.

This docstring says smooth_dr is added to “each ratio denominator”, but Line 194 also applies it to the final tprec + tsens + self.smooth_dr term. The parameter docs should match the implementation.

As per coding guidelines, "Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings."

🤖 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/losses/cldice.py` around lines 175 - 176, Update the docstring for the
parameter smooth_dr in clDice (referencing smooth_dr and the harmonic mean
computation using tprec and tsens) to explicitly state that smooth_dr is added
not only to each individual ratio denominator but also to the harmonic-mean
denominator (the final tprec + tsens + self.smooth_dr term) so the docs match
the implementation; locate the parameter description in the clDice/clDiceLoss
docstring and reword it to mention both the per-ratio denominators and the
combined harmonic denominator.