feat(vectorisers): Add FastEmbedVectoriser implementation of VectoriserBase and add fast embed dependency group - #224
Conversation
|
Have made the relevant changes to merge it in. I am now happy with it being merged in, needs to have one more person review it though. |
…show the supported models
There was a problem hiding this comment.
This looks great. I have updated it with some changes to make it work well with the rest of the package. I have also added a explicit model path parameter as FastEmbed requires both:
- a model name
- a model path
To load models locally and I thought having it in kwargs was too confuesing
There was a problem hiding this comment.
I really like this - its fast, and I've tested loading different models and then also using the FastEmbedVectoriser in the general_worflow.ipynb notebook as the tests suggest and it all works well.
Happy to approve as is but I added a few comments for potential changes.
One additional thought is - in the similar HuggingFaceVectoriser class we have a revision argument, but we don't seem to have that in this new FastEmbedVectoriser class. Should we/could we add that?
| def __init__( | ||
| self, | ||
| model_name: str, | ||
| specific_model_path: str | None = None, |
There was a problem hiding this comment.
Consider removing this parameter and then user can pass it as part of the kwarg to the constructor?
Looking at how this is handled in the HuggingFaceVectoriser class (HuggingFace also has its own way of local cache checking for models under the hood), we don't have a special parameter in that constructor.
So mirroring that might be good for consistency
There was a problem hiding this comment.
@jamie-ons added this so not sure of the rationale, don't mind either way.
There was a problem hiding this comment.
Will confirm when @jamie-ons returns from leave, but I think this was to allow someone to specify whether to use ONNX format weights on a model card if there's multiple options available
There was a problem hiding this comment.
The HuggingFaceVectoriser allows a user to put in a path to a local model as the model_name, however in fast embed this is not allowed.
I therefore decided to add in this specific model path as a key use of the FastEmbedVectoriser is to run models in enviroments where resources may be lower.
This could often mean that ability to download large packages (torch) or files (model weights) may be restricted. I also don't think the FastEmbed documentation is that clear about how to run models from a local download and so thought adding the argument would save them time in researching how to do it.
I will update the docstrings to make this clearer.
There was a problem hiding this comment.
Do we update the changelog with every PR on the package? I thought we just did one changelog update per release, and looked back at the merged PRs when writing it?
There was a problem hiding this comment.
Soz haven't had to think about CHANGELOGs in a while as that's done for me on scanner ;)
There was a problem hiding this comment.
Will have a look later today
frayle-ons
left a comment
There was a problem hiding this comment.
Looks good to me! But unit tests?
Sure il add some |
✨ Summary
Add a new
FastEmbedVectoriserto ClassifAI as a lightweight local embedding backend that avoidstorchandtransformersat runtime, alongside the newfastembedoptional dependency group, tests, and documentation updates.I have run a manual check in
general_workflow_demo.ipynbwithFastEmbedVectoriserand it runs end to end.I haven't done a performance benchmark, however happy to do so with guidance on how this is done for
classifai.I have not included the model caching or normalisation logic in
survey-assist-embed-coreto remain consistent with other implementations ofVectoriserBase.📜 Changes Introduced
FastEmbedVectoriseras a newVectoriserBaseimplementation usingfastembed.TextEmbeddingwith ClassifAI-style error handling.fastembedoptional dependency group and included it in theallextra.classifai[fastembed]install path.✅ Checklist
terraform fmt&terraform validate)🔍 How to Test
uv sync --all-extrasuv run pytest -quv run --extra fastembed python -c "from classifai.vectorisers import FastEmbedVectoriser; vectoriser = FastEmbedVectoriser(model_name='sentence-transformers/all-MiniLM-L6-v2'); print(vectoriser.transform('hello world').shape)"DEMO/general_workflow_demo.ipynband instantiateFastEmbedVectoriserthere. This has been manually validated on this branch.