Skip to content
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

Ensure ONNX model generation considers opset version #682

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kazssym
Copy link
Contributor

@kazssym kazssym commented Mar 27, 2024

This PR modifies the gen_processing_models function in onnxruntime_extensions/cvt.py to explicitly include the opset_version argument when calling make_onnx_model for both pre-processing and post-processing graphs.

Previously, the function only generated models if pre_g or post_g existed, without specifying the opset version. This update ensures that generated ONNX models adhere to the desired opset level, improving compatibility and deployment options.

This commit modifies the `gen_processing_models` function in `onnxruntime_extensions/cvt.py` to explicitly include the `opset_version` argument when calling `make_onnx_model` for both pre-processing and post-processing graphs.

Previously, the function only generated models if `pre_g` or `post_g` existed, without specifying the opset version. This update ensures that generated ONNX models adhere to the desired opset level, improving compatibility and deployment options.
@kazssym kazssym marked this pull request as ready for review March 27, 2024 13:31
@kazssym kazssym requested a review from a team as a code owner March 27, 2024 13:31
@wenbingl
Copy link
Member

/azp run onnxruntime-extensions.CI

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

return make_onnx_model(pre_g) if pre_g else None, \
make_onnx_model(post_g) if post_g else None
return make_onnx_model(pre_g, opset_version=opset) if pre_g else None, \
make_onnx_model(post_g, opset_version=opset) if post_g else None
Copy link
Member

@wenbingl wenbingl Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like it also need some fixing for the case of opset==None, check CI pipeline result for more details. Would you like to do that, or wait for my help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants