Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Thanks for splitting this out as a standalone PR and for the I verified a build carrying this fix on 8×H200 (SM90) with GLM-5.2-NVFP4 — TP8 / DCP2 / EP8 / MTP spec=5,
Full per-request results are in the #46514 thread: #46514 (comment). Happy to re-run against this PR's branch directly if it'd help review. |
|
Quick follow-up with DCP=4 data, in case it's useful for the review. I also ran the fix under DCP=4 on the same 8×H200 node (TP8 / DCP4 / EP8 / MTP spec=5, The fix held up cleanly across two benchmarks:
No |
bbcc337 to
a5c8978
Compare
…'s block-table width The DSA indexer decode path preallocates expanded_block_table_buffer with cdiv(max_model_len, block_size * total_cp_world_size) columns (per-rank, CP-divided), but the block table the runner hands in is sized independently and does not apply the CP divisor: its global width comes from the placeholder InputBatch built before KV-cache init, whose rows are cdiv(max_model_len, block_size) (gpu_model_runner.py), and that InputBatch is only rebuilt when the block-size lists disagree. On TP8/DCP2 at 1M context the two widths differ by 2x (buffer 8192 vs table 16384), and the variable-length decode path raises RuntimeError: The expanded size of the tensor (8192) must match the existing size (16384) at non-singleton dimension 1 while the uniform path is worse: _prepare_uniform_decode_kernel's copy loop is bounded by the destination stride, so it silently truncates or mixes block-table rows. Kernel-block splitting (blocks_per_kv_block > 1) and BlockTable alignment padding are a second, independent source of the same mismatch that this also guards against. Match the buffer to the observed block-table width instead, reallocating at most once during the first (warmup) build, before CUDA-graph capture. Reported-by: rikki in vllm-project#46514 Co-authored-by: Claude Signed-off-by: Mikhail Kostryukov <mike@triptrack.net>
a5c8978 to
96d4bc1
Compare
Guards the CP-divisor width mismatch: _expanded_block_table must resize the buffer to the runner's actual block-table width, else the variable-length decode path crashes and the uniform path silently truncates. CPU-only, via a lightweight stand-in (only expanded_block_table_buffer/device are touched). Co-authored-by: Claude Signed-off-by: Mikhail Kostryukov <mike@triptrack.net>
|
@pavanimajety could you take a look? Small bugfix: without it batch serving hits |
|
@MatthewBonanni you were last in indexer.py (#47327). Standalone bugfix: the indexer sizes its expanded block table from its own buffer instead of the runner's block-table width, so batch serving dies with EngineDead at concurrency ≥8. Verified by @rikki on 8×H200 with and without DCP, regression test included. |
…d-block-table Signed-off-by: Mikhail Kostryukov <mike@triptrack.net>
…'s block-table width The DSA indexer decode path preallocates expanded_block_table_buffer with cdiv(max_model_len, block_size * total_cp_world_size) columns (per-rank, CP-divided), but the block table the runner hands in is sized independently and does not apply the CP divisor: its global width comes from the placeholder InputBatch built before KV-cache init, whose rows are cdiv(max_model_len, block_size) (gpu_model_runner.py), and that InputBatch is only rebuilt when the block-size lists disagree. On TP8/DCP2 at 1M context the two widths differ by 2x (buffer 8192 vs table 16384), and the variable-length decode path raises RuntimeError: The expanded size of the tensor (8192) must match the existing size (16384) at non-singleton dimension 1 while the uniform path is worse: _prepare_uniform_decode_kernel's copy loop is bounded by the destination stride, so it silently truncates or mixes block-table rows. Kernel-block splitting (blocks_per_kv_block > 1) and BlockTable alignment padding are a second, independent source of the same mismatch that this also guards against. Match the buffer to the observed block-table width instead, reallocating at most once during the first (warmup) build, before CUDA-graph capture. (cherry picked from commit 96d4bc1 of vllm-project#48404) [backport] Resolved against local-inference-lab/vllm dev/gilded-gnosis: that base guards the uniform-decode branch with an extra `num_decodes * max_decode_len == num_decode_tokens` cudagraph token-padding condition which upstream main does not have. Kept the base's condition and layered the width-matched accessor on top; the variable-decode branch applied unchanged. Reported-by: rikki in vllm-project#46514 Co-authored-by: Claude Signed-off-by: Mikhail Kostryukov <mike@triptrack.net>
…d-block-table Signed-off-by: Mikhail Kostryukov <mike@triptrack.net>
|
@MatthewBonanni Would your please take a look at this pr? |
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
|
Thanks for the fix! I pushed some changes but the design still needs a bit of thought. Will revisit tomorrow |
|
This pull request has merge conflicts that must be resolved before it can be |
|
@MatthewBonanni Since #48404 is the companion width-fix for #46514, could you take a review pass on #46514 too while you're here? It rides on your #48642 dense-MHA gather; @Leoyzen has been running it in prod on 4×H200 for a week, rikki validated #48404 on 8×H200, and our battery is green — MERGEABLE and CI-green. |
|
@MatthewBonanni ok, sorry for this. My English not as good as i want :) |
|
I actually ended up taking a different approach. Superseded by #50302. I appreciate the contribution though! |
Pairs with b12x cb4920c: the sparkinfer -> b12x rename is breaking, so
both repos must move together. Brings native VLLM_NVFP4_MLA_SCALES_FILE
(previously ours), CKV prefetch, DCP query-split, and the b12x rename
that retires our stale SPARKINFER_* env names.
Conflict resolutions (6 files, 13 hunks):
- envs.py, b12x_mla_sparse.py, test_sparse_attn_indexer_b12x.py: purely
additive on both sides (empty merge base), unioned. Ours adds
VLLM_GLM_FP8_{DENSE,KERNEL}, VLLM_V2_ALLOW_SEQUENCE_PARALLELISM,
VLLM_B12X_BQ4_{PREFILL,CAPACITY} and the BQ4 workspace; upstream adds
the NVFP4 MLA scales env pair, CKV prefetch, and its own tests.
- indexer.py: upstream replaced our cdiv() with
get_indexer_max_num_blocks_per_req(), which is DCP-layout aware. Took
theirs -- the five trailing args are upstream's, so keeping our cdiv()
would have passed 5 args to a 2-arg function. Kept our comment; our
vllm-project#48404 fix lives in _expanded_block_table() and is untouched.
- mla.py (7 hunks): all the same collision -- our b4f7587
de-underscoring vs upstream's underscored names plus reflowed
f-strings. Hunk 1 also carried a real semantic change (_KV_FP8_ROPE
now derives from NVFP4_MLA_CACHE_FORMAT.fp8_rope, not a raw getenv).
Took upstream wholesale to preserve that, then re-applied our naming
as one systematic pass rather than hand-merging seven hunks.
- test_b12x_mla_fp8_rope_writer.py: took theirs, naming fixed by the
same pass.
Dropped the now-redundant _NVFP4_MLA_SCALES_ENV alias: upstream imports
the public NVFP4_MLA_SCALES_ENV and the f-strings already use it.
Syntax-checked only. NOT built or tested.
3 participants
Purpose
The DSA indexer's decode path preallocates
expanded_block_table_bufferwithcdiv(max_model_len, block_size * get_total_cp_world_size())columns (per-rank,CP-divided), but the
block_tablethe runner hands in does not apply the CPdivisor: its width comes from the placeholder
InputBatchbuilt before KV-cacheinit (
cdiv(max_model_len, block_size),gpu_model_runner.py:688), which isonly rebuilt when the block-size lists change. On TP8 / DCP2 / 1M the two widths
differ by 2× (buffer 8192 vs table 16384):
RuntimeError: The expanded size of the tensor (8192) must match the existing size (16384) at non-singleton dimension 1;_prepare_uniform_decode_kernel's copy loop isbounded by the destination stride, so it silently truncates/mixes rows instead
of raising.
Kernel-block splitting and
MultiGroupBlockTablealignment padding are a second,independent source of the same mismatch. The path is reachable without #46514
(the FlashInfer sparse DCP builder shares the buffer), hence a standalone fix.
Fix
_expanded_block_table(width)matches the buffer to the block-table widthactually presented, reallocating at most once during the first (warmup) build —
before CUDA-graph capture, so the address stays stable across capture/replay.
Both decode use-sites go through it.
Not a duplicate
Distinct root cause from the other open block-table-width fixes; none close the
CP-divisor gap:
max_model_lenauto-reduced afterInputBatchcreation): the widths come from two different
max_model_lenvalues and arefixed by rebuilding
InputBatch. Heremax_model_lenis unchanged, and thedivisor-less formula would not close the gap on rebuild.
keeps
* get_total_cp_world_size(), so on DCP the buffer stays 8192.Adapting to the delivered width also covers those triggers, but the change is
scoped to the indexer and composes with those PRs. The two width formulas could
instead be reconciled at the source (CP divisor on the placeholder
InputBatch);happy to move it there if preferred.
Testing
Reported and validated by @rikki on 8×H200 — TP8 / DCP2 / EP8 / MTP=5,
fp8_ds_mla, 1M max-len, imagepr46514-fresh-20260712-idxfix:max_tokens, ~524K context— 0 errors (previous image crashed in ~12 min with
[8, 8192] vs [8, 16384]);max_tokens=512, UUID needle in ~524K context — 4/4 retrievedverbatim.
Table + raw JSON: #46514 (comment)
Local CPU:
tests/v1/attention/test_indexer_dcp_localize.py+test_sparse_mla_backends.py→ 15 passed / 18 skipped (parity with base); fullGPU run left to CI.
AI assistance was used (Claude); the submitter reviewed every line.
Tested-by: @rikki