-
Notifications
You must be signed in to change notification settings - Fork 22k
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
Fix nll_loss crash on cpu where ignore_index is out of bounds #17328
Conversation
@@ -39,7 +39,7 @@ void THNN_(ClassNLLCriterion_updateOutput)( | |||
for (i = 0; i < batch_size; i++) { | |||
int cur_target = THLongTensor_fastGetLegacy1dNoScalars(target, i) - TH_INDEX_BASE; | |||
|
|||
if (cur_target >= 0 && cur_target < n_classes) { | |||
if ((cur_target >= 0 && cur_target < n_classes) || (cur_target == ignore_index)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be better if you move the inner if (cur_target == ignore_index) {
block outside this?
@pytorchbot rebase this please |
Sorry, I can't rebase this because the author of this PR didn't grant maintainers permission to modify the branch. Hey @soumith! If you click the Allow edits from maintainers checkbox on the right sidebar, I can rebase PRs automatically from you. Please consider letting me help you out ;) (To learn more about this bot, see Bot commands.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@soumith is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Fixes pytorch/pytorch#15508 Pull Request resolved: pytorch/pytorch#17328 Differential Revision: D14322629 Pulled By: soumith fbshipit-source-id: 7d02f372be78794782c18affcfc109ce30b1e91c
Fixes #15508