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

[fix] Added model2vec import compatible with current and newer version #2992

Merged
merged 10 commits into from
Oct 18, 2024

Conversation

Pringled
Copy link
Contributor

Hi!

Pull Request overview

  • Make sentence-transformers compatible with current and newer version of Model2Vec

Details

Starting from model2vec version 0.3, inference and distillation will be split up since inference will only need Numpy. This makes it much faster to initialize and install, since there is no dependency on Torch anymore for inference. This PR makes sentence-transformers compatible with both the old and new version, including an updated ImportError message for from_disillation.

Relevant Model2Vec commit: MinishLab/model2vec@010a7b7

@tomaarsen
Copy link
Collaborator

tomaarsen commented Oct 17, 2024

Hello!

This looks solid I think! Also, torch-less inference on model2vec itself is quite nice, well done.

Have you tested from_model2vec with the latest model2vec? I think this could fail:

static_model = StaticModel.from_pretrained(model_id_or_path)
embedding_weights = static_model.embedding.weight

  • Tom Aarsen

@tomaarsen
Copy link
Collaborator

I added some simple tests to make sure that things don't break in the future.

@Pringled
Copy link
Contributor Author

@tomaarsen good catch, those need to be converted! Will update the code

@tomaarsen
Copy link
Collaborator

@Pringled I already have a fix locally, I'll take care of it :)

@Pringled
Copy link
Contributor Author

Nice! That should work with both the old and new versions 💪

@tomaarsen tomaarsen merged commit 29535eb into UKPLab:master Oct 18, 2024
11 checks passed
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.

2 participants