Skip to content

[Bugfix][DSv4] Keep indexer scoring in breakable graphs - #52492

Merged
zyongye merged 2 commits into
mainfrom
bugfix/dsv4-breakable-indexer-shortcut
Aug 17, 2026
Merged

zyongye merged 2 commits into
mainfrom
bugfix/dsv4-breakable-indexer-shortcut

Conversation

@LucasWilkinson

@LucasWilkinson LucasWilkinson commented Aug 16, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

#49486 added a host-side short-context shortcut that skips learned indexer scoring. #51430 later moved DeepseekV4Indexer.forward into the captured region.

Breakable PIECEWISE capture uses short dummy metadata, so the shortcut gets baked into a graph that may later replay for a long cached prefix. Above 2048 tokens, C4 layers have more than 512 candidates and must score them; the captured shortcut instead selects candidates 0..511.

Do not take the shortcut while the CUDA stream is capturing. Eager execution retains it.

#52401 does not cover this path: it widens the eager region for Model Runner V1, while DSpark forces Model Runner V2.

Related to #52448.

Duplicate-work check

The required issue and open-PR searches found no matching fix. #51318 is different: it reverts adaptive C128A metadata packing. This C4 failure still reproduces on main at 8efa13b700, which includes #51318 and #52401.

Validation

.venv/bin/python -m pytest tests/v1/cudagraph/test_breakable_cudagraph.py -q
14 passed

.venv/bin/pre-commit run --files vllm/models/deepseek_v4/attention.py
All applicable hooks passed, including Ruff and Python 3.10 mypy.

DeepSeek-V4-Flash-0731 on 2x NVIDIA B200, TP2 + EP, DSpark-7, prefix caching, and FULL_AND_PIECEWISE breakable CUDA graphs:

Prompt length Unpatched main Patched
6144 E, to, to E, E, E
6543 The, Let, Let The, The, The

The exact #52448 workload also completed 8 waves of 32 concurrent requests with 0/256 length-cap runaways.

AI assistance

AI assistance was used for investigation, patch preparation, validation, and this description. The human submitter has reveiwed.

@mergify mergify Bot added nvidia bug Something isn't working labels Aug 16, 2026
Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
@LucasWilkinson
LucasWilkinson force-pushed the bugfix/dsv4-breakable-indexer-shortcut branch from 5428fde to 2b0be22 Compare August 16, 2026 04:53
@LucasWilkinson
LucasWilkinson marked this pull request as ready for review August 16, 2026 14:13

@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 repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@LucasWilkinson

Copy link
Copy Markdown
Collaborator Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #84093 for commit 2b0be22191a4.

@aoshen02

Copy link
Copy Markdown
Collaborator

Confirm when implementing Batch invariance with dpsk v4 flash base.

@aoshen02 aoshen02 added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 16, 2026
@github-project-automation github-project-automation Bot moved this to Ready in NVIDIA Aug 17, 2026
@zyongye
zyongye enabled auto-merge (squash) August 17, 2026 02:04
@zyongye

zyongye commented Aug 17, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #84134 for commit e6a64bff4011.

