Skip to content

[Bugfix] Align Qwen GDN gates with speculative tokens - #51812

Merged
Isotr0py merged 2 commits into
vllm-project:mainfrom
ZJY0516:fix/qwen-gdn-spec-gating-order
Aug 11, 2026
Merged

Isotr0py merged 2 commits into
vllm-project:mainfrom
ZJY0516:fix/qwen-gdn-spec-gating-order

Conversation

@ZJY0516

@ZJY0516 ZJY0516 commented Aug 11, 2026

Copy link
Copy Markdown
Member

Purpose

Fix Qwen GDN speculative decoding when a mixed batch places non-speculative tokens before speculative tokens.

mixed_qkv is gathered with spec_token_indx, but the fused recurrent update previously received the unsorted a and b gate tensors. The kernel consumes the first T_spec gate rows, so the gates could belong to different tokens than the gathered Q/K/V rows. Gather a and b with the same indices, while keeping the pure-spec fast path unchanged.

Model evaluation

Reproduced end-to-end with Qwen/Qwen3.5-2B, BF16, TP1, V1 runner, eager mode, two MTP draft tokens, and max_model_len=128. The small model-length limit makes the boundary condition quick to reach; the same condition can occur near any configured context limit.

Prompts of 120 and 64 tokens produced a mixed step with a one-token non-speculative row before a three-token speculative-decode row. The GDN metadata builder initially classifies any one-token non-speculative row as a decode based only on query length, without consulting whether the request is still prefilling. When a speculative decode is present, it reclassifies that row to the prefill kernel, but the row remains non-speculative for token splitting. Therefore either a true one-token decode or a final one-token prefill chunk can produce this mixed layout. Runtime metadata showed:

spec_token_indx     = (1, 2, 3)
non_spec_token_indx = (0,)

For the speculative request, compared with a no-MTP target-model reference from the first post-mixed-step token onward:

Metric Before After
Mean absolute chosen-logprob error 0.002755 0.000208
Maximum chosen-logprob error 0.020539 0.001690

The greedy token IDs were identical before and after because the logit drift did not cross an argmax boundary in this case. The same workload on the V2 runner put speculative tokens first and was already unaffected.

Tests

qwen 3.5 blackwell tests passed https://buildkite.com/vllm/ci/builds/83349/canvas?jid=019ff0e3-ac1a-4a4e-a797-ef903d9c7985&tab=output

.venv/bin/python -m pytest tests/kernels/mamba/test_gdn_forward_core_split.py -q
# 8 passed

.venv/bin/pre-commit run --files vllm/model_executor/layers/mamba/gdn/qwen_gdn_linear_attn.py
# passed, including Ruff and mypy

AI assistance

OpenAI Codex assisted with diagnosis, implementation, testing, and preparation of this PR. The human submitter must review every changed line and remains responsible for understanding and defending the change end-to-end.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added qwen Related to Qwen models bug Something isn't working labels Aug 11, 2026
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
@ZJY0516
ZJY0516 force-pushed the fix/qwen-gdn-spec-gating-order branch from 26c679f to 109be29 Compare August 11, 2026 12:05
@ZJY0516

ZJY0516 commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #83348 for commit 109be29c6dfd.

@ZJY0516

ZJY0516 commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #83349 for commit 37e0aa83b01f.

justtestingthingsx pushed a commit to meandmyboiclaude/vllm that referenced this pull request Aug 11, 2026
…-spec tokens precede spec tokens in a mixed batch

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arpera

arpera commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

LGTM

