Fetch models results from HF

[Samoed]

I've implemented a base version of retrieving results from the model card. This PR is a work in progress and is open for discussion. Currently, I’ve parsed the results for intfloat/multilingual-e5-large as a demo. During integration with other results, I ran into some issues, particularly with loading results using TaskResult. External results don’t have evaluation_time and mteb_version, which TaskResult requires.

@KennethEnevoldsen @x-tabdeveloping
Ref: embeddings-benchmark/mteb#1405, embeddings-benchmark/mteb#1373

[x-tabdeveloping]

This is awesome! You're doing god's work here :D Thanks

[x-tabdeveloping]

I think adding something invalid for the evaluation_time attribute like -1 would make quite a bit of sense. Otherwise we can also make it possible in the mteb library to allow None, though I'd prefer not to do this. What do you think @KennethEnevoldsen

[KennethEnevoldsen]

This looks really good! We should probably do this as a one-time thing only (don't support it going forward).

[KennethEnevoldsen]

Hmm maybe we should make it clear that this is NA

[KennethEnevoldsen]

(potentially by wrapping the original object to allow for None

[Samoed]

You mean that we should fetch results only one time for each model and not check them for updates?

[KennethEnevoldsen]

You mean that we should fetch results only one time for each model and not check them for updates?

Either that or we should update the create_meta CLI to also include MTEB version and eval time.

I would rather allow people to create a MTEB folder in the repo with the results.

1000 evaluation is also a bit hard to reason about:

I would much rather have:

score on MTEB(eng): 0.86
score on MTEB(multilingual): 0.71
...

[Samoed]

I would much rather have:
score on MTEB(eng): 0.86
score on MTEB(multilingual): 0.71

You mean that we should remove from mteb create_meta results of the tasks and left only average result of the benchmark? And all other results should be in repo of model?

[isaac-chung]

Hey @Samoed wow thanks for working on this!

I think adding something invalid for the evaluation_time attribute like -1 would make quite a bit of sense. Otherwise we can also make it possible in the mteb library to allow None, though I'd prefer not to do this. What do you think

Re: evaluation_time: using -1 is likely easier for us to keep track of the results that needed attention down the line. I'd keep away from using None - right now having the field check is good.

we should update the create_meta CLI to also include MTEB version and eval time.

Let's do that for sure.

I would rather allow people to create a MTEB folder in the repo with the results.

I think this could be a great main option forward, instead of PRs to the results repo (i.e. both are accepted, but the MTEB folder is preferred). People's results should be generated with MTEB version and eval time (see above). For time to be meaningful, we might also want to track hardware info, right? Every time we scan for the MTEB folders, we can validate the results objects as well.

I'm all for automating as much as we can :P

[isaac-chung]

Slightly related to the eval time topic, maybe we could start enforcing kg_co2_emissions to be filled in as well, as @x-tabdeveloping suggested? Maybe CodeCarbon takes into account both eval time and hardware together. Then this plot might be more meaningful when comparing different runs using different machines.

This will add 1 new dependency in MTEB but I think the benefits is worth it.

[KennethEnevoldsen]

+1 for enabling code-carbon by default

re -1 for values: This would lead to issues when e.g. taking a mean. I would use math.nan instead (it is a float and an invalid value).

It sounds like for now:

• Keep create_meta: it is still useful for adding certain tasks to metadata but it will not auto-update the leaderboard
• Use only embedding-benchmark/results though planning in the future to add the option to use a folder on HF

[Samoed]

For now what I should change, except -1 value?

[isaac-chung]

@KennethEnevoldsen in this PR embeddings-benchmark/mteb#1398, -1 was used in favor of NaN. To be consistent, should we keep the -1 approach? Unless some metrics aren't between [0-1].

[KennethEnevoldsen]

That PR is more about adding a warning to existing code (I also think it is a bad idea in that case)

Would def. change it to NA here.

[KennethEnevoldsen]

@Samoed if we add the NA here (runtime, version) I believe this is ready to merge. It is one of the few missing requirements for the new release of the leaderboard 2.0

[Samoed]

@KennethEnevoldsen I've updated results

[KennethEnevoldsen]

Hmm I believe NaN is not valid json, shouldn't it be "null"? If it can be read I guess it doesn't matter.

for the MTEB version I would probably change it to:
"mteb_version": "unknown"

[Samoed]

Yes, it’s a bit invalid as JSON, but Python can read it correctly.

Changing it to null would require updates to the MTEB loader, as it currently only accepts float values.
Now there are some tasks with NaN values too.

[Samoed]

It also seems the loader requires the version to be a parsable string

[KennethEnevoldsen]

I have added a PR which should allow the fix

[KennethEnevoldsen]

@Samoed feel free to just merge it in

[Samoed]

@KennethEnevoldsen I don't have permission to merge in this repository. Also, should we keep evaluation_time as math.nan, or change it to None?

[KennethEnevoldsen]

Either is fine - I just wanted to make sure that the json was generally valid

[KennethEnevoldsen]

@Samoed I think the update is in on main - updating to the latest version would allow us to get this merged in