@zyongye
zyongye merged commit 292187d into main Aug 17, 2026
64 checks passed
@zyongye
zyongye deleted the bugfix/dsv4-breakable-indexer-shortcut branch August 17, 2026 03:01
@github-project-automation github-project-automation Bot moved this from Ready to Done in NVIDIA Aug 17, 2026
aoshen02 added a commit to aoshen02/vllm that referenced this pull request Aug 18, 2026
The pin dropped any piecewise mode to FULL under VLLM_BATCH_INVARIANT, on the
premise that a per-step mode chosen from batch properties cannot be invariant.
That premise was measured and does not hold here: with the indexer's
short-context predicate no longer frozen against the capture-time dummy batch
(upstream vllm-project#52492), a 50-round soak of the full DeepSeek-V4-Flash-Base under
FULL_AND_PIECEWISE is bitwise clean, where the same soak leaked in 38.7% of
rounds before. Keeping the pin now costs the mixed-step graph for a guarantee
it was not providing.

So the mode is left alone and the warning carries what the pin used to: a
piecewise mode is invariant only while every path it can select is bitwise
equal for that model, which is verified for DeepSeek-V4 and unknown elsewhere.

Removed with it:
- the refusal for configurations that can use neither piecewise nor full,
  which existed only because the pin would otherwise undo their downgrades;
- `_full_cudagraph_unsupported_reason`, whose only caller that refusal was.
  vLLM's own downgrades to piecewise now stand unopposed, which is what they
  were written to do;
- the batch-invariant branches in `resolve_cudagraph_mode_and_sizes`, so those
  fallbacks resolve identically with and without invariance. Two tests now
  assert exactly that, rather than asserting the divergence.

Kept: every check that does not depend on the pin -- the capture-set escape
warning, and the refusals for microbatching, prefill context parallel, the
multimodal encoder dispatcher and the speculative-decode dispatchers. Those
close paths where a graph is chosen per step without consulting
cudagraph_mode, which no mode setting can fix.

Tested: tests/test_config.py -k the cudagraph/batch-invariant selection,
13 passed. Negative control: re-inserting the pin fails exactly the two
mode-survives cases and nothing else.

Not claimed: the soak that justifies this ran with the predicate bounded on a
static length, so both paths did full scoring. vllm-project#52492 instead keeps the eager
shortcut, and graph and eager then agree only while "candidates <= topk" makes
the shortcut's set equal to the real top-k -- an equal set is not an equal
order, and the order sensitivity of FlashMLA sparse decode is unresolved in
our measurements. A deployment enabling piecewise on top of vllm-project#52492 should
re-run that soak rather than inherit this result.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
aoshen02 added a commit to aoshen02/vllm that referenced this pull request Aug 18, 2026
… FULL pin is gone

The base moved to upstream aa99034 -- the commit the runtime image is built
from -- and upstream had done two of these jobs for us in the meantime: vllm-project#52492
fixes the indexer short-context predicate frozen at capture (our
bi/indexer-shortcut, now deleted), and vllm-project#51318 removed the adaptive C128A
metadata packing, which retires the width pin.

The cudagraph_mode=FULL pin is also gone: with the predicate fixed, the
production default runs 50/50 clean with both capture kinds present, so the PR
that carried the pin now closes the dispatchers cudagraph_mode cannot reach and
warns about the one choice that stays.

Also replaces the "pinning FULL costs nothing" paragraph with the per-branch
attribution of the +23% operator term, and marks NCCL_MNNVL_ENABLE=0 as the
node-local workaround it is rather than a batch-invariance requirement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lukealonso pushed a commit to local-inference-lab/vllm that referenced this pull request Aug 21, 2026
…#52492)

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: Yongye Zhu <zyy1102000@gmail.com>
zufangzhu pushed a commit to zufangzhu/vllm that referenced this pull request Aug 24, 2026
…#52492)

Signed-off-by: Lucas Wilkinson <lwilkins@redhat.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: Yongye Zhu <zyy1102000@gmail.com>
Signed-off-by: Zhu, Zufang <zufang.zhu@intel.com>
MykhailoTamarin added a commit to MykhailoTamarin/vllm-starter that referenced this pull request Aug 25, 2026
## Summary

Two EXL3 serving fixes on the DSPark DeepSeek-V4 image (`images/vllm-0_26_0-exl3/`), plus supporting config/version bumps.

### 1. Guard short-context topk skip against CUDA graph capture
`patch_attn_short_ctx_topk.py` — backport of upstream vLLM [PR #52492](vllm-project/vllm#52492) (guard for #49486).

The short-context all-candidates shortcut must not run while a CUDA stream is capturing: a graph captured on short dummy metadata bakes the skip in and replays it against longer cached prefixes (>2048 tokens), selecting candidates `0..topk-1` unscored and landing attention on the wrong context slices.

EXL3 runs FULL cudagraph (`VLLM_USE_BREAKABLE_CUDAGRAPH=0`) with `--cudagraph-capture-sizes 1` and `--enable-prefix-caching`, exactly the vulnerable configuration. Live serving is unaffected (capture happens once at boot); the #49486 TTFT win is kept for all real requests.

Change: added `and not torch.cuda.is_current_stream_capturing()` to the H3 short-context condition. `py_compile` OK.

### 2. Narrow `)Skip` artifact strip (replaces the wide version)
New `patch_strip_skip_artifact.py` — removes only the exact DSPark corrupt-draft token `)Skip` (vocab id 83480) from chat content, plus a single adjacent space.

The earlier wide patch removed a broad punctuation set (`,Skip` `.Skip` `.skip` `' Skip`) and globally collapsed multi-spaces, corrupting legit `Skip` words and agent/tool-written content (YAML, code). This version is deliberately narrow: bare `Skip`/`skip` (legit English), other punctuation forms, and unrelated whitespace are untouched.

Applied on both paths: streaming per-chunk deltas and non-streaming content, parser + plain; reasoning untouched. Bare string ops only — no `re` import needed in `serving.py`.

### Supporting changes
- `Dockerfile`: wires in the new `patch_strip_skip_artifact.py` build step
- `models/deepseek-v4-flash-0731-exl3-dspark.yaml`: image bump `vllm-0_26_0-exl3-0.1.8 → 0.1.9`, `--gpu-memory-utilization 0.91 → 0.9`
- `README.md`: max concurrency updated `2.33x → 1.97x` to match the GMU change
- `images/vllm-0_26_0-exl3/README.md`: document patch #18
- `models/logs/deepseek-v4-flash-0731-exl3-dspark.log`: refreshed startup log

## Testing
- `py_compile` passes for both modified/new patches
- Validated locally via `./vllm-manager.sh --local start --model deepseek-v4-flash-0731-exl3-dspark` (flags parsed)
- DRY_RUN only — no remote/real docker commands run

---------

Co-authored-by: Your Name <you@example.com>
Co-authored-by: Pi <pi@mikhei777.org.ua>
Reviewed-on: https://git.mikhei777.org.ua/admin/vllm-starters/pulls/79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working nvidia ready ONLY add when PR is ready to merge/full CI is needed

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants