Skip to content

Conversation

@justinchuby
Copy link
Member

@justinchuby justinchuby commented Dec 12, 2025

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

  • Added a new file, output_fix.py, implementing OutputFixPass. 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.

@justinchuby justinchuby requested review from a team and titaiwangms as code owners December 12, 2025 23:48
@justinchuby justinchuby changed the title Add IdentityFixPass to fix invalid graphs with direct input-output connections Add OutputFixPass to fix invalid graphs with direct input-output connections Dec 12, 2025
@justinchuby justinchuby requested a review from Copilot December 12, 2025 23:49
Signed-off-by: Justin Chu <[email protected]>
Copy link
Contributor

Copilot AI left a 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 OutputFixPass to 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
Copy link

codecov bot commented Dec 12, 2025

Codecov Report

❌ Patch coverage is 95.08197% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.66%. Comparing base (7283e95) to head (b73f540).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/onnx_ir/passes/common/output_fix.py 94.91% 1 Missing and 2 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

justinchuby and others added 5 commits December 12, 2025 15:54
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
@justinchuby justinchuby changed the title Add OutputFixPass to fix invalid graphs with direct input-output connections Add OutputFixPass to fix invalid graph outputs Dec 13, 2025
@justinchuby justinchuby added this to the 0.1.13 milestone Dec 13, 2025
@justinchuby justinchuby requested a review from Copilot December 13, 2025 00:38
@justinchuby justinchuby requested a review from xadupre December 13, 2025 00:41
Copy link
Contributor

Copilot AI left a 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.

Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
@justinchuby justinchuby linked an issue Dec 16, 2025 that may be closed by this pull request
@justinchuby justinchuby self-assigned this Dec 16, 2025
@justinchuby justinchuby requested a review from Copilot December 16, 2025 21:12
Copy link
Contributor

Copilot AI left a 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.

Signed-off-by: Justin Chu <[email protected]>
@justinchuby justinchuby enabled auto-merge (squash) December 17, 2025 01:05
@justinchuby justinchuby disabled auto-merge December 17, 2025 01:40
@justinchuby justinchuby enabled auto-merge (squash) December 17, 2025 01:42
@justinchuby justinchuby merged commit 26b5ed3 into main Dec 17, 2025
22 checks passed
@justinchuby justinchuby deleted the copilot/add-identity-node-pass branch December 17, 2025 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Passes] Add identity fix

3 participants