-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add Support for BAAI/bge-m3 #197
base: main
Are you sure you want to change the base?
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
This one has successfully passed all the tests conducted locally !Additionally I have updated onnx_embedding.py file as it was generating this error :
fastembed/tests/test_text_onnx_embeddings.py:48: fastembed/fastembed/text/text_embedding.py:89: in embed self = <onnxruntime.capi.onnxruntime_inference_collection.InferenceSession object at 0x10217eb90>
E onnxruntime.capi.onnxruntime_pybind11_state.InvalidArgument: [ONNXRuntimeError] : 2 : INVALID_ARGUMENT : Invalid input name: token_type_ids ../.pyenv/versions/3.10.10/lib/python3.10/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py:220: InvalidArgument |
Fixes #107 |
CI passed. Awesome. I see you've contributed the ONNX weights to the BAAI/bge-m3 HF model repo. https://huggingface.co/BAAI/bge-m3 We can now use the original repo as the source? |
Done ! |
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.
🚀🚀
As mentioned in the issue — we'd want to support BGE-M3 with not just dense vectors, but all 3: SPLADE and ColBERT. It's not very useful as just a dense model and adds to the confusion. Would appreciate it if you can add a PR with a new category of models e.g. |
@NirantK , I found a |
however , when I tested it with my bge-m3 onnx model repo for ColBERT vectors .This was the output:https://colab.research.google.com/drive/1SNhC089nCiLixhQVGT-VmYQeOu7WHdba?usp=sharing |
Could we test |
@Anush008 could you please review this ? |
We'll have to wait for Nirant to take a look at those. |
@NirantK could you please review this ? |
First off, thanks a ton for the love and contributions @Ya-shh — really appreciate. Thank you for the investigations and looking around for more ONNX exports e.g. aapot, ddmitov, official BGE-M3 — none of them do all 3 as far as I can tell Given the time and effort you've put across PRs, I believe it'd be helpful to expand how I'm thinking about this right now: BGE-M3
Tests and NotebooksThe notebooks are a bit difficult to make sense of:
Suggestions
|
@NirantK Thank you immensely for your kind words and recognition. Your appreciation means a lot to me! Your detailed feedback is incredibly helpful in guiding my efforts further. I'm deeply grateful for the opportunity to expand on my contributions and improve the ONNX exports. I wholeheartedly agree with your suggestions and will prioritize them accordingly. ⎯Regarding your thoughts on As for the notebooks, I understand the importance of clarity and consistency. Rest assured, I'll work diligently to ensure they provide comprehensive comparisons and tests for all three vectors. Your suggestion to begin with a Lastly, I appreciate your reminder to close existing PRs before opening new ones. Streamlining our workflow will undoubtedly lead to greater efficiency and effectiveness. I'll make it a priority to address any outstanding tasks promptly. Once again, thank you for your unwavering support and encouragement. I'm too excited to delve deeper into the ONNX export investigation and share my findings with you through an example notebook. Your review and feedback are eagerly anticipated! |
Canonical vector values: https://colab.research.google.com/drive/1OTZcLxWWAmJqV1ZDS57W5x6zJPRzku4n?usp=sharing
Onnx model: https://huggingface.co/yashvardhan7/bge-m3-onnx/tree/main
Contributed ONNX weights to the BAAI/bge-m3 HF model repo :https://huggingface.co/BAAI/bge-m3/tree/main/onnx