-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Made onxxrutime an optional dependency #2722
base: main
Are you sure you want to change the base?
Made onxxrutime an optional dependency #2722
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
99e186a
to
4374b6c
Compare
@tazarov can you take a look here? |
Just chiming in in favor of this change - we're running into a problem where we'd like to install |
@mangin, you have a point about alpine. Arguably though, there is a build for onnx that runs on musl distros, although the pypi package isn't there, which makes it a tad more difficult to run onnx and therefore Chroma on alpine. That said, this change can be challenging to merge as it will break DX. Consider this: today you install chroma and doing something like: collection.add(ids=[...], documents=[...]) just works. With an optional onnxruntime, users will get an error as optional deps are not installed by default. Which creates a friction for the starting experience. We're also in the process of transitioning away onnx-based default EF, so that would be a natural point for merging the proposed solution in this PR. Either way, I think this needs a bit deeper consideration. So give me couple of days to do some experiments. |
Could you instead recommend |
Description of changes
Now onxxrutime is used to calculate default embedding. But onxxrutime doesn't support all operation systems (for example it doesn't work with alpine at all)
I think to replace onxxruntime to something else it's a big task, but at least we can allow to run chromadb without default embedding if onxxrutime is not present.
Summarize the changes made by this PR.