Skip to content

[Bugfix] Sparse MLA: enable fp8_ds_mla dense prefill - #48642

Merged
MatthewBonanni merged 4 commits into
vllm-project:mainfrom
MatthewBonanni:fix-sparse-mla-fp8-dense-prefill
Jul 16, 2026
Merged

MatthewBonanni merged 4 commits into
vllm-project:mainfrom
MatthewBonanni:fix-sparse-mla-fp8-dense-prefill

Conversation

@MatthewBonanni

@MatthewBonanni MatthewBonanni commented Jul 14, 2026

Copy link
Copy Markdown
Member

Purpose

PR #47327 added dense-MHA prefill routing for sparse MLA, exposing two fp8_ds_mla mixed-batch bugs:

  • req_id_per_token could exceed the top-k tensor length and cause out-of-bounds writes.
  • Dense MHA could not gather the packed 656-byte FP8 cache, and FlashMLA assumed its metadata covered the full batch.

This PR bounds the converter input, adds arbitrary sequence starts to the packed FP8 gather/upconvert kernel, and supports decode-only MQA metadata while dense MHA handles prefills. It supersedes #48612.

AI assistance was used. The human submitter reviewed the diff.

Tests

.venv/bin/python -m pytest tests/kernels/test_cp_gather_fp8.py -q
.venv/bin/python -m pytest tests/v1/attention/test_sparse_mla_backends.py -q -k 'flashmla_fp8_paths or cache_dtype_aliases or triton_convert or forward_bf16_kv_slices'
.venv/bin/python -m pytest 'tests/v1/attention/test_sparse_mla_backends.py::test_sparse_backend_decode_correctness[1.0-1.0-64-1-fp8_ds_mla-mixed_small-FlashMLA]' -q
.venv/bin/pre-commit run --files <changed files>
  • FP8 gather: 16 passed
  • Sparse-MLA regressions: 16 passed, 515 deselected
  • Mixed decode/dense-prefill correctness: 1 passed
  • Pre-commit: passed

GSM8K

RedHatAI/DeepSeek-V3.2-NVFP4-FP8-BLOCK, 4×B200, TP2/DP2/EP, FLASHMLA_SPARSE, fp8_ds_mla, official 5-shot lm_eval, all 1,319 samples:

Routing Flexible Strict
Dense MHA 89.69% 89.54%
Forced sparse MQA 89.92% 89.69%

The difference is below the ~0.84-point standard error. No measurable accuracy regression from dense-MHA routing was observed. Throughput is not compared because scheduler batch sizes differed.

Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
@drakosha

Copy link
Copy Markdown
Contributor

Ran your branch with the #46514 DCP change on top, on 4×H200 GLM-5.2-NVFP4 (TP4/DCP4/EP, fp8_ds_mla, MTP=3, prefix caching + offload). Your dense-MHA seq_starts gather is correct under DCP: a prefix-cache-hit test (short questions reading a cached ~1.4k briefing back through the new gather) recalls every planted fact, with prefix-cache hits confirmed — the DCP path a DSV3.2 eval won't reach. Decode throughput is at parity with our gated build. The DCP merge was clean (only conflict: the req_id slice vs our DCP branch in _forward_fp8_kv_mixed_batch).

Two heads-ups:

(AI assistance used; every claim checked against source.)

@gaby

gaby commented Jul 15, 2026

Copy link
Copy Markdown

@MatthewBonanni PR #48379 was merge, this can be closed ?

@MatthewBonanni

Copy link
Copy Markdown
Member Author

@gaby, this PR is still necessary to resolve the listed issues

Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>

@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.

@MatthewBonanni MatthewBonanni changed the title [BugFix] Sparse MLA: enable fp8_ds_mla dense prefill [Bugfix] Sparse MLA: enable fp8_ds_mla dense prefill Jul 15, 2026
@mgoin mgoin added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 16, 2026

@mgoin mgoin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks reasonable to me, will rely on CI and manually unblocked tests

@MatthewBonanni
MatthewBonanni enabled auto-merge (squash) July 16, 2026 21:54
@MatthewBonanni
MatthewBonanni merged commit 67f9046 into vllm-project:main Jul 16, 2026
242 checks passed
waynehacking8 added a commit to waynehacking8/vllm that referenced this pull request Jul 17, 2026
…ect#48642)

Upstream vllm-project#48642 dropped the seq_lens parameter from
cp_gather_and_upconvert_fp8_kv_cache (total_tokens now derives from
dst.size(0)), which silently misaligned this test's positional args
after rebase.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jer3fzhUR2bNKxM1Prqnsg
Signed-off-by: waynehacking8 <waynehacking8@gmail.com>
@MatthewBonanni
MatthewBonanni deleted the fix-sparse-mla-fp8-dense-prefill branch July 20, 2026 13:54
MatthewBonanni added a commit to faresobeid/vllm that referenced this pull request Jul 23, 2026
Resolves conflicts from upstream main, which rewrote history under the
branch (18 of the branch's 19 commits are stale duplicates of commits
already on main; those files resolve to main's versions).

Real resolutions preserve the HiSparse work on top of main's changes:

- flashmla_sparse.py: rebuilt on main's dense-MHA-prefill rework
  (vllm-project#47327/vllm-project#48642) - super().build() metadata splits, metadata-based
  _build_fp8_separate_prefill_decode, MQA-token req_id slicing - while
  keeping the HiSparse coordinator, swap-in paths, and
  decode_split/prefill_split sched-metas (now fed from metadata counts).
  Chunk regains per-chunk device seq_lens for host-context staging
  (main removed the field with the op's seq_lens param).
- nixl/base_worker.py: DRAM/VRAM mem_type registration grafted onto
  main's restructured single-cache-per-layer loop;
  is_kv_layout_blocks_first -> virtually_split_kv_in_blocks.
- nixl/pull_worker.py: _remote_agents now keyed by (dp, tp) tuples.
- mla_attention.py: _prepare_hisparse_for_batch composed with main's
  PCP latent-cache gather at both KV-update sites.
- mooncake store worker: finally-based task_done accounting kept,
  main's per-request exception logging adopted; close() drains the
  kv_recv_threads list (renamed on main).
- kv_cache_utils.py: main's _get_packed_kv_cache_layout kept alongside
  the HiSparse host-pool budget helpers.
- cooperative_topk.cuh: kept the tie-overflow -1 padding hardening.
- CMakeLists.txt: main's PagedAttention removal (vllm-project#47361) kept;
  hisparse_kernels.cu block retained.

Semantic fixes for HiSparse x dense-MHA prefill co-occurrence:

- Swap-in mode in _forward_fp8_kv_separate_prefill_decode keys off
  batch composition (fp8_metadata.num_prefill_tokens), not q shape:
  mixed batches write newest rows to the host pool, so pure-decode
  slot-mapping resolution would read a stale hot-buffer slot.
- Early returns in _forward_bf16_kv / _forward_fp8_kv_mixed_batch
  compare against q.shape[0] so a decode-only q (dense-routed prefill)
  cannot run an empty prefill sub-call and zero-bind the split
  sched-metas.
- use_mha is gated off for HiSparse layers: the dense prefill's
  chunked-context gather CUDA-reads the KV cache, which is a pinned
  CPU pool under HiSparse; the sparse MQA path stages host context
  explicitly.

Verified: no conflict markers, ruff/mypy/typos/clang-format pre-commit
hooks pass. GPU tests (tests/v1/attention/test_sparse_mla_backends.py)
still need a CUDA machine.

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

Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working performance Performance-related issues ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants