-
Notifications
You must be signed in to change notification settings - Fork 530
Add active parameter to ModelMeta
#3837
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
base: main
Are you sure you want to change the base?
Add active parameter to ModelMeta
#3837
Conversation
|
Just added csv for now. Will remove memory usage column, add active embedding to ModelMeta of each model and then add column to LB. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a CSV file containing model parameter data for models in the MTEB leaderboard. The file documents total parameters, active parameters, and input embedding parameters for over 500 models. This addresses issues #3258 and #3259.
Key Changes
- Addition of a CSV file documenting parameter counts (total, active, and input embedding) for 554 models
- The CSV includes success/error status for each model, tracking whether parameter extraction succeeded or encountered issues
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Samoed @KennethEnevoldsen @isaac-chung For model: Also in csv these 3 parameters came from below code. import numpy as np
from transformers import AutoModel
model_name = "google/embeddinggemma-300m"
model = AutoModel.from_pretrained(model_name)
input_params = np.prod(model.get_input_embeddings().weight.shape)
total_params = np.sum([np.prod(x.shape) for x in list(model.parameters())])
active_params = total_params - input_paramsSo, what exactly, I should add in ModelMeta? Should I add 2 new field in ModelMeta with name |
|
Let's modify leaderboard in separate PR
Yes, this is expected that total parameters = active parameters + embedding paramters
Probably yes
Let's discuss this in issue |
ModelMeta
|
Converting this to a draft given the current state, but great to see it!
|
Yes, I will be adding the logic to calculate automatically in from_hub method. I think MOE are case, where I was not able to calculate it. Hence I think atleast in ModelMeta we can keep n_parameters and add both active and embedding. Also, as I have already calculated these parameters for around 315 models, and around 60 models are propritory out of 498 models we have currently.
For now, we can use csv. But, I think its better to keep both in ModelMeta as then we could also handle it for MOE case, where these property will not be useful. |
|
I have updated files with these meta and also added it to |
|
Previously when I was doing |
|
Strange. I don't have this behavior |
|
Hmm do we want to use the csv? I think we should rather use the ModelMeta object - if not then I think we should use the json format similar to what we do with descriptive statistics |
|
I think csv just for demonstration |
Its just for demonstration of all results I have calculated, and I had filled ModelMeta of models using that csv. |
KennethEnevoldsen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would convert active parameters to a property and just add embedding parameters.
|
Converted |
5eda71b to
3db3369
Compare
|
@KennethEnevoldsen @Samoed How do I actually check for MOE, here problem is, |
Yes, but you can set
I'm not sure about automating it. I think model authors can submit their score by themselves. You can try to test this on |
But, kenneth want to have it only for MOE models in ModelMeta, and for rest of models it should not be present. So, I was thinking how to get MOE. I am okay with keeping it for all models in ModelMeta, but then what's point of keeping it as a property? though it will be used for future models where results are not reported.
So, I think the best way is to keep |
|
1 more question in ModelMeta, it can't be named as |
Yes, you shouldn't fill it for other models, I think. I don't see a problem here.
I think you can name change |
Yes, problem is just to identify which models are MOE. |
|
I have added As per my analysis, below are list of model2vec models, let me know if there is any model which is not, or are there any other model2vec models:
|
|
I don't know why a typecheck error is coming; I have also added explicit conditions for that. And I have also tested code for both sentence-transformer and cross-encoder models. Also, there are some conflicts in dependencies. |
|
|
||
| meta = cls._from_hub(model.model.name_or_path, revision, compute_metadata) | ||
| try: | ||
| if isinstance(model.model, PreTrainedModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this check is necessary because you'll get valid cross-encoder model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I though typecheck is failing because it was not able to identify this one. So, I have added explicit checks.
| meta = cls._from_hub(name, revision, compute_metadata) | ||
| try: | ||
| first_module = model[0] | ||
| if hasattr(first_module, "auto_model"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need these checks?
closes #3258