-
Notifications
You must be signed in to change notification settings - Fork 17
Add OutputFixPass to fix invalid graph outputs #269
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
Conversation
…nnections Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
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.
Pull request overview
This PR introduces OutputFixPass, a new transformation pass that ensures ONNX model validity by inserting Identity nodes when graph inputs are directly used as graph outputs. This addresses an ONNX specification requirement where direct input-to-output connections are not permitted.
Key Changes:
- Implements
OutputFixPassto detect and fix direct input-to-output connections by inserting Identity nodes - Handles main graphs, subgraphs, and function bodies recursively
- Preserves output metadata (name, shape, type, doc_string, and metadata_props) when inserting Identity nodes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/onnx_ir/passes/common/output_fix.py |
Implements the new OutputFixPass with logic to insert Identity nodes between direct input-output connections |
src/onnx_ir/passes/common/output_fix_test.py |
Comprehensive test suite covering various scenarios including subgraphs, functions, and edge cases |
src/onnx_ir/passes/common/__init__.py |
Registers and exports OutputFixPass in the common passes module |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #269 +/- ##
==========================================
+ Coverage 77.28% 77.66% +0.38%
==========================================
Files 40 41 +1
Lines 5097 5157 +60
Branches 1027 1039 +12
==========================================
+ Hits 3939 4005 +66
+ Misses 860 851 -9
- Partials 298 301 +3 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
This pull request introduces a new pass,
OutputFixPass, to the ONNX IR transformation pipeline. The main purpose of this pass is to ensure that the model's outputs conform to ONNX specifications by inserting Identity nodes where necessary. This helps to fix cases where graph inputs are directly used as outputs or where the same value is used multiple times as an output, both of which are not allowed by ONNX.The most important changes include:
New Pass Implementation
output_fix.py, implementingOutputFixPass. This pass automatically inserts Identity nodes in two scenarios: when a graph input is directly used as an output, and when a value is used multiple times as an output. The pass applies to both the main graph and all subgraphs/functions.AI use
I used copilot to create the tests and some of the logic.