[Feat] Support thinking_token_budget in Model Runner V2 - #46727
Conversation
|
Thanks @chaunceyjiang, at first glance this looks pretty good, I'll try to take a closer look soon! |
NickLucche
left a comment
There was a problem hiding this comment.
thanks @chaunceyjiang I think this is looking clean
njhill
left a comment
There was a problem hiding this comment.
Thanks @chaunceyjiang!
I think the other thing we should aim to improve is the scan of whole context each time where we only need to consider new tokens.
I pushed a draft change for this here, not sure about it yet though: njhill@f2d3627 |
444bc29 to
3f90e27
Compare
| last_start = -1 | ||
| last_end = -1 | ||
|
|
||
| for i in tl.range(scan_pos, total_len): |
There was a problem hiding this comment.
From what I understand, during prefill, if we're processing a long context (e.g., ~128k tokens), we currently perform a forward scan over the entire prompt to locate the <START_THINKING> and <END_THINKING> tokens. In the common case where the <START_THINKING> token appears near the end of the prompt template, this results in an unnecessary O(128k) forward scan.
One potential optimization would be to search for the thinking boundary tokens in reverse instead. Most reasoning prompt templates place the <START_THINKING> and <END_THINKING> instructions towards the end of the prompt, so a reverse scan would likely find them much sooner and reduce the amount of work performed during prefill. This would probably help lower the TTFT overhead at large context.
cc: @njhill
|
Hi all, This PR is causing issues with the thinking token phrase leaking into the chat. V1 previously worked around this but it looks like after X tokens even if we aren't in a reasoning block the phrase will leak out into the main response. |
|
@dafeliton, could you provide the steps to reproduce this issue? |
|
@chaunceyjiang Sure. With gemma 31b, add these args: Then set the reasoning budget to 1500 and tell the model to output something long such as a 50 paragraph essay. The reasoning end str appears in the main response. There were previous PRs for V1 that alleviated this issue. Sample: To find the enthalpy of neutralization, we first determine the total amount of 1. Calculate moles of
2. Calculate moles of
3. Identify the limiting reactant 4. Calculate the total enthalpy Correct Option: |
- Clamp budgets to int32 range in add_request; oversized request values previously raised numpy OverflowError, crashing the engine - Match the forced-end prefix purely on content so a resumed prompt ending with a partial end marker continues it instead of duplicating its first token (prompt_len plumbing removed from the kernel as no longer needed) - Vectorize the cold marker scan in blocks of 1024 positions (~34x faster worst-case cold resume at 64k history) - Only copy budgets to UVA when a value actually changed, instead of every scheduling step Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Nick Hill <nickhill123@gmail.com>
9aa4d54 to
1752c78
Compare
njhill
left a comment
There was a problem hiding this comment.
Thanks @chaunceyjiang! I pushed one more commit with some additional fixes and optimizations.
|
Hi @chaunceyjiang, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
1 similar comment
|
Hi @chaunceyjiang, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
|
/ci run |
|
✅ Triggered Buildkite CI #82766 for commit |
This fork and upstream independently wrote vllm/v1/worker/gpu/sample/ thinking_budget.py in the same week: 9fedae272b created it here on 2026-08-06, and upstream PR vllm-project#46727 (72c0d67) created the same path on 2026-08-07, arriving in the rebase. Neither derives from the other, and the rebase kept ours while taking upstream's test file, so upstream's 12 tests ran against an API they were never written for. Comparing the two on the behaviour they share, upstream's is the better implementation and this commit takes it wholesale. Pattern matching. Ours advanced a KMP cursor with no failure table, so a marker with a border is missed outright: pattern [1,1,2] in [7,1,1,1,2,9] -> ours [] upstream [2] pattern [1,2,1] in [7,1,2,1,2,1,9] -> ours [1] upstream [1,3] Every marker shipped today (<think>, <|START_THINKING|>, <mm:think>) is border-free, so this was latent rather than live, but upstream matches the full pattern at each candidate position and cannot express the bug. Natural vs forced end markers. Upstream tracks natural_reasoning_end_ token_ids separately from reasoning_end_token_ids, so a parser whose own end marker differs from the configured forced one still terminates detection correctly. Ours conflated the two. Resumed prompts. Upstream derives how much of the end sequence a resumed prompt already emitted and continues from the next marker token, rather than relying on incremental device state surviving preemption. Host synchronisation. Upstream forces inside the kernel. Ours copied three state arrays to host and rebuilt the forced rows in Python, one .item() per row, on every step that forced. Ours was also carrying a self-inflicted defect that upstream's design cannot have: eleven kernel-written state tensors, whose host arrays are not aliased to device memory, needing a bespoke StagedWriteTensor to avoid being clobbered on flush. Upstream keeps three cached scalars. The fork's own extensions -- the logits-processing gate fix, the hesitation-marker penalty, and the answer reserve -- are re-applied on top of this base in the following commit, where they read as a small reviewable delta against upstream instead of as a parallel universe. tests/v1/worker/gpu/sample/test_thinking_budget.py asserted the removed internals throughout and is deleted here; it is rewritten against the new base in the next commit. Upstream's tests/v1/worker/test_gpu_thinking_ budget.py passes 12/12 at this commit. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Naadir Jeewa <naadir@randomvariable.co.uk>
…#46727) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Signed-off-by: Nick Hill <nickhill123@gmail.com> Co-authored-by: Nick Hill <nickhill123@gmail.com>
…#46727) Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Signed-off-by: Nick Hill <nickhill123@gmail.com> Co-authored-by: Nick Hill <nickhill123@gmail.com>
In some quantized models, such as GLM-5.2 or Qwen quantized models, the model may generate long reasoning traces.
Purpose
Support thinking_token_budget in Model Runner V2
Test Plan
see e2e
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)