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 a few typos and do a tiny refactor #187

Merged
merged 15 commits into from
Jul 5, 2024

Conversation

sadra-barikbin
Copy link
Contributor

@sadra-barikbin sadra-barikbin commented May 7, 2024

Hi there!

To fix a few typos and do a tiny refactor.

@sadra-barikbin sadra-barikbin changed the title Fix a tiny bug & typo Fix a tiny bug & typo in evaluation_tracker.py and model_config.py May 7, 2024
@sadra-barikbin
Copy link
Contributor Author

Also there's no argument called model_config defined in both run_evals_accelerate.py and run_evals_nanotron.py. Is this correct?

if args.model_config:
config = args.model_config["model"]

@NathanHB
Copy link
Member

hey thanks for the PR ! the model config arg was deprecated in favor of a file. we forgot to remove the in the constructor.

@sadra-barikbin sadra-barikbin changed the title Fix a tiny bug & typo in evaluation_tracker.py and model_config.py Fix a few comment typos and do a tiny refactor May 12, 2024
@sadra-barikbin sadra-barikbin changed the title Fix a few comment typos and do a tiny refactor Fix a few typos and do a tiny refactor May 12, 2024
@sadra-barikbin
Copy link
Contributor Author

I'm done @NathanHB

@sadra-barikbin sadra-barikbin requested a review from NathanHB May 22, 2024 09:33
@NathanHB
Copy link
Member

hey ! thanks for the PR, lgtm, i will test it locally next week and merge asap :)

@clefourrier
Copy link
Member

Hi @sadra-barikbin , just took a look at your PR!
Please revert the import you made and resolve the merge conflicts and we'll be good to merge :)

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!

@clefourrier clefourrier merged commit 843a0f8 into huggingface:main Jul 5, 2024
2 checks passed
hynky1999 pushed a commit to hynky1999/lighteval that referenced this pull request Jul 12, 2024
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.

3 participants