add Multilingual E5 models #1
Conversation
|
This is a benchmark review for experiment This pull request was cloned from |
brendanator
left a comment
brendanator
left a comment
There was a problem hiding this comment.
Hey @brendanator - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Docstrings: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| // Large model is something wrong, model.onnx size is only 546kB | ||
| // /// Large model of multilingual E5 Text Embeddings |
There was a problem hiding this comment.
It's helpful for future maintenance to include a more detailed comment on why the Large model is commented out and under what conditions it might be revisited or fixed.
| // Large model is something wrong, model.onnx size is only 546kB | |
| // /// Large model of multilingual E5 Text Embeddings | |
| // The Large model of multilingual E5 Text Embeddings is currently not included due to an issue with its ONNX file size (only 546kB), | |
| // indicating a potential export or conversion problem. This model variant should be revisited once the underlying issue is resolved | |
| // or if a correctly sized model file becomes available. |
| } | ||
|
|
||
| /// Private method to return an instance | ||
| fn new(tokenizer: Tokenizer, session: Session) -> Self { |
There was a problem hiding this comment.
TextEmbedding::new
Reason for update: The function signature and internal logic have changed to include a new field need_token_type_ids.
Suggested new docstring:
/// Private method to return an instance, determining if `token_type_ids` are needed based on the session inputs.
This PR adds intfloat/multilingual-e5-base, -small models.
these models have no "token_type_ids" inputs, so I introduce check logic.
curiously, intfloat/multilingual-e5-large onnx model is only 546kB(small is 470MB, base is 1.11GB). and it can't run inference. so, I commented out for large model definition.