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

ENH Adds label_smoothing to cross entropy loss #63122

Closed
wants to merge 43 commits into from

Conversation

thomasjpfan
Copy link
Contributor

@thomasjpfan thomasjpfan commented Aug 12, 2021

@thomasjpfan thomasjpfan added module: nn Related to torch.nn triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Aug 12, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 12, 2021

🔗 Helpful links

💊 CI failures summary and remediations

As of commit 1a41ce5 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@thomasjpfan thomasjpfan changed the title ENH Adds label_smoothing to targets as class indices ENH Adds label_smoothing to targets as class indices in cross_entropy Aug 12, 2021
@ezyang ezyang removed their request for review August 12, 2021 13:49
@albanD albanD removed their request for review August 12, 2021 15:03
Copy link
Contributor

@jbschlosser jbschlosser 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 tackling this! Added some comments below. To summarize:

  • I think we want the end result to handle both ignore_index and class prob targets
  • Can the implementation be simplified by doing label smoothing as additional math outside of the call to at::nll_loss_nd and doing a linear combination?

aten/src/ATen/native/LossNLL.cpp Outdated Show resolved Hide resolved
aten/src/ATen/native/LossNLL.cpp Outdated Show resolved Hide resolved
aten/src/ATen/native/LossNLL.cpp Outdated Show resolved Hide resolved
aten/src/ATen/native/LossNLL.cpp Outdated Show resolved Hide resolved
torch/nn/modules/loss.py Outdated Show resolved Hide resolved
test/test_nn.py Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@jbschlosser jbschlosser left a comment

Choose a reason for hiding this comment

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

LGTM!

@facebook-github-bot
Copy link
Contributor

@datumbox has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@datumbox has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@codecov
Copy link

codecov bot commented Aug 28, 2021

Codecov Report

Merging #63122 (1a41ce5) into master (3abb606) will increase coverage by 0.00%.
The diff coverage is 88.88%.

@@           Coverage Diff           @@
##           master   #63122   +/-   ##
=======================================
  Coverage   66.73%   66.74%           
=======================================
  Files         698      698           
  Lines       90892    90922   +30     
=======================================
+ Hits        60661    60690   +29     
- Misses      30231    30232    +1     

@facebook-github-bot
Copy link
Contributor

@jbschlosser merged this pull request in d3bcba5.

jbschlosser added a commit to jbschlosser/nestedtensor that referenced this pull request Sep 2, 2021
Summary:
Adds awareness of the new `label_smoothing` functionality in `cross_entropy` added in pytorch/pytorch#63122 to nestedtensor's implementation.

Fixes broken test: `test.test_nested_tensor_functional.TestFunctional`.

Differential Revision: D30730728

fbshipit-source-id: 34d907b76a1d2058cc7bc7d12c271cc3088a351b
facebook-github-bot pushed a commit to pytorch/nestedtensor that referenced this pull request Sep 2, 2021
Summary:
Pull Request resolved: #452

Adds awareness of the new `label_smoothing` functionality in `cross_entropy` added in pytorch/pytorch#63122 to nestedtensor's implementation.

Fixes broken test: `test.test_nested_tensor_functional.TestFunctional`.

Reviewed By: cpuhrsch

Differential Revision: D30730728

fbshipit-source-id: 04f146d6de7f764f165059e4b5654d7f39142e38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed Merged module: nn Related to torch.nn open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
7 participants