-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Update config validation #34726
base: main
Are you sure you want to change the base?
Update config validation #34726
Conversation
The failing tests seem unrelated to my changes and all tests passed locally for me, is there something I'm missing here? |
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.
Cool, thanks for working on this! The tests failing are not related and re-trigerring run should help :)
integer_arguments = ["num_beams", "num_beam_groups", "top_k", "no_repeat_ngram_size"] | ||
positive_arguments = [ |
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.
we can add all integer
values in positive
values, as they cannot be negative anyways. So we check in one place if it is a positive integer
"num_beam_groups", | ||
"no_repeat_ngram_size", | ||
] | ||
probability_arguments = ["top_p", "typical_p", "min_p", "epsilon_cutoff", "eta_cutoff"] |
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.
can also check on positive float numbers which are: diversity-penalty, repetition-penalty, encoder-repetition-penalty
What does this PR do?
Fixes #33690
This PR adds extra validation for generation parameters to fail early when invalid values are passed and not let things fail silently as suggested in the issue.
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@gante @zucchini-nlp