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.

Sign up for GitHub

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

Jump to bottom

Fix softmax_cross_entropy to handle -inf logits correctly when corresponding label is 0. #898

Merged
merged 1 commit into from
Apr 7, 2024
Merged

Fix softmax_cross_entropy to handle -inf logits correctly when corresponding label is 0. #898

merged 1 commit into from
Apr 7, 2024

Conversation

carlosgmartin
Copy link
Contributor

@carlosgmartin carlosgmartin mentioned this pull request Apr 2, 2024
Closed
@fabianp
Copy link
Member

fabianp commented Apr 3, 2024

Would perhaps using logsumexp (https://jax.readthedocs.io/en/latest/_autosummary/jax.scipy.special.logsumexp.html) be a solution here?

Also, please add tests :-)

@carlosgmartin
Copy link
Contributor Author

@fabianp Added tests.

log_softmax uses logsumexp internally: #896 (comment).

@carlosgmartin carlosgmartin mentioned this pull request Apr 5, 2024
Open
7 tasks
vroulet
vroulet approved these changes Apr 5, 2024
View reviewed changes
Copy link
Collaborator

@vroulet vroulet left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks again!
I understood also my confusion, you are modifying cross_entropy and not cross_entropy_with_integer_labels so it makes a lot of sense. Sorry for the confusion and thanks again!

@copybara-service copybara-service bot merged commit b0c04dc into google-deepmind:main Apr 7, 2024
3 of 6 checks passed
@carlosgmartin carlosgmartin deleted the softmax_cross_entropy_inf branch April 7, 2024 22:48
@vroulet vroulet mentioned this pull request Apr 8, 2024
Merged
copybara-service bot pushed a commit that referenced this pull request Apr 8, 2024
@vroulet
Correct handling of -inf in softmax_cross_entropy. Fix #898.
febee69 
Added tests for correct gradient evaluations.

PiperOrigin-RevId: 622956379
@copybara-service copybara-service bot mentioned this pull request Apr 8, 2024
Merged
copybara-service bot pushed a commit that referenced this pull request Apr 9, 2024
@vroulet
Correct handling of -inf in softmax_cross_entropy. Fix #898.
977e69b 
Added tests for correct gradient evaluations.

PiperOrigin-RevId: 622956379
copybara-service bot pushed a commit that referenced this pull request Apr 9, 2024
@vroulet
Correct handling of -inf in softmax_cross_entropy. Fix #898.
8852a72 
Added tests for correct gradient evaluations.

PiperOrigin-RevId: 623180578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vroulet vroulet vroulet approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@carlosgmartin @fabianp @vroulet