[fix][whisper]: fix max_new_tokens handling - #46795
Merged
Merged
Conversation
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: whisper |
Collaborator
Author
|
run-slow: whisper |
Contributor
|
This comment contains models: ["models/whisper"] |
Contributor
|
CI Dashboard: View test results in Grafana |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
vasqu
approved these changes
Jul 20, 2026
vasqu
left a comment
Collaborator
There was a problem hiding this comment.
Wow thanks for catching this. It's really hard to keep up with custom generation loops 😢
stevhliu
pushed a commit
to stevhliu/transformers
that referenced
this pull request
Jul 30, 2026
fix max_new_tokens handling
Sainava
pushed a commit
to Sainava/Sai-transformers
that referenced
this pull request
Aug 3, 2026
fix max_new_tokens handling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
PR #44130 changed how the generation loop enforces
max_length: it is now the strict total decoder length (forced/initial tokens + generated content), whereas the oldcache_positionbased approach effectively didn't count one position.Whisper prepends forced decoder tokens before the transcript (
[<|startoftranscript|>, <|lang|>, <|transcribe|>, <|notimestamps|>]). So when a user passesmax_length=20they mean "20 content tokens," and_set_max_new_tokens_and_lengthis supposed to inflate the limit to leave room for that forced prefix. Two defects broke this:The compensation was computed but discarded.
The function computed
max_length = min(generation_config.max_length + num_initial_tokens, …)but a refactor Fix whisper kwargs and generation config #30018 dropped the line that wrote it back. That refactorreworked
_set_max_new_tokens_and_lengthfrom returning a kwargs dict (which setkwargs["max_length"]) to mutatinggeneration_configin place. The max_new_tokens branch got its in-place replacement, but the max_length branch's equivalent (generation_config.max_length = max_length) was never carried over. It became dead code, harmless under the old loop, actively harmful once [generate] Completely stop relying oncache_positionto prepare inputs #44130 made max_length strict.An off-by-one hidden in num_initial_tokens.
The compensation used
decoder_input_ids.shape[-1] - 1. That -1 existed to cancel the old loop's free (uncounted) position, so the net inflation still came out to the full prefix length. PR [generate] Completely stop relying oncache_positionto prepare inputs #44130 removed that quirk, so the -1 now under-counts the prefix by one, leaving the output one token short even after re-adding the assignment.TD;DR: #30018 planted latent dead code, #44130 activated it
Fixes:
test_modeling_whisper.py::WhisperModelIntegrationTests::test_tiny_generationtest_modeling_whisper.py::WhisperModelIntegrationTests::test_tiny_en_generation