-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(embeddings): use stdlib array type for improved performance #2060
Conversation
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.
Thanks for this!
I think you'll need to rebase against next
to fix the test errors in this PR fyi
82d3156
to
7a6517d
Compare
I've rebased to next. Regarding the defaults, it still has the same fallback to inspect the result. Adding the parameter is currently a side-effect of having numpy installed in the same venv, no? |
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.
Very nice, thank you!
Implements #2059
Changes the default embedding request to use base64 responses (unless overriden).
Changes the default response parser to use the builtin array type to convert from the base64 to a float32 compact array then directly into a list.
4x faster than the current default (see issue with benchmark)
20% faster than with numpy, although I've left the numpy code in. The numpy code could be removed completely.