-
Notifications
You must be signed in to change notification settings - Fork 72
[pass] Create version converter pass #2214
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 adds a version conversion pass that uses the onnxscript version converter with an option to fall back to the onnx C API converter when necessary.
- Renamed constant CURRENT_MAX_ONNX_OPSET to SUPPORTED_MAX_ONNX_OPSET and added SUPPORTED_MIN_ONNX_OPSET.
- Introduced a new function version_supported and modified the ConvertVersionPass to incorporate a fallback mechanism.
- Updated the convert_version API to accept a fallback parameter.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
onnxscript/version_converter/_version_converter.py | Updated op set constants, added version_supported function with a potentially incorrect inequality chain. |
onnxscript/version_converter/init.py | Modified convert_version API to include a fallback parameter with an inconsistent default value compared to its docstring. |
onnxscript/ir/passes/common/version_converter.py | Added ConvertVersionPass that leverages the updated version converter and fallback mechanism. |
Co-authored-by: Copilot <[email protected]>
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Co-authored-by: Shubham Bhokare <[email protected]>
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 version conversion pass that utilizes the onnxscript version converter with an optional fallback to the ONNX C API when a target version is unsupported. Key changes include test coverage for version conversion, updates to the version converter implementation (including renaming constants and adding a version_supported helper), and integration of the new pass into the PyTorch API conversion flow.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/version_converter/version_conversion_test.py | Added tests ensuring the conversion behavior when fallback is enabled. |
onnxscript/version_converter/_version_converter_test.py | Fixed a typo in the test class name from "ApapterCoverageTest" to "AdapterCoverageTest". |
onnxscript/version_converter/_version_converter.py | Updated constants and implemented the version_supported helper; improved warning logic. |
onnxscript/version_converter/init.py | Introduced the ConvertVersionPass and updated the convert_version function signature. |
onnxscript/_framework_apis/torch_2_6.py | Modified the conversion flow to use the new pass, with a warning for unsupported opset < 18. |
Comments suppressed due to low confidence (1)
onnxscript/version_converter/_version_converter.py:137
- [nitpick] Consider renaming the loop variable 'input' to avoid shadowing the built-in function 'input'.
for input in converted_model.graph.inputs:
Use both the onnxscript version converter and optionally fall back to the onnx version converter if the target version is unsupported.
Created
version_supported
helper function for users to check if a target version is supported by the onnxscript version converter.Use the converter in pytorch apis.