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

Add version config option. #181

Merged
merged 8 commits into from
May 3, 2024

Conversation

PhilipMay
Copy link
Contributor

see #172

@PhilipMay PhilipMay changed the title Add version config option. [WIP] Add version config option. Apr 30, 2024
@PhilipMay
Copy link
Contributor Author

@clefourrier

You write:

and edit the task_table.jsonl to set "version" to 0

I do not understand why. When version has the default of 0 I do not need to change that file.
Only if someone changes the prompt or the underlying data the version can be increased.

What do you think?

@PhilipMay
Copy link
Contributor Author

@clefourrier can you please review the PR. I added some TODO comments. They need to be checked.

@jphme maybe you also could have a quick look?

@PhilipMay PhilipMay changed the title [WIP] Add version config option. Add version config option. May 2, 2024
@clefourrier
Copy link
Member

Hi @PhilipMay !
I think it's better to make explicit the task version in the json file, otherwise it will be too easy to forget to edit it afterwards.

@PhilipMay
Copy link
Contributor Author

Hi @PhilipMay ! I think it's better to make explicit the task version in the json file, otherwise it will be too easy to forget to edit it afterwards.

Ahh I see. I made the change.

@PhilipMay
Copy link
Contributor Author

Hi @PhilipMay ! I think it's better to make explicit the task version in the json file, otherwise it will be too easy to forget to edit it afterwards.

Should I also set the version to 0 in https://github.com/huggingface/lighteval/blob/main/community_tasks/arabic_evals.py ?

@clefourrier
Copy link
Member

Yep, let's be exhaustive :)

@PhilipMay
Copy link
Contributor Author

Yep, let's be exhaustive :)

I am done with that.

@clefourrier
Copy link
Member

Nice! Just need to merge main in your branch and I think we'll be good to go

Copy link
Member

@clefourrier clefourrier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks, let's merge once the tests pass

@clefourrier clefourrier merged commit c89b386 into huggingface:main May 3, 2024
2 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.

None yet

2 participants