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

Integrate mme5 models #2081

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Aradhye2002
Copy link
Contributor

@Aradhye2002 Aradhye2002 commented Feb 17, 2025

Changes/additions

mteb/models/mme5_models.py
mteb/models/mme5

Explanation for adding the mme5 folder:

haon-chen/mmE5 doesn't expose the the repository as a pip installable package, hence it is necessary to have the required model classes and functions (along with their dependencies) available locally. One option could have been to dump all these files in mme5_models.py itself, however, due to the large number of lines of code, this approach has been adopted.

Closes #2079

Code Quality

  • Code Formatted: Format the code using make lint to maintain consistent style.

Documentation

  • Updated Documentation: Add or update documentation to reflect the changes introduced in this PR.

Testing

  • New Tests Added: Write tests to cover new functionality. Validate with make test-with-coverage.
  • Tests Passed: Run tests locally using make test or make test-with-coverage to ensure no existing functionality is broken.

Adding a model checklist

  • I have filled out the ModelMeta object to the extent possible
  • I have ensured that my model can be loaded using
    • mteb.get_model(model_name, revision) and
    • mteb.get_model_meta(model_name, revision)
  • I have tested the implementation works on a representative set of tasks.

mteb/models/mme5_models.py
@Aradhye2002 Aradhye2002 marked this pull request as ready for review February 17, 2025 06:19
@Aradhye2002
Copy link
Contributor Author

It seems the mmE5 repo is not installable via pip install. Can't seem to think of a way of using the MMEBModel, ModelArguments and load_processor from their repository (which is required for loading the model) without explicitly adding their repository to the mteb repo and then changing the PYHTHONPATH. This seems quite inelegant and I'm open to possible alternative solutions.

@Aradhye2002 Aradhye2002 changed the title Integrate mme5 models Integrate mme5 models (#2079) Feb 17, 2025
@Samoed
Copy link
Collaborator

Samoed commented Feb 17, 2025

You can copy their model class and other functions from library

@Aradhye2002
Copy link
Contributor Author

You can copy their model class and other functions from library

I see. Should I add the helper functions and model classes to the mme5_models.py (created) file itself?

@Samoed
Copy link
Collaborator

Samoed commented Feb 17, 2025

Yes, I think this is best option

@Samoed Samoed changed the title Integrate mme5 models (#2079) Integrate mme5 models Feb 17, 2025
@Samoed
Copy link
Collaborator

Samoed commented Feb 17, 2025

You should copy only required classes, not full repo

@Aradhye2002
Copy link
Contributor Author

Aradhye2002 commented Feb 17, 2025

You should copy only required classes, not full repo

I've only kept the required classes and functions and deleted the rest (only the directory structure has been maintained). Let me know if you still want them in the same file ie. mme5_models.py

@Samoed
Copy link
Collaborator

Samoed commented Feb 17, 2025

This seems a lot of files. I think we can wait until they convert to library or integrated to sentence transformers

@Aradhye2002
Copy link
Contributor Author

This seems a lot of files. I think we can wait until they convert to library

Noted.

@gowitheflow-1998 gowitheflow-1998 self-assigned this Feb 18, 2025
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.

Integrate mmE5
3 participants