[Whisper] Check length of prompt + max new tokens #26164
Conversation
| decoder_start_token_id, *text_prompt_ids = prompt_ids | ||
| # Slicing the text prompt ids in a manner consistent with the OpenAI implementation | ||
| # to accomodate context space for the prefix (see https://github.com/openai/whisper/blob/c09a7ae299c4c34c5839a76380ae407e7d785914/whisper/decoding.py#L599) | ||
| text_prompt_ids = text_prompt_ids[-self.config.max_length // 2 - 1 :] |
There was a problem hiding this comment.
The equivalent variable for the slicing in the OAI implementation is n_ctx: https://github.com/openai/whisper/blob/c09a7ae299c4c34c5839a76380ae407e7d785914/whisper/decoding.py#L599
This corresponds to our max_target_positions:
=> I've corrected this in our implementation. Note that for the pre-trained checkpoints, we set max_length = max_target_positions, so this won't change the behaviour here.
However, it will fix the behaviour for newly initialised checkpoints, where max_length does not match max_target_positions (former defaults to 20, latter defaults to 448).
|
The documentation is not available anymore as the PR was closed or merged. |
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
Nice! It's not backward comp but it's fix as you mentioned so let's go with this! 💉
3 participants
What does this PR do?
Fixes #25422: adds a check for the combined length of prompt + max new tokens. If this total exceeds the model's max length (
max_target_positions), we throw an error.cc @connor-henderson @Helene-Maxcici