@Isotr0py
Isotr0py merged commit 5af7c8d into vllm-project:main Aug 11, 2026
95 checks passed
@Isotr0py
Isotr0py deleted the fix/qwen-gdn-spec-gating-order branch August 11, 2026 15:35
xianbaoqian added a commit to xianbaoqian/recipes that referenced this pull request Aug 14, 2026
`Qwen3_5ForConditionalGeneration` is already registered at 0.17.0, so that is
the floor. The runs behind this recipe used the pinned Docker image (a fork
build in the 0.27 range), so older releases are unverified rather than
known-good; `spec_decoding` in particular wants the gated-delta-net
speculative fix (vllm-project/vllm#51812 / #51674), which no released tag
carries yet.

The floor stays a NUMBER rather than the string "nightly" the 2.4T sibling
uses, because maxVersion() parses each field through `parseInt(...) || 0` —
"nightly" scores 0.0.0 and loses to the kv_store YAMLs' 0.21.0, understating
the requirement in the Install header whenever a Mooncake KV-offload option is
selected.

Difficulty drops to beginner: every variant loads on a single GPU at TP1.

The MXFP4 troubleshooting note is scoped to NVIDIA, where the missing linear
method support actually bites, rather than reading as a blanket statement.

Signed-off-by: Tiezhen WANG <38108242+xianbaoqian@users.noreply.github.com>
@malaiwah

Copy link
Copy Markdown

This landed before we could contribute to it, so this is not a report — it is the reachability datum
the PR did not need but a backporter might: the batch composition your fix guards against occurs on
real traffic, in a shipped configuration, at 0.515 events per thousand GDN metadata builds.

We instrumented vllm/v1/attention/backends/gdn_attn.py on a Qwen3.5-architecture hybrid to count,
per metadata build, which branch ran and — on the mixed branch — whether spec_token_indx was the
identity, i.e. whether the speculative tokens were actually the leading tokens of the batch. The
instrument reads only spec_sequence_masks_cpu and query_start_loc_cpu, both already on the host,
so it adds no sync and cannot perturb the schedule. The additivity of the instrument against the
vendored file was checked opcode-by-opcode and is reversible to the original sha256.

Three arms, one physical RTX 5090, MTP-3, 8 concurrent streams, adversarial-by-design traffic:

arm flags metadata builds on the gather branch misordered rate
R1 prefix caching off, --max-num-batched-tokens 2048 3,329 2,112 (63 %) 0 < 0.90 / 1000 (95 % upper bound, rule of three)
R3 prefix caching off, --max-num-batched-tokens 512 (amplifier) 8,065 6,915 (86 %) 0 < 0.372 / 1000
R2 --enable-prefix-caching --mamba-cache-mode align, 8,192-token window 5,825 2,211 3 0.515 / 1000 builds, 1.357 / 1000 gather-branch builds

Two readings, and the second is the interesting one:

  1. The batch reorder does most of the work. The gather branch is not rare — it runs on 63–86 % of
    metadata builds at 8 streams — and in 9,027 gather-branch builds without prefix caching the
    speculative tokens were leading every single time. reorder_batch_to_split_decodes_and_prefills
    moves every prefill-shaped row behind the speculative decodes, including a brand-new short prompt
    (verified by executing the image's own reorder over seven hand-built compositions).
  2. What the reorder does not protect against is a non-speculative row sharing a low region with a
    speculative one.
    All three events had the same composition: six speculative decodes and one
    non-speculative row, with the first misordered token at index 20 — tokens 0–19 were five
    speculative decodes of four tokens, token 20 began a four-token non-speculative row, and the sixth
    speculative decode's four gate rows sat behind it and were read from the wrong tokens
    (max_displaced_spec_tokens = 4).

The mechanism that supplies the pseudo-speculative row is the scheduler's speculative padding: a
request admitted needing exactly one new token is padded to 1 + num_spec_tokens and
[-1] * num_spec_tokens is written into scheduled_spec_decode_tokens
(vllm/v1/core/sched/scheduler.py:865-878 and :1052 in our build), which makes the runner classify
it as speculative (gpu_model_runner.py:2240-2249) while the reorder leaves it in region 1 because
it is not done prefilling. A full prefix-cache hit is exactly how num_new_tokens == 1 arises, so
this needs prefix caching on — which is why R1 and R3, with the cache off, are clean, and why we
would expect the rate to be workload-dependent rather than fixed. The non-speculative row ahead of it
was a chunked prefill whose final chunk was 2–4 tokens.

We also predicted, from source, a second mechanism that needs no prefix caching — a speculative
decode clamped by --max-num-batched-tokens to exactly one token, which silently reclassifies it as
non-speculative (scheduler.py:511-518 then :637-642) — and gave it a 4x amplifier in R3. It
produced zero events in 9,027 gather-branch builds (95 % upper bound 0.332 per thousand), so on
this build it is bounded-rare rather than shown reachable. Recording that because it is the mechanism
that would matter on ordinary traffic, and our measurement does not support claiming it.

What we deliberately do not claim: that those three miscomputed forward passes changed any
emitted token. Your own measured per-event effect (mean absolute chosen-logprob error 0.002755) is
about 30x below our run-to-run floor on this build (0.08231278, two identically-flagged fresh
servers), so an A/B logprob comparison at 8 streams is pre-determined to report "below resolution"
whether the module matters or not. We therefore measured whether the defective path executes, and
report only that.

Relevant to the neighbouring open issues in this family, all of which turn on a row being classified
as a decode when it is not: #49918, #51562, #47123.

Method, counts, the pre-registered decision rule (fixed before the GPU window opened), the reorder
simulation and the raw per-worker dumps: receipts/gdn-gate-concurrency.json and
receipts/gdn-gate-raw/ in https://github.com/malaiwah/qwen38-27b-exl3. Build is a downstream fork
(0.11.2.dev280+…20260810.r34, upstream integration tree 4d006a43) that predates this PR;
gdn_attn.py, flashinfer.py, backend.py and gpu_model_runner.py are unmodified vendored
upstream code there, and this PR does not touch gdn_attn.py, so the batch composition we measured
is the same patched or unpatched.

Unrelated to this PR but from the same measurement campaign, in case the flags look familiar: the align-mode admission livelock we hit at a 262 k window is #52520#52520.

jsboige added a commit to jsboige/vllm that referenced this pull request Aug 20, 2026
…A/B profile + bench script

Overlay: single-file COPY of the GDN linear-attn module with upstream PR
vllm-project#51812 applied (align a/b gate gathers with spec_token_indx — correctness
fix for MTP on hybrid GDN, absent from v0.27.1; patch applies cleanly to
the v0.27.1 blob, pure Python, static sanity check at build).

Profile: image/model/kv-dtype/mtp-tokens parameterized for the 4-variant
same-night A/B grid (fp8+MTP3 GS-32 / MTP-8 / TQ+MTP-3+canary / GS-128
philbert440). Bench: ab_bench.py — canary, acceptance (pre/post load),
single-stream 3x300, N=16 aggregate.

Co-Authored-By: Claude-Code <noreply@anthropic.com>
jsboige added a commit to jsboige/vllm that referenced this pull request Aug 20, 2026
…ut, GS-128 no gain, noise dominates

Bracket 3.6-A(462)->3.6-B(844) N=16 t/s over one hour invalidates
cross-model comparison this window; machine regression shown FLUCTUATING
(844 = 88% of the 08-14 reference at 19:26). Correctness findings hold
regardless of noise: turboquant_k8v4+MTP-3 produces silently degenerate
output on stock v0.27.1+vllm-project#51812 (canary r4g 0.94+, sample captured);
MTP-8 halves acceptance (0.22) and single-stream vs MTP-3; GS-128 brings
no gain over GS-32 (Todd-gap hypothesis eliminated).

Co-Authored-By: Claude-Code <noreply@anthropic.com>
kevinhirsch added a commit to kevinhirsch/vLLM-2080Ti-Definitive-Kevin that referenced this pull request Aug 24, 2026
…vllm#51812)

In a mixed batch where non-spec tokens precede spec tokens,
GdnLinearAttention._forward_core's spec-decode path gathered mixed_qkv
(and therefore q/k/v) by spec_token_indx but passed the raw, un-indexed
a/b sigmoid gate tensors into fused_sigmoid_gating_delta_rule_update.
Row i of q/k/v then no longer described the same original token as
row i of a/b -- the fused recurrent update applied gate values from
the wrong token. Upstream reproduced this at a max_model_len boundary
with 2 MTP draft tokens on Qwen/Qwen3.5-2B; the boundary-condition
framing (short remaining context, mixed non-spec/spec batch) is
generic and plausible under our 524K-context, many-concurrent-request
agentic workload.

Fix: compute a_spec/b_spec via the same .index_select(0,
spec_token_indx) already used for mixed_qkv_spec, in both branches of
the "1. Convolution sequence transformation" section, and use them in
the "2.1: Process the multi-query part" call instead of raw a/b.

Note on file path: upstream's diff (and this task's brief) named
vllm/model_executor/layers/mamba/gdn/qwen_gdn_linear_attn.py /
vllm/v1/attention/backends/gdn_attn.py respectively; this fork predates
upstream's gdn/ subdirectory split and keeps the layer at
vllm/model_executor/layers/mamba/gdn_linear_attn.py (gdn_attn.py is the
separate attention-*backend* metadata-builder file, home of our
#51508 port -- untouched here, next in the harvest queue for
#52078/#53077). Verified `_forward_core` at gdn_linear_attn.py:1299 is
the sole call site matching upstream's context (only one
`q=query_spec` callsite with `a=a, b=b` in the file).

