Skip to content

Add Support for intfloat/multilingual-e5-large-instruct :Fixes #140 #181

Closed
Ya-shh wants to merge 2 commits intoqdrant:mainfrom
Ya-shh:e5-large-instruct
Closed

Add Support for intfloat/multilingual-e5-large-instruct :Fixes #140 #181
Ya-shh wants to merge 2 commits intoqdrant:mainfrom
Ya-shh:e5-large-instruct

Conversation

@Ya-shh
Copy link
Copy Markdown

@Ya-shh Ya-shh commented Apr 1, 2024

Description:

This pull request addresses issue #140 by integrating the intfloat/multilingual-e5-large-instruct model from Hugging Face into the repo. This update extends capabilities, enabling it to support more diverse language embeddings and accommodate various numerical data types seamlessly.

Changes:

•Model Support: The intfloat/multilingual-e5-large-instruct model is now supported, expanding our project's multilingual
processing capabilities.

•Documentation Update: Updated the Supported_Models.ipynb notebook to include documentation and usage examples for the new model.

•Configuration Update: Modified e5_onnx_embedding.py to include configuration settings specific to the intfloat/multilingual-e5-large-instruct model, ensuring optimal performance.

•Testing: Enhanced test_text_onnx_embeddings.py with new test cases designed to validate the output embeddings of the intfloat/multilingual-e5-large-instruct model against predefined documents.

•CANONICAL_VECTOR values : Here is a reference Colab Notebook

Onnx file :

HuggingFaceHub:
https://huggingface.co/yashvardhan7/multilingual-e5-large-instruct/tree/main

Test:

All test cases have successfully passed with the inclusion of the latest intfloat/multilingual-e5-large-instruct model.

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 1, 2024

@NirantK I have added Huggingface url which contains onnx model file, also both the tests are passed.

@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 2, 2024

@NirantK could you please review the changes?

Comment thread fastembed/text/e5_onnx_embedding.py Outdated
@Ya-shh Ya-shh marked this pull request as draft April 2, 2024 11:07
@Anush008
Copy link
Copy Markdown
Member

Anush008 commented Apr 2, 2024

Hello @Ya-shh. You've gated your model on HF, which is why a3a3e06 failed. The download should be good after you remove it.

cc58940 won't be necessary.

@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 2, 2024

Hello @Ya-shh. You've gated your model on HF, which is why a3a3e06 failed. The download should be good after you remove it.

cc58940 won't be necessary.

Thanks a lot ! @Anush008 . I have disabled gated user access, you could try it again.

@Ya-shh Ya-shh marked this pull request as ready for review April 2, 2024 16:09
@Ya-shh Ya-shh marked this pull request as draft April 2, 2024 16:25
@Ya-shh Ya-shh marked this pull request as ready for review April 2, 2024 16:33
@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 2, 2024

Hey @Anush008 , now it won't fail . You could run it !

@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 2, 2024

@NirantK all checks have passed !

Comment thread tests/test_text_onnx_embeddings.py Outdated
@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 2, 2024

@Anush008 I have committed the changes .Now it's ready !

Copy link
Copy Markdown
Member

@Anush008 Anush008 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

Copy link
Copy Markdown
Member

@Anush008 Anush008 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Torch canonical values you've shared in the notebook seem to be different from the ones in the test.

@Anush008 Anush008 self-requested a review April 2, 2024 17:50
@Ya-shh Ya-shh marked this pull request as draft April 7, 2024 18:06
@Ya-shh Ya-shh marked this pull request as ready for review April 8, 2024 05:23
@Ya-shh Ya-shh marked this pull request as draft April 8, 2024 06:18
@Ya-shh Ya-shh marked this pull request as ready for review April 8, 2024 08:01
@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 8, 2024

Hi @Anush008, In reference to the issue discussed in #190, I have identified the error in this (PR) as being related to the atol values. Initially, I incorrectly modified the canonical vector values; however, I've now made the necessary corrections to the atol values. Additionally, I am addressing the token_type_ids issue mentioned in #190 (currently marked as draft). All other concerns raised regarding this PR have been resolved.

@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 8, 2024

