-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[onnx] Update onnx to 1.16.2 and fix downstream #40067
[onnx] Update onnx to 1.16.2 and fix downstream #40067
Conversation
…Monica/update_onnx
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.
Hey @MonicaLiu0311, Sorry, I didn't realize you were also editing openvino
here when I merged #41077
Please resolve the merge conflicts and mark "ready for review"
+ find_package(CUDNN REQUIRED) | ||
set(CUDNN_FRONTEND_INCLUDE_DIR ${CMAKE_CURRENT_LIST_DIR}/../third_party/cudnn_frontend/include) |
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.
vendored dependency?
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.
This patch line actually comes from: 36246/files , let's ask @Neumann-A to get the answer. I just organized the patches for Dependencies.cmake
here because they are too scattered and a bit confusing.
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.
I agree this looks broken, but it isn't a change from the status quo, since it just applies more-fixes.patch on top of this one.
# as well as ONNX Optimizer targets for other cmake libraries to use. | ||
|
||
+include(CMakeFindDependencyMacro) | ||
+find_dependency(onnx CONFIG) |
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.
Also needs find_dependency(protobuf CONFIG)
, and probably others. It's technically benign here because onnx itself drags in protobuf.
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.
I don't like everything here but everything I don't like was already here, so I think we should take this update. @JavierMatosD are there any remaining over-your-dead-body issues?
Ah, I see above he just wanted a merge, so I'm going to merge. Thanks @MonicaLiu0311 ! |
Could you please also add ONNX 1.17.0 ? |
Fixing by #42942. |
Fixes #39891.
onnx
Update to 1.16.2.
The usage test passed on
x64-windows
(header files found):onnx-optimizer
Update to 0.3.19. Fix dependency
onnx
and remove the outdated patchremove-outdate-headers.patch
.The usage test passed on
x64-windows
(header files found):openvino
Fix default-feature
onnx
:libtorch
Fix dependency
onnx
and clean up confusion regardingDependencies.cmake
patches.The "supports" clause reflects platforms that may be fixed by this new version.Any fixed CI baseline entries are removed from that file../vcpkg x-add-version --all
and committing the result.