-
Notifications
You must be signed in to change notification settings - Fork 531
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
Replace num_beams
with generations_per_sample
#971
Conversation
Overall, these changes are so that we use the same variable names in both Right now, the PR Make CodeEval respect device_eval_batch_size uses the variable |
I believe this PR is redundant with the accompanying foundry PR of the composer you linked: #956 I didn't raise a DeprecationWarning because I do think specifying |
I'm going to close this PR, it is being covered in #956 . Let me know if you disagree. |
In
composer
'sbuild_icl_dataloader
function (here), we specify the number of generations per sample with the variablegenerations_per_sample
.But when we call
build_icl_dataloader
inllm-foundry
here, we specified the number of generations per sample with the variablenum_beams
. This was confusing becausenum_beams
is also used as a parameter for beam search in inference.To resolve this, I replaced all instances of
num_beams
withgenerations_per_sample
inllm-foundry
. Ifgenerations_per_sample
is not specified in the config andnum_beams
is specified, we will use the value set bynum_beams
and raise a warning thatnum_beams
is depreciated.Cheers!