Refactoring of internal structure #105
Conversation
HF sources were tried first. If they failed or were not present, fallback to GCS. |
| DefaultEmbedding = TextEmbedding | ||
| FlagEmbedding = TextEmbedding |
There was a problem hiding this comment.
I think this retains enough backward compatibility for most users.
There was a problem hiding this comment.
that was our thought as well
| JinaOnnxEmbedding, | ||
| ] | ||
|
|
||
| @classmethod |
There was a problem hiding this comment.
How do we handle the case of models which we use for both dense and sparse? E.g. BGE-M3
There was a problem hiding this comment.
we would need to implement both TextEmbedding and SparseEmbedding(tba) classes for those models. I am strictly against changing the function output format depending on the model name
fastembed/text/onnx_models.py
Outdated
| @@ -0,0 +1,133 @@ | |||
| supported_flag_models = [ | |||
There was a problem hiding this comment.
I'd prefer to have a flat models.json or similar config-like file, instead of 3 different lists.
If we want to separate the models into different classes and files, we should move this completely to those corresponding files — that way the places to update in any new model addition are:
- New class in a file
- Embedding Registry
- Tests
That's it
There was a problem hiding this comment.
I am ok with moving lists to the corresponding implementation. But having a single list doesn't work. It creates very ugly hacks as with the "exclude" param of the list models method.
But what about multiple GCS and multiple HF courses with different names? |
I did a manual run, but it would be nice to mention a setup for the hook in dev docs |
|
Added 2 new models to main @generall — let's have those here as well, and then we're good to merge this from my end! |
Co-authored-by: George Panchuk <[email protected]>
generall
force-pushed
the
refactoring-off-everything
branch
from
c67ebb9 to
7883fa3
Compare
fastembed/text/e5_onnx_embedding.py
Outdated
| } | ||
| }, | ||
| { | ||
| "model": "xenova/paraphrase-multilingual-mpnet-base-v2", |
There was a problem hiding this comment.
| "model": "xenova/paraphrase-multilingual-mpnet-base-v2", | |
| "model": "sentence-transformers/paraphrase-multilingual-mpnet-base-v2", |
This is the model name. I missed this in the PR from Anush. Apologies for the confusion.
fastembed/text/e5_onnx_embedding.py
Outdated
| { | ||
| "model": "xenova/multilingual-e5-large-quantized", | ||
| "dim": 1024, | ||
| "description": "Multilingual model. Recommended for non-English languages", | ||
| "size_in_GB": 2.24, | ||
| "sources": { | ||
| "hf": "xenova/multilingual-e5-large", | ||
| } | ||
| }, |
There was a problem hiding this comment.
| { | |
| "model": "xenova/multilingual-e5-large-quantized", | |
| "dim": 1024, | |
| "description": "Multilingual model. Recommended for non-English languages", | |
| "size_in_GB": 2.24, | |
| "sources": { | |
| "hf": "xenova/multilingual-e5-large", | |
| } | |
| }, |
We can delete this completely, since the model is covered by Qdrant now.
Why?
excludeparam of the model list, multiple sources for the same model (how it was supposed to work?), remove the gcp fallback wrapper