[ONNX] Fix numerical errors in softmax when dim is not last dimension #37326
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💊 Build failures summary and remediationsAs of commit 2a9c810 (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 on the GitHub issue tracker. This comment has been revised 5 times. |
p12tic
force-pushed
the
onnx-softmax-fix-numerical-errors
branch
from
63c4108
to
ea8f934
Compare
|
@p12tic could you take a look at the comments and update your PR? Thanks! |
|
@BowenBao Yes, I will do so. This week my priorities were elsewhere, thus the delay. |
p12tic
force-pushed
the
onnx-softmax-fix-numerical-errors
branch
from
ea8f934
to
2a9c810
Compare
|
@BowenBao I've applied your suggestions to the PR. Thanks for suggesting them to the PR instead of doing it yourself, I've got to learn what max normalization in softmax is :-) |
There was a problem hiding this comment.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@houseroad merged this pull request in d16c823. |
ShawnZhong
pushed a commit
to ShawnZhong/pytorch
that referenced
this pull request
May 5, 2020
…pytorch#37326) Summary: Fixes pytorch#34585. This PR improves the workaround for the problem of different semantics between ONNX softmax and Pytorch softmax. In Pytorch the `dim` parameter specifies over which dimension normalize the values. ONNX on the other hand always coerces the input into a 2D tensor and the `axis` parameter specifies which dimensions represent rows and columns of the resulting tensor. As a result, only when we are normalizing the last dimension (`dim == ndim - 1`) semantics are the same. Previously this was handled by recognizing the `dim == ndim - 1` case and using `softmax` for that. All other cases used a fallback path of explicit invocations of exp, reducesum and div operators to compute the result. Unfortunately, this results in numeric errors when input values are large: the result of exp will produce infinity on both numerator and denumerator and the division of that will result in NaN. This can be improved by transposing the input tensor so that we can reuse ONNX softmax. Similar approach has been applied to `logsoftmax` function in pytorch#30433. Pull Request resolved: pytorch#37326 Reviewed By: hl475 Differential Revision: D21389712 Pulled By: houseroad fbshipit-source-id: 554fd1b98231a28984c30c7e7abd3c0643386ff7
bharatr21
pushed a commit
to bharatr21/pytorch
that referenced
this pull request
May 5, 2020
…pytorch#37326) Summary: Fixes pytorch#34585. This PR improves the workaround for the problem of different semantics between ONNX softmax and Pytorch softmax. In Pytorch the `dim` parameter specifies over which dimension normalize the values. ONNX on the other hand always coerces the input into a 2D tensor and the `axis` parameter specifies which dimensions represent rows and columns of the resulting tensor. As a result, only when we are normalizing the last dimension (`dim == ndim - 1`) semantics are the same. Previously this was handled by recognizing the `dim == ndim - 1` case and using `softmax` for that. All other cases used a fallback path of explicit invocations of exp, reducesum and div operators to compute the result. Unfortunately, this results in numeric errors when input values are large: the result of exp will produce infinity on both numerator and denumerator and the division of that will result in NaN. This can be improved by transposing the input tensor so that we can reuse ONNX softmax. Similar approach has been applied to `logsoftmax` function in pytorch#30433. Pull Request resolved: pytorch#37326 Reviewed By: hl475 Differential Revision: D21389712 Pulled By: houseroad fbshipit-source-id: 554fd1b98231a28984c30c7e7abd3c0643386ff7
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #34585.
This PR improves the workaround for the problem of different semantics between ONNX softmax and Pytorch softmax.
In Pytorch the
dimparameter specifies over which dimension normalize the values. ONNX on the other hand always coerces the input into a 2D tensor and theaxisparameter specifies which dimensions represent rows and columns of the resulting tensor. As a result, only when we are normalizing the last dimension (dim == ndim - 1) semantics are the same.Previously this was handled by recognizing the
dim == ndim - 1case and usingsoftmaxfor that. All other cases used a fallback path of explicit invocations of exp, reducesum and div operators to compute the result. Unfortunately, this results in numeric errors when input values are large: the result of exp will produce infinity on both numerator and denumerator and the division of that will result in NaN.This can be improved by transposing the input tensor so that we can reuse ONNX softmax.
Similar approach has been applied to
logsoftmaxfunction in #30433.