Create edges with arg positons correctly accounting for non-existing args #18462
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
non-existing non-trainling args.
Contributor
pranavsharma
left a comment
There was a problem hiding this comment.
Do you've a test case?
Make sure we do not skip on the non-existing arguments in the middle, because shape inferece relies on their proper position. This also affects the argument position in the Edges that must be properly rebuilt each time If node branch is inlined. Make sure that when we rename Defs in subgraphs, new renamed defs are created in those subgraphs instead of pointing to outer scope defs. Add unit test.
gramalingam
approved these changes
Nov 20, 2023
kleiti
pushed a commit
to kleiti/onnxruntime
that referenced
this pull request
Mar 22, 2024
…args (microsoft#18462) ### Description Truncate traling non-existing arguments. Make sure we do not skip on the non-existing arguments in the middle, because shape inferece relies on their proper position. This also affects the argument position in the Edges that must be properly rebuilt each time If node branch is inlined. Make sure that when we rename Defs in subgraphs, new renamed defs are created in those subgraphs instead of pointing to outer scope defs. Add unit test. ### Motivation and Context This is a follow up for microsoft#18105 Currently, the non-trailing arguments are simply ignored and the edges are created with potentially incorrect positions.
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.
Description
Truncate traling non-existing arguments.
Make sure we do not skip on the non-existing arguments in the middle,
because shape inferece relies on their proper position.
This also affects the argument position in the Edges that must be properly rebuilt
each time If node branch is inlined.
Make sure that when we rename Defs in subgraphs, new renamed defs are created in those subgraphs
instead of pointing to outer scope defs.
Add unit test.
Motivation and Context
This is a follow up for #18105
Currently, the non-trailing arguments are simply ignored and the edges are created
with potentially incorrect positions.