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]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The change clarifies the smooth_dr docstring in SoftclDiceLoss and adds regression tests for SoftclDiceLoss and SoftDiceclDiceLoss. The new tests check finite losses on all-zero inputs and confirm that different smooth_dr values produce different loss outputs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% 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 clearly matches the main change: fixing a clDice loss division-by-zero issue in the harmonic mean.
Description check	✅ Passed	The description covers the fix, rationale, and tests, but it omits the template's Types of changes section and issue reference.
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

---

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

[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

[ericspod]

Hi @Mr-Neutr0n please have a look at the conflicts that need resolving, There's been changes to the section of code you wanted to update, unfortunately you'll have to untangle where your changes should go.

[Mr-Neutr0n]

Hi @ericspod — I rebased onto the current dev branch and resolved the conflict.

While resolving it I saw that the core guard you asked for is already present on dev (added in #8703): the precision/sensitivity denominators use smooth_dr and the harmonic mean uses smooth, with smooth validated as positive. So the source-level division-by-zero issue is already covered upstream.

I turned this PR into the missing regression coverage instead:

• Added zero-input finite-output tests for both SoftclDiceLoss and SoftDiceclDiceLoss.
• Added non-default smooth_dr tests to verify the parameter is wired through and actually changes the result.
• Clarified the SoftclDiceLoss smooth_dr docstring.

The branch is now clean on top of dev. Let me know if you'd rather I close this and open a fresh test-only PR.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@tests/losses/test_cldice_loss.py`:
- Around line 117-132: The current tests for SoftclDiceLoss only cover all-zero
inputs, so they miss the non-overlap case this change is meant to protect. Add
explicit regression tests using non-overlapping input/target tensors for each
affected loss class in test_cldice_loss.py, and assert the returned loss is
finite; use the existing SoftclDiceLoss test methods as the pattern and extend
the corresponding loss-specific test sections referenced by the current
additions.

🪄 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: 051147a5-e817-4147-a8d1-548927d84a7f

📥 Commits

Reviewing files that changed from the base of the PR and between 746afac and 45216c2.

📒 Files selected for processing (2)

• monai/losses/cldice.py
• tests/losses/test_cldice_loss.py

[ericspod]

Hi @Mr-Neutr0n I guess other changes got to this fix first, sorry about that. I'm fine with keeping this PR and merging shortly, if you could add the tests as Coderabbit suggests we'd be good to go.

[Mr-Neutr0n]

Hi @ericspod — added the non-overlap finiteness tests for both SoftclDiceLoss and SoftDiceclDiceLoss as suggested. The branch is rebased on dev and should be ready for another look.

[ericspod]

@Mr-Neutr0n looks good now thanks, I can pass your DCO for you if it's okay with you.

[Mr-Neutr0n]

Thanks for the review, @ericspod. Yes, please go ahead and pass the DCO for me — the commit messages are already signed-off. Let me know if anything else is needed.