-
Notifications
You must be signed in to change notification settings - Fork 22.1k
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
[ONNX] Export split with list of sizes #33161
Conversation
💊 CircleCI build failures summary and remediationsAs of commit 6f8ba4f:
Detailed failure analysisOne may explore the probable reasons each build failed interactively on the Dr. CI website. 🕵️ 1 new failure recognized by patternsThe following build failures do not appear to be due to upstream breakage: pytorch_xla_linux_xenial_py3_6_clang7_build (1/1)Step: "Build" (full log | pattern match details)
|
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.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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.
LGTM, some minor comments regarding comments.
@@ -661,6 +661,45 @@ static void fuseUnbindListUnpack(Block *b) { | |||
} | |||
} | |||
|
|||
// Traced Split with list of sizes is being converted to ONNX as SplitToSequence + SequenceAt. | |||
// Example IR |
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.
Nit: we should add ONNX IR version check.
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.
Let's land it now. But we should have a check on ONNX IR version. Old IR doesn't support sequence I think.
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.
@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 9823662. |
Summary: Exporting Split with a dynamic list of split_sizes is not supported. This PR enables export using onnx SplitToSequence + SequenceAt Pull Request resolved: pytorch#33161 Reviewed By: hl475 Differential Revision: D19860152 Pulled By: houseroad fbshipit-source-id: 300afedc22b01923efb23acd1a3627aa146bb251
Exporting Split with a dynamic list of split_sizes is not supported.
This PR enables export using onnx SplitToSequence + SequenceAt