Repository navigation
[Whisper] Fix batch decode_with_timestamps in WhisperTokenizer.decode() - #47997
Merged
Merged
Conversation
…er.decode() PR #40936 (05c0e1d) made batch_decode delegate to decode() natively, passing a 2D list of token ids. The decode_with_timestamps branch was not updated to handle batched input, causing TypeError when >= compared a list to an int. Add the same isinstance check already present in the else branch. Fixes test_whisper_longform_single_batch (first failing in CI on 2025-11-28, introduced by PR #40936 on 2025-11-27). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: whisper |
|
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. |
Contributor
CI recapDashboard: View test results in Grafana |
Collaborator
Author
|
Despite this touching a file outside the tests directory, the analysis shows the root cause and the fix is clearly necessary and correct. Merging directly. |
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.
Summary
Continuation of #47995 ([Whisper] Fix integration test failures on A10G).
Fixes
test_whisper_longform_single_batch, one of the 5 tests left in Group 5 of that PR as requiring changes outside the test file.Why the test passed before PR #40936 (commit
05c0e1d390)Before
05c0e1d390, the base classPreTrainedTokenizerBase.batch_decodeiterated per-sequence, callingdecode()once per sequence with a single 1D list of token ids.decode()inWhisperTokenizertherefore always received a 1D input, so bothdecode_with_timestamps=TrueandFalseworked correctly for both single and batch inputs.Root cause
PR #40936 (commit
05c0e1d390, "rm slow tokenizers", Nov 27 2025) changed the base classbatch_decodeto pass the full 2D batch (list of token id lists) todecode()in one call instead of iterating per-sequence.decode()inWhisperTokenizerwas partially updated to handle the 2D input:elsebranch (non-timestamp path) gotisinstance(text, list)handling for_filter_timestamp_idsdecode_with_timestamps=Truebranch was missed — it still calls_decode_with_timestamps(filtered_ids, ...)unconditionallyWhen
batch_decode(decode_with_timestamps=True)is called with a batch,filtered_idsis now a list of lists._decode_with_timestampsiterates over it and immediately hitstoken >= timestamp_beginwheretokenis a list →TypeError: '>=' not supported between instances of 'list' and 'int'.Bisect confirmed: parent commit
01c51596ecpasses,05c0e1d390fails. First seen in CI on 2025-11-28 (the day after the commit landed).Why only this one test is affected
decode_with_timestamps=Trueis used in exactly one integration test in the entire whisper test file —test_whisper_longform_single_batch(line 2348). All other tests use either:output_offsets=True— the newer structured return format, a different code path entirelybatch_decode(skip_special_tokens=True)— no timestamp decodingThe other 4 tests remaining in Group 5 of #47995 have completely unrelated root causes and are not addressed here.
Fix
Add the same
isinstancebatch check to thedecode_with_timestampsbranch that was already applied to theelsebranch: detect a 2D input and process each sequence individually.Test
WhisperModelIntegrationTests::test_whisper_longform_single_batch— verified passing on runner (A10G, torch 2.13 / CUDA 13.0).🤖 Generated with Claude Code