[Bugfix][Spec Decode] Trim the optimistic spec-decode tokens on every pipeline rank - #574
Open
Peuqui wants to merge 1 commit into
Open
[Bugfix][Spec Decode] Trim the optimistic spec-decode tokens on every pipeline rank#574Peuqui wants to merge 1 commit into
Peuqui wants to merge 1 commit into
Conversation
With async scheduling the drafter's accepted count is assumed optimistically and `prev_num_draft_len` placeholders are appended to `output_token_ids` before the model runs. That extend is not gated on the pipeline rank, but the trim that undoes it sat in an `elif` behind `if not is_last_rank:` and so ran only on the final rank. The deferred correction fixes the token counts, never the placeholders, so on every other rank the cached output grew by the draft length each step and never shrank. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Purpose
Under async scheduling with speculative decoding,
_update_statesoptimisticallyassumes every draft token was accepted and appends that many
-1placeholders toreq_state.output_token_ids:That extend is not gated on the pipeline rank. The trim that undoes it was:
so it only ever ran on the final pipeline rank. The deferred correction
(
correct_spec_decode_token_counts) adjustsnum_computed_tokensandnum_tokens_no_spec, but it never removes the placeholders — the trim is theonly thing that does. On every non-final rank the cached output therefore grew
by the draft length each speculative step and never shrank, so the cached state
drifted away from the scheduler's view of the same request.
The fix turns the
elifinto anif. Single-rank behaviour is unchanged:there
is_last_rankis True, the preceding branch is not taken, and the trimran before this change too.
prev_num_draft_lenis assigned inInputBatch.update_req_spec_token_ids,which runs on every rank, so the growth is not specific to any model or backend.
Test Plan
Environment: checkout at
origin/main4f19ef7 with the compiled extensions of a1Cat-vLLM 1.5.0 wheel linked in (see Limitations).
Test Result
New test alone: 1 passed.
tests/v1/worker/test_gpu_model_runner.py: 39 passed, 2 failed, then theprocess aborts during
cleanup_dist_env_and_memory. Withvllm/v1/worker/gpu_model_runner.pyreverted toorigin/mainand the new testkept, the same command gives 38 passed, 3 failed and the identical abort —
the one extra failure is the new test:
The two failures present on both trees are
test_non_last_pp_rank_profiles_with_speculative_config[ngram]and[draft_model]. They are pre-existing and unrelated: both die insideFlashInfer with
i.e. the FlashInfer paged-prefill kernel does not launch on this bench's GPUs
(Turing/Volta). The corrupted CUDA context is also what aborts the interpreter
at teardown, which is why pytest never prints its own failure report for them.
tests/v1/worker/test_gpu_model_runner_pp.py: 11 passed.The existing PP consistency tests
test_update_states_pp_non_async_multi_request_keeps_token_buffers_consistentand
test_update_states_pp_async_multi_request_keeps_rank_state_consistentpass both before and after, so the newly reachable trim does not disturb the
non-async hand-off path.
pre-commit over both files: ruff check, ruff format, typos, mypy-local, SPDX
headers, root lazy imports, forbidden imports, the torch.cuda-call check,
config-docstring check, attention-backend docs and the boolean-ops check all
Passed.
pre-commit run mypy-3.10 --hook-stage manual: Passed.Not a duplicate
Checked on 2026-09-08 against every open PR. Three open PRs touch
vllm/v1/worker/gpu_model_runner.py— #241, #239 and #235 — and none of theirhunks in that file contain
output_token_ids,is_last_rank,num_output_tokensor_update_states. No open issue mentions the optimisticspec-decode extend or the output trim.
Why it matters in practice
On our own deployment (2x RTX 8000 + 2x V100, TP2 x PP2, MTP) the unbounded
overshoot is not cosmetic. The inflated length feeds the discard/chunked-prefill
bookkeeping; after enough speculative rounds the discard mask flips on the
non-final rank only, the PP broadcast guard then disagrees between ranks — the
final rank keeps sending sampled-token broadcasts while the first rank skips
them — and the run wedges in NCCL. That is what sent us looking for this in the
first place; the unit test above is the reduced, deterministic form of it.
Already reported upstream
The same
elifis present in upstream vLLM (vllm/v1/worker/gpu_model_runner.pyat 9e905f7, 2026-09-01), so this is not a fork-specific regression. I reported it
there on 2026-08-28 as vllm-project/vllm#54260, including the failure chain
above; that issue is still open and unanswered. I am opening the fix here
because this is the tree I run and can test against.
Limitations
Unit tests and linting run against this tree. I have not booted current main
from source on this hardware — that is a multi-hour CUDA build — so there is no
end-to-end throughput number attached to this change. The defect and the fix are
demonstrated by the unit test above, which fails on the unmodified tree and
passes with the change.
AI assistance: this change was developed with Claude (Anthropic) as a coding
assistant. Every changed line was reviewed by me and the test runs above were
executed on my hardware; I can defend the change end to end.
🤖 Generated with Claude Code