Added tests/v1/worker/test_gdn_spec_gate_alignment.py: _forward_core is
CUDA-only (Triton, causal_conv1d, forward-context), so per the sibling
#51508 port's precedent (test_mamba_stale_rows.py), this replicates the
fixed indexing expression on small CPU tensors as an alignment
contract test, plus a companion test proving the pre-fix (raw a/b)
behavior actually misaligns on a mixed non-spec-then-spec batch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Aiakos1818 pushed a commit to Aiakos1818/vllm that referenced this pull request Sep 14, 2026
Ports of upstream PRs that apply to this fork's Qwen3.8-27B GDN + MTP3
stack. Each was rebased by hand onto v0.27.1 + the sm75/qwen3.8 base.

- vllm-project#51812 [Bugfix] Align Qwen GDN gates with speculative tokens (v0.28):
  gather `a`/`b` with the same `spec_token_indx` as mixed_qkv before the
  fused recurrent update, so a mixed batch (non-spec tokens ahead of spec
  tokens) no longer feeds another token's gates to the kernel.
- vllm-project#56196 [Bugfix] Mamba: store a short prefill chunk's conv state in its
  own block: with prefix caching a chunk shorter than the conv state wrote
  its new state through the pointer it read the initial state from,
  overwriting the shared prefix block. The destination block is now
  resolved once from `block_idx_last_scheduled_token` and used in every
  branch. Ported with its regression test.
- vllm-project#49436 [Perf] 3D-grid tiling of the state-copy Triton kernels (v0.28):
  new `_memcpy_u64_tiled` head/body/tail device function partitions the
  temporal state's u64 body across `_TEMPORAL_TILES=16` CTAs (small-batch
  HBM fill), lifts the 8B-alignment assert to a warning, and collapses to
  a single-CTA copy for conv states. Ported with its tests.

Not ported: vllm-project#52789 (internal prefill checkpoints for Mamba prefix
caching). It is Kimi-K3 KDA only — `num_prefill_checkpoint_blocks` is set
solely by `vllm/models/kimi_k3/nvidia/kda.py` under the flashkda backend,
so the scheduler/manager infrastructure is inert for GDN models. The
analogous capability here is the durable-anchor work in the kv commit.

Tests: 149 CPU (host-tier/spill/prefix/mamba-align/api) pass; kernel
suites pass on the 2080Ti (memcpy 120, precopy 75, causal_conv1d 156; the
8 float64-reference `varlen` failures are pre-existing on this machine and
fail identically without the port). The vllm-project#56196 test fails on the unpatched
kernel and passes with it.

E2E on the 9.6e9 profile (real NVMe SSD tier): S resident 384,704 ->
V0 deep revert cached 350,400 -> T evict -> R SSD restore cached 382,400
(sha db8b8e836881534b, same as the pre-port baseline) -> V2 post-restore
revert cached 350,400; stores=3 restores=2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working qwen Related to Qwen models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants