Skip to content
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

Ability to set max_seq_len to the SentenceTransformers components #8335

Closed
bglearning opened this issue Sep 5, 2024 · 0 comments · Fixed by #8334
Closed

Ability to set max_seq_len to the SentenceTransformers components #8335

bglearning opened this issue Sep 5, 2024 · 0 comments · Fixed by #8334
Assignees
Labels
2.x Related to Haystack v2.0

Comments

@bglearning
Copy link
Contributor

bglearning commented Sep 5, 2024

Is your feature request related to a problem? Please describe.
The SentenceTransformer models have a max_seq_len attribute.

In theory, we could set it with the model_max_length in tokenizer_kwargs which then eventually should set that attribute here.

However, it seems to be unreliable. We saw it with bge-m3 but could also be the case for other models. (Colab)
This can result in OOM error when embedding.

Update: found the "issue". The max_seq_length is read into the kwargs from this config json at this point in _load_sbert_model and thus the max_seq_length is not None here and so it doesn't use model_max_length from the tokenizer_kwargs.

Describe the solution you'd like
Would be good to have the ability to set the max_seq_len in the (currently three) SentenceTransformers components as in v1.

We could possibly also intercept the tokenizer_kwargs and use model_max_length from it if it's set.

@bglearning bglearning added the 2.x Related to Haystack v2.0 label Sep 5, 2024
@sjrl sjrl self-assigned this Sep 6, 2024
@sjrl sjrl closed this as completed in #8334 Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Related to Haystack v2.0
Projects
None yet
2 participants