-
Notifications
You must be signed in to change notification settings - Fork 46
Enable embedding caching on all vectorizers #320
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
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.
This looks great! Nice culmination of a lot of work. Had one non-blocking suggestion to consider, totally optional. 👍
|
||
try: | ||
# Efficient batch cache lookup | ||
cache_results = await self.cache.amget(texts=texts, model_name=self.model) |
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.
The whole amget / efficiency aspect of this work is nice. 🔥
|
||
model: str | ||
dtype: str = "float32" | ||
dims: Optional[int] = None | ||
dims: Annotated[Optional[int], Field(strict=True, gt=0)] = None | ||
cache: Optional[EmbeddingsCache] = Field(default=None) |
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.
Passing a cache object in to get caching feels elegant
async def _aembed(self, text: str, **kwargs) -> List[float]: | ||
"""Asynchronously generate a vector embedding for a single text. | ||
|
||
Note: This implementation falls back to the synchronous version as |
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 could be worth logging when these methods are called.
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.
done!
Adds support to the `BaseVectorizer` class to have an optional `EmbeddingsCache` attached. - Refactored the subclass vectorizers to implement private embed methods and then let the base class handle the cache wrapper logic. - Fixed some circular imports. - Fixed async client handling in the cache subclasses (caught during testing). - Handle some typing checks and pydantic stuff related to private attrs and custom attrs. TODO in a separate PR: - Add embeddings caching to our testing suite (CI/CD speed up??) - Add embeddings caching to our SemanticRouter
Adds support to the `BaseVectorizer` class to have an optional `EmbeddingsCache` attached. - Refactored the subclass vectorizers to implement private embed methods and then let the base class handle the cache wrapper logic. - Fixed some circular imports. - Fixed async client handling in the cache subclasses (caught during testing). - Handle some typing checks and pydantic stuff related to private attrs and custom attrs. TODO in a separate PR: - Add embeddings caching to our testing suite (CI/CD speed up??) - Add embeddings caching to our SemanticRouter
Adds support to the
BaseVectorizer
class to have an optionalEmbeddingsCache
attached.TODO in a separate PR: