Add fastembed integration #210
Conversation
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…rom fastembed instead of fastvector * fix(qdrant_client.py): remove unnecessary print statement
NirantK
changed the title
* feat(README.md): add section for Fast Embeddings + Simpler API * fix(README.md): fix formatting of code block and update code example for Qdrant Client usage *
NirantK
changed the title
NirantK
changed the title
|
Overall it would be nice to have tests for this integration |
* feat(qdrant_client.py): add support for adding and querying documents with fastembed installed * test(qdrant_client.py): add tests for adding and querying documents with and without fastembed installed
…ntAPIExtensions for upsert_docs and search_docs methods
* feat(qdrant_client.py): add return type hint to QdrantClient.search_docs method
…in qdrant_client.py
…ixture * test(test_qdrant_client.py): add test for client_close function
* chore(test_qdrant_client.py): reformat import statements for better readability
… DefaultEmbedding instead of FlagEmbedding * refactor(qdrant_client.py): refactor code to remove unnecessary loop
* feat(qdrant_client.py): add support for search parameters in search method * refactor(qdrant_client.py): refactor indexing logic to handle embeddings correctly
…s in collection * test(test_fast_embed.py): remove unused code * test(test_fast_embed.py): add TODO comment for future assertions
…dencies * feat(pyproject.toml): add fastembed dependency to fastembed group
…rantClient constructor
* fix(test_fast_embed.py): add default values for test_no_install parameters * fix(test_fast_embed.py): skip test if FastEmbed is installed
|
haven't yet looked deep into the PR, but all the packages inside if it is actually required to modify them, https://github.com/qdrant/pydantic_openapi_v3 should be upgraded first |
* feat(qdrant_client.py): add QueryResponse class
…or QueryResponse class
|
Hi @NirantK, I made some changes into PR:
I am going to suggest some changes in |
qdrant_client/qdrant_fastembed.py
Outdated
| return self.search( | ||
| collection_name=collection_name, | ||
| query_vector=query_vector, | ||
| query_filter=query_filter, | ||
| search_params=search_params, | ||
| limit=limit, | ||
| offset=offset, | ||
| with_payload=with_payload, | ||
| with_vectors=with_vectors, | ||
| score_threshold=score_threshold, | ||
| **kwargs, | ||
| ) |
There was a problem hiding this comment.
A major point of API improvement is a simpler Response type, this is why I created QueryResponse and simplified the ScoredPoint and discarded a lot of information.
Would it possible to retain that here?
There was a problem hiding this comment.
Could you please elaborate how plain dict it better than a type?
Previous version converted QueryResponse into dict before returning it, in fact QueryResponse was only used internally and gave no info to the user about the response structure.
If you propose to replace return type ScoredPoint -> QueryResponse that I can agree on
qdrant_client/qdrant_fastembed.py
Outdated
| with_payload: Union[bool, Sequence[str], models.PayloadSelector] = True, | ||
| with_vectors: Union[bool, Sequence[str]] = True, | ||
| score_threshold: Optional[float] = None, | ||
| embedding_model: str = DEFAULT_EMBEDDING_MODEL, |
There was a problem hiding this comment.
The query endpoint should NOT accept embedding_model!
This can lead to oversight e.g. folks trying to use different models for add and query. This means we've to persist the state across multiple client sessions. One way to do so would be to have that as part of the payload itself perhaps?
qdrant_client/qdrant_fastembed.py
Outdated
| offset: int = 0, | ||
| with_payload: Union[bool, Sequence[str], models.PayloadSelector] = True, | ||
| with_vectors: Union[bool, Sequence[str]] = True, |
There was a problem hiding this comment.
Do these added params makes the function more consistent with the client at the cost of simplicity?
We'd want to have only collection_name, limit and query_text ideally!
Anyone who cares about these params enough to read the docs and understand those — should use the APIs which our client already has!
There was a problem hiding this comment.
We can hide those under **kwargs
qdrant_client/qdrant_fastembed.py
Outdated
| def query_batch( | ||
| self, | ||
| collection_name: str, | ||
| query_texts: List[str], | ||
| query_filter: Optional[models.Filter] = None, | ||
| search_params: Optional[models.SearchParams] = None, | ||
| limit: int = 10, | ||
| offset: int = 0, | ||
| with_payload: Union[bool, Sequence[str], models.PayloadSelector] = True, | ||
| with_vectors: Union[bool, Sequence[str]] = True, | ||
| score_threshold: Optional[float] = None, | ||
| embedding_model: str = DEFAULT_EMBEDDING_MODEL, | ||
| **kwargs: Any, | ||
| ) -> List[List[types.ScoredPoint]]: |
There was a problem hiding this comment.
The most notable difference is that this accepts a List[str] as queries — this is not worth learning for most MLE!
Can we make a call from the query endpoint to support this behaviour instead? Thereby removing the need for engineer to learn this — and keeping it as a separate function at the same time?
|
@generall merge when you're satisfied? |
This PR adds two new functions
addandqueryand a new return object typeQueryResponse. This makes it a lot easier for folks coming from NLP and looking to use SoTA Embedding (beating OpenAI) but several times faster.Improvements:
optimum— this makes the model ~2x faster compared to the PyTorch runtime on CPU