-
Notifications
You must be signed in to change notification settings - Fork 72
[IR] Create a shape inference pass using onnx shape inference #2117
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
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 a new shape inference pass that leverages onnx.shape_inference to automatically determine tensor shapes in the model. It adds the implementation for the pass, corresponding unit tests, and a minor cleanup in the model saving routine.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
onnxscript/ir/passes/common/shape_inference.py | Implements the new shape inference pass using onnx.shape_inference. |
onnxscript/ir/passes/common/shape_inference_test.py | Adds tests to validate that the pass correctly infers shapes and data types. |
onnxscript/ir/_io.py | Updates the initializer tensor extraction for consistency in the model saving function. |
Comments suppressed due to low confidence (1)
onnxscript/ir/passes/common/shape_inference_test.py:24
- [nitpick] The node is created with an empty string as its name; consider providing a descriptive name to improve debugging and traceability.
add_node = ir.Node("", "Add", inputs=inputs)
❌ 4 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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 a new shape inference pass that temporarily removes large initializers to reduce model size for ONNX shape inference, and adds corresponding tests to ensure the pass functions as expected.
- Implements a shape inference pass in onnxscript/ir/passes/common/shape_inference.py.
- Adds tests to verify correct inference and the restoration of initializers in onnxscript/ir/passes/common/shape_inference_test.py.
- Updates documentation in _pass_infra.py and refactors initializer value handling in _io.py to improve clarity.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
onnxscript/ir/passes/common/shape_inference_test.py | Added tests verifying shape and type inference on node outputs. |
onnxscript/ir/passes/common/shape_inference.py | Implementation of the shape inference pass using onnx.shape_inference. |
onnxscript/ir/passes/_pass_infra.py | Updated docstring to clarify the meaning of the "modified" attribute. |
onnxscript/ir/_io.py | Simplified variable usage for initializing tensor backup. |
Comments suppressed due to low confidence (1)
onnxscript/ir/passes/common/shape_inference_test.py:38
- Consider adding a test case where an initializer's const_value exceeds _BIG_TENSOR_SIZE_LIMIT to validate that large initializers are temporarily removed and then properly restored after shape inference.
# Perform shape inference
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 a new shape inference pass leveraging onnx.shape_inference to support large models by temporarily removing oversized initializers.
- Added the ShapeInferencePass class and an infer_shapes function in onnxscript/ir/passes/common/shape_inference.py
- Introduced tests to verify the shape inference behavior and initializer restoration in onnxscript/ir/passes/common/shape_inference_test.py
- Made minor documentation and code adjustments in onnxscript/ir/passes/_pass_infra.py and onnxscript/ir/_io.py to improve clarity and consistency
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
onnxscript/ir/passes/common/shape_inference.py | New shape inference pass implementation |
onnxscript/ir/passes/common/shape_inference_test.py | Added tests for the shape inference pass |
onnxscript/ir/passes/_pass_infra.py | Updated PassResult docstring |
onnxscript/ir/_io.py | Refactored initializer value extraction for clarity |
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, thanks!
…oft#2117) It handles large models by removing the initializers before sending the model to onnx shape inference.
…oft#2117) It handles large models by removing the initializers before sending the model to onnx shape inference.
…oft#2117) It handles large models by removing the initializers before sending the model to onnx shape inference.
It handles large models by removing the initializers before sending the model to onnx shape inference.