-
Notifications
You must be signed in to change notification settings - Fork 27.7k
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
Whisper Prompting max_new_tokens #25422
Comments
Hi @Helene-Maxcici! Thanks for writing this issue, there’s definitely an out of bounds issue here. Appreciate you catching the precedence issue that the slicing doesn’t quite match OpenAI’s, we should change that in the fix PR so its slicing one less than half the max_length instead one one more than half. Ultimately it’s not at the root of this problem since the prompt isn’t competing for space with anything else, like a prefix, and we could just decrement the max_new_tokens param by 1 and this script would run, or alternatively after updating the slicing to match OpenAI’s we could still increment max_new_tokens by 2 to 226 and it would still have this error. Instead, I think the issue is that the length stopping criteria warning here doesn’t capture the out of bounds issue for this model since the it looks here for |
I'm not sure if this will help or not but I faced the same error running generated_tokens = (
model.generate(
input_features=batch["input_features"].to("cuda"),
decoder_input_ids=batch["labels"][:, :4].to("cuda"),
max_new_tokens=448,
) However if I use PEFT model as in model = WhisperForConditionalGeneration.from_pretrained(
peft_config.base_model_name_or_path, device_map="auto", load_in_8bit=True)
model = PeftModel.from_pretrained(model, evaluate_model) I don't face this issue if I set the |
Thanks for the excellent issue description @Helene-Maxcici and for the astute remarks @connor-henderson! IMO each of the findings deserves a PR of its own:
Would you like to open a PR for one or both of these issues @Helene-Maxcici? Happy to help guide the integration process, or answer any questions / queries along the way! |
Hi @sanchit-gandhi , thank you for your response! I would be happy to open a PR for each. |
Thank you for opening a well-explained issue, @Helene-Maxcici! 🤗 Since this issue is particular to Whisper, which modifies |
The slicing bug was fixed by @connor-henderson in #23724. The check for exceeding the max length of the model should be fixed by #26164. |
System Info
transformers
version: 4.31.0Who can help?
@sanchit-gandhi
Information
Tasks
examples
folder (such as GLUE/SQuAD, ...)Reproduction
Bug Related
We keep
model.config.max_length=448
. The error happens when:len(prompt_ids) + max_new_tokens > model.config.max_length + 1
max_new_tokens
inmodel.generate()
eos
token and starts repeating some sequence of tokens.Output:
The bug might be caused by no condition set on
max_new_tokens
inside thegenerate()
function, which might be a general bug for generation and not only for prompting.Note
Also, as I was reading the code I noticed this line:
text_prompt_ids = text_prompt_ids[-self.config.max_length // 2 - 1 :]
It slices the text prompt ids and takes
(self.config.max_length // 2 + 1)
tokens instead of(self.config.max_length // 2 - 1)
as taken in the original code of Whisper here.Expected behavior
model.max_length
.max_new_tokens=224 ( = max_length // 2)
during prompting.The text was updated successfully, but these errors were encountered: