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

Share downloaded ort with ort-extensions #913

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

baijumeswani
Copy link
Contributor

So far, ort-extensions and ort both download ort to build.

With this pull-request, ort-genai can share the downloaded ort header dir with ort-extensions.

@skyline75489
Copy link
Contributor

From what I can tell, the exact version of ORT downloaded by ort-extension is NOT the same as the one download by GenAI, is this going to be a problem?

@baijumeswani
Copy link
Contributor Author

From what I can tell, the exact version of ORT downloaded by ort-extension is NOT the same as the one download by GenAI, is this going to be a problem?

It should not matter. ort lib api is backwards compatible and ort-extensions only uses header file of ort for building. So, if any problems, it can be caught at compile time.

@skyline75489
Copy link
Contributor

skyline75489 commented Sep 24, 2024

Is it possible to prevent onnxruntime-extension from downloading its own ORT at all? Seems that ONNXRUNTIME_PKG_DIR would help. We probably need more tweak for Android and iOS, but at least it's a start.

See: https://github.com/microsoft/onnxruntime-extensions/blob/6b94f4d7a597f0199f3e44d020d601d408605736/cmake/ext_ortlib.cmake#L4

@skyline75489
Copy link
Contributor

Oh I was mistaken. Setting ONNXRUNTIME_INCLUDE_DIR would also prevent ortx from downloading ORT. Nice!

See: https://github.com/microsoft/onnxruntime-extensions/blob/6b94f4d7a597f0199f3e44d020d601d408605736/CMakeLists.txt#L297

@baijumeswani baijumeswani merged commit 9ec7c5c into main Sep 24, 2024
10 of 13 checks passed
@baijumeswani baijumeswani deleted the baijumeswani/use-common-ort branch September 24, 2024 05:56
@baijumeswani
Copy link
Contributor Author

Thanks for the review :)

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