Skip to content

Fix incomplete activation validation in HausdorffDTLoss#8841

Open
aymuos15 wants to merge 2 commits intoProject-MONAI:devfrom
aymuos15:fix/hausdorff-activation-validation
Open

Fix incomplete activation validation in HausdorffDTLoss#8841
aymuos15 wants to merge 2 commits intoProject-MONAI:devfrom
aymuos15:fix/hausdorff-activation-validation

Conversation

@aymuos15
Copy link
Copy Markdown
Contributor

@aymuos15 aymuos15 commented May 4, 2026

The validation check for mutually exclusive activation options was incomplete - it only checked sigmoid and softmax but not other_act, despite the error message explicitly mentioning other_act.

Without this check, passing e.g. sigmoid=True, other_act=relu silently stacks both activations in forward() (other_act(sigmoid(x))) instead of applying only one, producing an incorrect loss with no warning.

Before:

if int(sigmoid) + int(softmax) > 1:
    raise ValueError("... [sigmoid=True, softmax=True, other_act is not None].")

After:

if int(sigmoid) + int(softmax) + int(other_act is not None) > 1:
    raise ValueError("... [sigmoid=True, softmax=True, other_act is not None].")

This is consistent with the validation in dice.py and tversky.py which correctly include all three options in the check.

Added tests for:

  • sigmoid=True with other_act
  • softmax=True with other_act
  • All three options combined

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).

The validation check for mutually exclusive activation options was
incomplete - it only checked sigmoid and softmax but not other_act,
despite the error message explicitly mentioning other_act.

Before:
```python
if int(sigmoid) + int(softmax) > 1:
    raise ValueError("... [sigmoid=True, softmax=True, other_act is not None].")
```

After:
```python
if int(sigmoid) + int(softmax) + int(other_act is not None) > 1:
    raise ValueError("... [sigmoid=True, softmax=True, other_act is not None].")
```

This is consistent with the validation in dice.py and tversky.py which
correctly include all three options in the check.

Added tests for:
- sigmoid=True with other_act
- softmax=True with other_act
- All three options combined

Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 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: 1e2f9f24-88ca-4a6f-9ac1-5a1b4688c6be

📥 Commits

Reviewing files that changed from the base of the PR and between 65beb58 and 484bc7a.

📒 Files selected for processing (2)
  • monai/losses/hausdorff_loss.py
  • tests/losses/test_hausdorff_loss.py

📝 Walkthrough

Walkthrough

The change enhances validation in HausdorffDTLoss to enforce mutual exclusivity among activation function parameters: sigmoid, softmax, and other_act. Previously, the validation only checked sigmoid and softmax, allowing incompatible combinations involving other_act. The fix extends the incompatibility guard to include all three parameters. Corresponding test coverage is added to assert ValueError is raised for all invalid option combinations.

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 33.33% 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 specifically describes the main change: fixing incomplete activation validation in HausdorffDTLoss.
Description check ✅ Passed Description covers the problem, solution, code changes, and test additions. Matches template structure with Types of changes checked appropriately.
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
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

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.

1 participant