Skip to content

Support thinking budgets during MTP speculative decoding - #2230

Open
YuhuaBillChen wants to merge 3 commits into
Blaizzy:mainfrom
YuhuaBillChen:fix/mtp-thinking-budget-speculative
Open

YuhuaBillChen wants to merge 3 commits into
Blaizzy:mainfrom
YuhuaBillChen:fix/mtp-thinking-budget-speculative

Conversation

@YuhuaBillChen

@YuhuaBillChen YuhuaBillChen commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

The server currently rejects thinking_budget when an MTP drafter is enabled. This change allows MTP to use the existing ThinkingBudgetCriteria: it shortens verified blocks when the criterion requests a forced token, then feeds the closing sequence through the model so generation and cache state remain consistent.

For targets requiring uniform batch acceptance, a boundary in one row limits the commit depth for every row. Token controls observe only the retained output. Requests without a thinking budget keep the existing fast path. DFlash and Eagle3 retain their existing rejection.

The existing criterion's counting and closing-token semantics are unchanged; this is not a new definition of an exact reasoning-token limit.

Validation

  • Re-ran test_generate.py, test_speculative.py, and test_server.py on M4 Pro: 1,269 passed, with no skipped or deselected tests. The previously reported NVFP4 failures did not reproduce in this run.
  • All seven changed files in the tested source copy match PR head 5fa214f6 by SHA-256.
  • Gemma 4 31B MXFP4 with the Gemma 4 31B QAT assistant MXFP4, MTP block size 3:
    • B1 without a budget: 32 generated tokens, MTP activity confirmed.
    • B1 with budget 8: 32 generated tokens; response contains a shortened thought channel and subsequent final content.
    • B2 with budgets 8 and 20: concurrent requests each generated 48 tokens, both returned HTTP 200 and subsequent final content, without a reported cache/rollback error. This exercises a target requiring uniform batch acceptance.
    • Explicit markers: thinking_start_token="<|think|>", thinking_end_token="<channel|>", enable_thinking=true. Counting starts from the prompt marker and includes the generated channel header.
  • Downstream Qwen3.8-27B MXFP4 server logs corroborate completed generation, APC-hit repeats, and overlapping requests. These used a downstream runtime rather than an otherwise unmodified PR checkout. The earlier exact 512-reasoning-token claim is omitted because the raw response/token trace was not recovered during this audit.

The Gemma runs are short functional smoke tests and ended at their output caps. They do not establish long-generation stability, answer accuracy, or a throughput improvement. Exact forced-token boundaries are covered by focused tests; the HTTP smoke responses alone do not provide a token-level proof.

@pierre427

Copy link
Copy Markdown

One idea that would remove the zero-accept rounds. Right now a forced token is only injected at the start of the next round with accepted_list[row] = 0, so each token of the close sequence costs one full verify, and with uniform acceptance that sets every row to 0 for that round. If the budget exposed a pure lookup, forced_token(prefix_ids) -> Optional[int], _apply_mtp_token_controls could resolve it inside the current round:

speculative/mtp.py, per row (non-uniform case)

hist = history[original_row] # committed ids for this row
for p, tok in enumerate(new_tokens):
f = budget[original_row].forced_token(hist + new_tokens[:p])
if f is not None and tok != f:
new_tokens_list[row] = new_tokens[:p] + [f] # f replaces the target sample
accepted_list[row] = p # d1..dp stay committed
break

A draft that already equals the forced token is simply accepted. A mismatch looks exactly like an ordinary rejection with a replacement token, so the cache commit path doesn't change. Because the lookup has no state, rollback and EOS truncation can't desync it. For uniform acceptance you'd still take the min across rows, as now.

We ship this pattern downstream as a logits processor (ThinkingBudgetProcessor, tests), including multi-token close markers. To be upfront: that coverage is CPU/unit-level, and we haven't GPU-qualified it under MTP.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants