Disabling ONNX IR v4 sematics for opset 8 or lower. #28990
Closed
Conversation
This file contains hidden or 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
houseroad
reviewed
Nov 4, 2019
test/onnx/test_operators.py
Outdated
| # and initializers are listed as ONNX graph input, in accordance | ||
| # with ONNX IR v3 semantics (which apply to opset version <= 8). | ||
| x = torch.ones(20, 16, 50, 40, requires_grad=True) | ||
| self.assertONNX(nn.Conv2d(16, 13, 3, bias=False), x, opset_version=8, |
There was a problem hiding this comment.
can we use a smaller kernel and zero them, otherwise, we may problem due to the randomness in the initializers.
There was a problem hiding this comment.
Yes, that makes sense. I have updated the PR to use a smaller kernel and all ones weights. Thanks.
spandantiwari
force-pushed
the
spandantiwari/restrict_irv4_on_opset
branch
from
cfaba28 to
2112fcf
Compare
|
@houseroad - updated the PR based on your comment. |
|
@houseroad - there's only one failing job, which is unrelated and seems to be a stall ( |
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 509d963. |
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.
Currently,
keep_initializers_as_inputargument intorch.onnx.exportAPI can be used to choose whether to export an ONNX model with IR v3 or v4 semantics. Currently, the implementation does not check for which opset is being used for export. This is an issue because ONNX IR v4 is valid only for opset 9 and above (as listed here), and opset 8 or lower export withkeep_initializers_as_input=Falsewill create a illegal ONNX graph.This change fixes this by introducing a check on opset version when deciding whether to export ONNX IR v3 or v4.