@NirantK, All the issues with this PR have been resolved.

@Ya-shh Ya-shh requested a review from NirantK April 8, 2024 08:34
Comment thread tests/test_text_onnx_embeddings.py Outdated

canonical_vector = CANONICAL_VECTOR_VALUES[model_desc["model"]]
assert np.allclose(embeddings[0, : canonical_vector.shape[0]], canonical_vector, atol=1e-3), model_desc["model"]
assert np.allclose(embeddings[0, : canonical_vector.shape[0]], canonical_vector, atol=0.05), model_desc["model"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't want to use such large atol values. 1e-3 is already quite large — please find an ONNX model which clears the tests.

Copy link
Copy Markdown
Author

@Ya-shh Ya-shh Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes , I totally agree with you. But even with the e5-small Tests failed at all close

@Ya-shh Ya-shh marked this pull request as draft April 8, 2024 13:03
@Ya-shh Ya-shh marked this pull request as ready for review April 10, 2024 18:14
@Anush008
Copy link
Copy Markdown
Member

You can maybe remove the other models when trying the tests locally.

@Ya-shh Ya-shh marked this pull request as draft April 11, 2024 12:15
@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 12, 2024

@NirantK I have tried every possible way to quantise this model .Optimized it with different levels graph optimizations . And the result remain the same for O4, O3 and O2 with an atol=1e-3

E AssertionError: intfloat/multilingual-e5-large-instruct
E assert False
E + where False = <function allclose at 0x101d18270>(array([ 0.01044647, 0.02897521, 0.00060697, -0.04193534, 0.02606635],\n dtype=float32), array([ 0.01020065, 0.02367218, 0.00117696, -0.04327099, 0.0288757 ]), atol=0.001)
E + where <function allclose at 0x101d18270> = np.allclose

I have even quantized this model further using Static quantization but still no change in the onnx weights .It only passes for atol=1e-2. As you said 1e-3 is already quite large value. So we really can’t add support for this model.

@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 12, 2024

Here is the Onnx model

CANONICAL_VECTOR values :Colab Notebook

@Ya-shh Ya-shh requested a review from NirantK April 12, 2024 15:57
@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 12, 2024

I have tried all of these but still these fails at atol=1e-3. But passes at atol=1e-2 while testing them locally.

Colab notebooks

"O2" level graph optimizations

"O3" level graph optimizations

"O4" level graph optimizations

Onnx/export space

Onnx model using Onnx/export space on HF

@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 12, 2024

@NirantK, locally I have tested both Xenova/multilingual-e5-small & intfloat/multilingual-e5-small onnx models but they also fails at allclose : #123

E AssertionError: intfloat/multilingual-e5-small
E assert False
E + where False = <function allclose at 0x110ad0170>(array([ 0.04931236, 0.02415175, -0.0384715 , -0.08884481, 0.08710264],\n dtype=float32), array([ 0.03131689, 0.03093922, -0.03511665, -0.06727391, 0.08508426]), atol=0.001)
E + where <function allclose at 0x110ad0170> = np.allclose

But unlike e5-large-instruct , e5-small also fails at atol=1e-2

E AssertionError: intfloat/multilingual-e5-small
E assert False
E + where False = <function allclose at 0x10456d470>(array([ 0.04931236, 0.02415175, -0.0384715 , -0.08884481, 0.08710264],\n dtype=float32), array([ 0.03131689, 0.03093922, -0.03511665, -0.06727391, 0.08508426]), atol=0.01)
E + where <function allclose at 0x10456d470> = np.allclose

These multilingual models really require more investigation.

@Ya-shh Ya-shh marked this pull request as ready for review April 18, 2024 05:45
@Ya-shh Ya-shh marked this pull request as draft April 19, 2024 07:24
@Ya-shh Ya-shh marked this pull request as ready for review April 19, 2024 07:48
@Ya-shh
Copy link
Copy Markdown
Author

Ya-shh commented Apr 19, 2024

@NirantK, I am closing this PR for now. Whenever you have time to review, kindly take the time to go through all the attached notebooks. Your thorough review will greatly assist me in addressing any issues with these models. Thank you for your valuable suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants