-
Notifications
You must be signed in to change notification settings - Fork 238
[5694695][AutoCast] Preserve outer scope variable types in subgraphs #717
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
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.
@jricker2 do you mean that replacing lines 317~320 with
vi.type.ClearField("tensor_type")solves the ORT with CUDA EP issue that you're observing?Uh oh!
There was an error while loading. Please reload this page.
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.
Yes exactly, just commenting those lines out actually resolves the issue (lines 321-326, non-subgraph block)
I also found that if I cleared the value_info from the original fp32 graph then ran shape inference to re-populate it, then fed this graph into the model optimizer I had no issues. It seems to be an issue with how the graph was produced (exported by torch I believe). I don't own the creation/export of the original torch model, and the workaround is straightforward so I decided to not look much further into it (also because I found debugging value_info related issue to be very time consuming, tools like DL designer are not very helpful for this).
Anyhow, from what I can tell having a clear value_info before shape inference is the best way to go as opposed to pre-filling with generic shape/type.
edit: as to not take anything away from this PR - there are no subgraphs in the model I had this issue with, I just saw that this touched similar parts of code as I was looking into so figured I ask. Don't want to hold this up.
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.
Thanks for the confirmation. Please let us know when you have an IP-free repro so we can check if the suggested WAR is enough to fix this issue. Thanks.