Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add label smoothing param in DiceCELoss #8000

Merged
merged 8 commits into from
Aug 9, 2024

Conversation

ytl0623
Copy link
Contributor

@ytl0623 ytl0623 commented Aug 7, 2024

Fixes #7957

Description

In this modified version I made the following changes:

  1. Added label_smoothing: float = 0.0 parameter in __init__ method, default value is 0.0.
  2. When creating the self.cross_entropy instance, pass the label_smoothing parameter to nn.CrossEntropyLoss.
  3. Added self.label_smoothing = label_smoothing in the __init__ method to save this parameter for access when needed.

For example:

from monai.losses import DiceCELoss

# Before
criterion = DiceCELoss()
criterion.cross_entropy.label_smoothing = 0.1

# Now
criterion = DiceCELoss(label_smoothing=0.1)

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Fixes Project-MONAI#7957

Signed-off-by: ytl0623 <david89062388@gmail.com>
Copy link
Contributor

@KumoLiu KumoLiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, minor comments inline.

monai/losses/dice.py Show resolved Hide resolved
monai/losses/dice.py Outdated Show resolved Hide resolved
@KumoLiu KumoLiu requested review from ericspod and Nic-Ma August 7, 2024 17:01
Fixes Project-MONAI#7957

Signed-off-by: ytl0623 <david89062388@gmail.com>
Copy link
Contributor

@KumoLiu KumoLiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick update.

@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 8, 2024

/build

Signed-off-by: ytl0623 <david89062388@gmail.com>
@ytl0623
Copy link
Contributor Author

ytl0623 commented Aug 8, 2024

Hi @KumoLiu,

Please review the changes to ensure the maximum length limit.

Thank you!

@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 8, 2024

/build

@ytl0623
Copy link
Contributor Author

ytl0623 commented Aug 8, 2024

Hi @KumoLiu,

I've encountered an issue with the Blossom-CI on GitHub, specifically with build #15928, which has failed.
Could you please help me look into the error and resolve it?

Thank you in advance for your assistance!

@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 8, 2024

Hi @ytl0623,
Here is the error message:

[2024-08-08T08:40:54.212Z] ======================================================================
[2024-08-08T08:40:54.212Z] ERROR: test_ill_shape (tests.test_ds_loss.TestDSLossDiceCE)
[2024-08-08T08:40:54.212Z] ----------------------------------------------------------------------
[2024-08-08T08:40:54.212Z] Traceback (most recent call last):
[2024-08-08T08:40:54.212Z]   File "/home/jenkins/agent/workspace/MONAI-premerge/monai/tests/test_ds_loss.py", line 146, in test_ill_shape
[2024-08-08T08:40:54.212Z]     loss = DeepSupervisionLoss(DiceCELoss())
[2024-08-08T08:40:54.212Z]   File "/home/jenkins/agent/workspace/MONAI-premerge/monai/monai/utils/deprecate_utils.py", line 223, in _wrapper
[2024-08-08T08:40:54.212Z]     return func(*args, **kwargs)
[2024-08-08T08:40:54.212Z]   File "/home/jenkins/agent/workspace/MONAI-premerge/monai/monai/losses/dice.py", line 735, in __init__
[2024-08-08T08:40:54.212Z]     self.cross_entropy = nn.CrossEntropyLoss(weight=weight, reduction=reduction, label_smoothing=label_smoothing)
[2024-08-08T08:40:54.212Z] TypeError: __init__() got an unexpected keyword argument 'label_smoothing'
[2024-08-08T08:40:54.212Z] 
[2024-08-08T08:40:54.212Z] ======================================================================
[2024-08-08T08:40:54.212Z] ERROR: test_result_0 (tests.test_ds_loss.TestDSLossDiceCE)
[2024-08-08T08:40:54.212Z] ----------------------------------------------------------------------
[2024-08-08T08:40:54.212Z] Traceback (most recent call last):
[2024-08-08T08:40:54.212Z]   File "/usr/local/lib/python3.9/dist-packages/parameterized/parameterized.py", line 620, in standalone_func
[2024-08-08T08:40:54.212Z]     return func(*(a + p.args), **p.kwargs, **kw)
[2024-08-08T08:40:54.212Z]   File "/home/jenkins/agent/workspace/MONAI-premerge/monai/tests/test_ds_loss.py", line 141, in test_result
[2024-08-08T08:40:54.212Z]     diceceloss = DeepSupervisionLoss(DiceCELoss(**input_param), **input_param2)
[2024-08-08T08:40:54.212Z]   File "/home/jenkins/agent/workspace/MONAI-premerge/monai/monai/utils/deprecate_utils.py", line 223, in _wrapper
[2024-08-08T08:40:54.212Z]     return func(*args, **kwargs)
[2024-08-08T08:40:54.212Z]   File "/home/jenkins/agent/workspace/MONAI-premerge/monai/monai/losses/dice.py", line 735, in __init__
[2024-08-08T08:40:54.212Z]     self.cross_entropy = nn.CrossEntropyLoss(weight=weight, reduction=reduction, label_smoothing=label_smoothing)
[2024-08-08T08:40:54.212Z] TypeError: __init__() got an unexpected keyword argument 'label_smoothing'

And the reason is that label_smoothing is introduced after 1.9.1, could you please add a check and only send it to CrossEntropyLoss after 1.9.1.
https://github.com/pytorch/pytorch/blob/v1.10.0/torch/nn/modules/loss.py

Something like:

if pytorch_after(2, 1):

Thanks!

Signed-off-by: ytl0623 <david89062388@gmail.com>
@ytl0623
Copy link
Contributor Author

ytl0623 commented Aug 8, 2024

Hi @KumoLiu,

I think this is okay now. Could you please help me make some corrections?

Thanks for your help.

@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 8, 2024

Could you please run ./runtests.sh --autofix locally to do the codeformat check? Thanks!

Signed-off-by: ytl0623 <david89062388@gmail.com>
@ytl0623
Copy link
Contributor Author

ytl0623 commented Aug 8, 2024

Hi @KumoLiu,

Done! Thanks :)

@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 8, 2024

/build

@KumoLiu KumoLiu enabled auto-merge (squash) August 8, 2024 16:48
@KumoLiu
Copy link
Contributor

KumoLiu commented Aug 9, 2024

/build

@KumoLiu KumoLiu merged commit 0bb05d7 into Project-MONAI:dev Aug 9, 2024
28 checks passed
rcremese pushed a commit to rcremese/MONAI that referenced this pull request Sep 2, 2024
Fixes Project-MONAI#7957 

### Description
In this modified version I made the following changes:
1. Added `label_smoothing: float = 0.0` parameter in `__init__` method,
default value is 0.0.
2. When creating the `self.cross_entropy` instance, pass the
`label_smoothing` parameter to `nn.CrossEntropyLoss`.
3. Added `self.label_smoothing = label_smoothing` in the `__init__`
method to save this parameter for access when needed.

For example:
```
from monai.losses import DiceCELoss

# Before
criterion = DiceCELoss()
criterion.cross_entropy.label_smoothing = 0.1

# Now
criterion = DiceCELoss(label_smoothing=0.1)
```

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: ytl0623 <david89062388@gmail.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
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.

Add label smoothing param in DiceCELoss
2 participants