-
Notifications
You must be signed in to change notification settings - Fork 227
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
Extend lm_eval functionality #1729
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Yaser Afshar <[email protected]>
Co-authored-by: Yaser Afshar <[email protected]>
Co-authored-by: Yaser Afshar <[email protected]>
Improve after first pass review
@yafshar, @alexey-belyakov this is part II |
Can you add examples that you tried to the PR README? |
Related to the examples to add, @yafshar, I've added llama-3.2-1B. While for this model (w/o Instruct) the accuracy we get is the same as reported in https://huggingface.co/meta-llama/Llama-3.2-1B-Instruct (i.e., 32.2), I've noticed that for other llama models it is sometimes around 2-3 point less, so I'm checking if everything is correct. As soon as done, I will add also an example for ifeval. I believe we should remove older examples, rather than simply add another couple. What is your thought? |
@12010486 please let me know when the PR ready for the review! Thanks |
@yafshar ack, I will ping you. In the background I'm adding needed or useful parameters, but I'm blocked on ifeval task, as accuracies on that task are still not replicable by our code. |
logger.info( | ||
f"Model type is '{self._config.model_type}', part of the Gemma family--a BOS token will be used as Gemma underperforms without it." | ||
) | ||
|
||
self._max_length = options.max_length |
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.
@12010486 is this correct? You set this before in previous PR, so the super class max_length
is using this, but the current derived class has a different max_length
@property
def max_length(self) -> int:
return self.buckets[-1]
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.
Good catch @yafshar. We need the property to be defined that way, or else max_lengh is going to change every time causing OOM. I can remove line 192, though
Hello! Thank you for the progress on this update! |
One minor comment concerning GSM8K. |
@veritas9872, very useful comments! In fact, using |
What does this PR do?
Extended
run_lm_eval.py
to support latest tasks, like MMLU, MMLU-Pro, IFEval and GSM8K.Before submitting