[ADD] soft_label argument in DiceCELoss #8666
Conversation
WalkthroughAdded a Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
monai/losses/dice.py (1)
882-895: Consider exposingsoft_labelinDiceFocalLossfor API consistency.
DiceCELossnow exposessoft_label, butDiceFocalLoss(andGeneralizedDiceFocalLossat lines 1008-1019) instantiateDiceLoss/GeneralizedDiceLosswithout this option. Users needing soft labels with focal loss would face asymmetric API behavior.This can be addressed in a follow-up PR if desired.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
monai/losses/dice.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. 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. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/losses/dice.py
🔇 Additional comments (3)
monai/losses/dice.py (3)
70-70: LGTM!Parameter correctly added, documented in Google-style, and stored for use.
Also applies to: 102-104, 129-129
282-282: LGTM!Consistent with DiceLoss implementation.
Also applies to: 306-307, 332-332
677-677: LGTM!Correctly exposes
soft_labeland passes it to the internalDiceLoss.Also applies to: 719-720, 743-743
| lambda_dice: float = 1.0, | ||
| lambda_ce: float = 1.0, | ||
| label_smoothing: float = 0.0, | ||
| soft_label: bool = False, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find test files and search for soft_label references
find . -type f -name "*test*dice*" -o -name "*dice*test*" | head -20
echo "---"
rg -n "soft_label" --type=py -g '*test*' | head -30Repository: Project-MONAI/MONAI
Length of output: 1666
🏁 Script executed:
#!/bin/bash
# Check test_dice_ce_loss.py for soft_label coverage
echo "=== test_dice_ce_loss.py content ==="
wc -l ./tests/losses/test_dice_ce_loss.py
echo "---"
head -100 ./tests/losses/test_dice_ce_loss.py
echo "---"
rg -n "soft_label" ./tests/losses/test_dice_ce_loss.py
# Check monai/losses/dice.py around line 677
echo ""
echo "=== monai/losses/dice.py around line 677 ==="
sed -n '670,690p' ./monai/losses/dice.pyRepository: Project-MONAI/MONAI
Length of output: 5289
🏁 Script executed:
#!/bin/bash
# Check how soft_label is used in DiceCELoss
echo "=== Check DiceCELoss implementation of soft_label ==="
sed -n '600,750p' ./monai/losses/dice.py | grep -A 30 "class DiceCELoss"
echo ""
echo "=== Full __init__ and forward of DiceCELoss ==="
sed -n '600,800p' ./monai/losses/dice.py | tail -100Repository: Project-MONAI/MONAI
Length of output: 6067
🏁 Script executed:
#!/bin/bash
# Let's verify that soft_label is actually being used in DiceLoss forward
echo "=== Check if soft_label is used in DiceLoss.forward ==="
rg -n "soft_label" ./monai/losses/dice.py | head -20Repository: Project-MONAI/MONAI
Length of output: 822
🏁 Script executed:
#!/bin/bash
# Verify the test_dice_loss.py has soft_label tests
echo "=== test_dice_loss.py soft_label test cases ==="
sed -n '35,50p' ./tests/losses/test_dice_loss.pyRepository: Project-MONAI/MONAI
Length of output: 850
Add unit test for DiceCELoss with soft_label=True.
The soft_label parameter was added to DiceCELoss but test cases in test_dice_ce_loss.py do not cover this parameter. Add test cases with soft_label=True to ensure proper functionality, consistent with existing DiceLoss tests.
1 participant
Fixes # .
Description
MONAI's DiceLoss supports soft_label in case the segmentation mask (i.e., the target) contains non-binary values. This functionality is already implemented. However, when using DiceCELoss it is not possible to specify the argument 'soft_label' for the DiceLoss part.
This simple fix adds 'soft_label' as an argument of the DiceCELoss to be able to control whether the DiceLoss part should use the soft or hard label formulation.
Types of changes