[ONNX] Use optional op to keep None in results for ONNX internal tests #84789
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
[ghstack-poisoned]
titaiwangms
requested review from
BowenBao and
shubhambhokare1
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/84789
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit fe480ce: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
pytorch-bot
bot
added
the
release notes: onnx
facebook-github-bot
added
cla signed
oncall: jit
titaiwangms
changed the title
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Sep 9, 2022
ghstack-source-id: 0c21a08c9ffe1a389d743d7c5246254381fdb5b2 Pull Request resolved: #84789
titaiwangms
changed the title
…l tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Sep 15, 2022
ghstack-source-id: 2c539bc8431413af56007110fb8139ca9619771e Pull Request resolved: #84789
kit1980
reviewed
Sep 23, 2022
| int opset_version); | ||
|
|
||
| // Replace None in output with Optional node (opset > 15) if it's | ||
| // script model. This helps align the output format in internal tests. |
There was a problem hiding this comment.
Please clarify "internal tests". Which tests?
titaiwangms
changed the title
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Sep 27, 2022
ghstack-source-id: 2090c4a36b66b105314fd326a6c1a733d41af9ba Pull Request resolved: #84789
|
/easycla As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details. This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign. |
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Oct 5, 2022
ghstack-source-id: 6ba264eea7f048efe595c154a1ffc7ee474963d2 Pull Request resolved: #84789
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Oct 25, 2022
ghstack-source-id: f2bfa2003b57db97c1823794f9ad94a213b547ef Pull Request resolved: #84789
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Dec 5, 2022
ghstack-source-id: 320ca8ea5751bb8cb967f86beb67ccb1bf7dbd93 Pull Request resolved: #84789
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Dec 5, 2022
ghstack-source-id: d7980fbee2b1f42973b32374cdfdd0007767c50c Pull Request resolved: #84789
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Dec 5, 2022
ghstack-source-id: 224c02ec64790b4fe8451c1487f8b7d2910b3d19 Pull Request resolved: #84789
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Dec 6, 2022
ghstack-source-id: eb70de774a659b9d7e3f1ae7fa5fae71c8ae60f4 Pull Request resolved: #84789
Closed
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Dec 6, 2022
ghstack-source-id: eb70de774a659b9d7e3f1ae7fa5fae71c8ae60f4 Pull Request resolved: #84789
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. cc EikanWang jgong5 wenzhe-nrv sanchitintel [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Feb 5, 2023
ghstack-source-id: 0a2e05bc716a76e9f5edb25e686a3548f9ecc300 Pull Request resolved: #84789
…ternal tests" All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept. #83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None. This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones. cc EikanWang jgong5 wenzhe-nrv sanchitintel [ghstack-poisoned]
titaiwangms
added a commit
that referenced
this pull request
Feb 8, 2023
ghstack-source-id: 4272457a55ebdb618e4028406295fb5c8bb540db Pull Request resolved: #84789
|
@pytorchbot merge -g |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
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.
Stack from ghstack (oldest at bottom):
All this time, PyTorch and ONNX has different strategy for None in output. And in internal test, we flatten the torch outputs to see if the rest of them matched. However, this doesn't work anymore in scripting after Optional node is introduced, since some of None would be kept.
#83184 forces script module to keep all Nones from Pytorch, but in ONNX, the model only keeps the ones generated with Optional node, and deletes those meaningless None.
This PR uses Optional node to keep those meaningless None in output as well, so when it comes to script module result comparison, Pytorch and ONNX should have the same amount of Nones.
cc @EikanWang @jgong5 @wenzhe-nrv @sanchitintel