Skip to content

[ONNX] Upgrade onnx and onnxruntime#12729

Merged
driazati merged 1 commit intoapache:mainfrom
sfvaroglu:sevin/upgrade_onnx
Sep 20, 2022
Merged

[ONNX] Upgrade onnx and onnxruntime#12729
driazati merged 1 commit intoapache:mainfrom
sfvaroglu:sevin/upgrade_onnx

Conversation

@sfvaroglu
Copy link
Contributor

@sfvaroglu sfvaroglu commented Sep 8, 2022

Upgrade onnx and onnxruntime to latest

@sfvaroglu
Copy link
Contributor Author

@tvm-bot rerun

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Just a quick note that modifications in these scripts are not currently tested in CI, as the current CI workflow won't pickup the images created by a given PR to be effectively used to run the tests on that CI run.

There is ongoing work by @driazati to make that happen, but we're not there yet.

In the meantime I'd like to understand what is the motivation for the update?

@sfvaroglu
Copy link
Contributor Author

Thanks for the PR. Just a quick note that modifications in these scripts are not currently tested in CI, as the current CI workflow won't pickup the images created by a given PR to be effectively used to run the tests on that CI run.

There is ongoing work by @driazati to make that happen, but we're not there yet.

In the meantime I'd like to understand what is the motivation for the update?

Thanks for the quick comment. A couple of reasons... It's been a while since we updated onnx version in TVM and I'd like to understand the gap in the onnx frontend (given opset 17 is available now). I'd also like to add some onnx models to CI for model testing, and onnx model hub is available 1.11.0+.

What would be the best way to update CI images and run tests in this case? cc @driazati

@driazati
Copy link
Member

driazati commented Sep 8, 2022

Currently there's no good solution outside of running things locally, in the past for these kinds of changes it's been more of a merge it first, build new Docker images, then test the changes when trying to update the Docker images to the new ones. I will dust off #11893 today and try to get it merged, then that'll test everything in 1 PR before merging.

@sfvaroglu
Copy link
Contributor Author

Looks like ci_arm is failing to build onnxoptimizer even after upgrading cmake to 3.22? I'm not sure if it's required for arm and perhaps it could live in a separate *.sh file. Any thoughts? @driazati @leandron

@leandron
Copy link
Contributor

Looks like ci_arm is failing to build onnxoptimizer even after upgrading cmake to 3.22? I'm not sure if it's required for arm and perhaps it could live in a separate *.sh file. Any thoughts? @driazati @leandron

It is required as some work was recently done to enable Integration Tests that use onnx on ci_arm, and align versions of all frontend frameworks that work on both platforms. So when upgrading frontends that are already supported, I don't think we should keep separate scripts/versions unless there is no expected support in the target project.

@sfvaroglu
Copy link
Contributor Author

@tvm-bot rerun

@sfvaroglu sfvaroglu changed the title Upgrade onnx, onnxruntime, onnxoptimizer [ONNX] Upgrade onnx and onnxruntime Sep 19, 2022
@sfvaroglu
Copy link
Contributor Author

@tvm-bot rerun

@sfvaroglu
Copy link
Contributor Author

@driazati @leandron @AndrewZhaoLuo please review

@driazati driazati merged commit 18909a4 into apache:main Sep 20, 2022
@leandron
Copy link
Contributor

I'm seeing one test failing on GPU machines with some ONNX message, with Docker images to contain this update.

Can you have a look? https://ci.tlcpack.ai/job/docker-images-ci/job/docker-image-run-tests/231/testReport/junit/cython.tests.python.frontend.onnx/test_forward/Test___frontend__GPU_3_of_6___test_roi_align_cuda_/

xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
Upgrade onnx and onnxruntime to latest
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.

3 participants