Repository navigation
[BugFix][SpecDecode] Refresh replicated PCP draft graph cache mappings - #16300
Conversation
Signed-off-by: Madilyn537 <zhexuanwu12@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in replicated PCP speculative decoding where persistent KV cache buffers were not being correctly refreshed for MLA/GQA architectures during FULL graph replay. By refactoring the mapping refresh logic into a shared method, the patch ensures these buffers remain consistent across batches and draft steps without disrupting existing metadata rebuild paths. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Attention][Feature] Refresh replicated prefill mappings for draft speculatorSuggested PR Summary:
### What this PR does / why we need it?
This PR refactors the speculator to refresh persistent cache mappings used by replicated drafts during graph prefill. Specifically, it extracts `_refresh_replicated_prefill_mappings` from `_prepare_replicated_prefill_attn` and ensures it is called even when `_prepare_replicated_prefill_attn` is bypassed in `build_draft_attn_metadatas`. It also adds comprehensive unit tests to verify that captured cache buffers are refreshed correctly and that bypass guards preserve metadata.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Tested with new unit tests in `tests/ut/worker/test_mtp_pcp_speculator_v2.py`, including `test_graph_prefill_refreshes_captured_cache_buffers`, `test_prepare_replicated_prefill_preserves_bypass`, and `test_graph_prefill_without_real_batch_preserves_metadata`.|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [BugFix] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
Signed-off-by: Madilyn537 <zhexuanwu12@gmail.com>
| assert prepared_attn_metadata is not None | ||
| attn_metadata = prepared_attn_metadata | ||
| else: | ||
| self._refresh_replicated_prefill_mappings(num_reqs_padded, num_tokens_padded) |
There was a problem hiding this comment.
Add a TOTO about deleting it when FIA remove its check.
| num_tokens_padded=num_tokens_padded, | ||
| ) | ||
|
|
||
| def _prepare_replicated_prefill_attn( |
There was a problem hiding this comment.
Cause prepare_attn in model_runner just modify pcp's variables.
Signed-off-by: Madilyn537 <zhexuanwu12@gmail.com>
Signed-off-by: Madilyn537 <zhexuanwu12@gmail.com>
Signed-off-by: Madilyn537 <zhexuanwu12@gmail.com>
|
/rerun [Bot]: rerun completed. Rerun (failed jobs only):
|
|
/rerun Rerun (failed jobs only):
|
|
LGTM. |
…hado/vllm-ascend into main_fix_mrv2_eagle3_mamba * 'main_fix_mrv2_eagle3_mamba' of https://github.com/windshado/vllm-ascend: (42 commits) Update vllm_ascend/worker/v2/model_states/mamba_hybrid.py [Feature][Kimi K3 DSPark] Enable TP for context_proj (vllm-project#16344) [BugFix][SpecDecode] Refresh replicated PCP draft graph cache mappings (vllm-project#16300) [Feature][Model] Integrate Triton KeyPool indexing for GLM-5.3-Flash (vllm-project#16253) [BugFix][Offloader] Re-bind params to NZ static buffers after npu_format_cast (vllm-project#15415) [Feature][Model] Integrate AscendC KDA and causal convolution for GLM-5.3-Flash (vllm-project#16251) [Performance][Communicator] Replace per-layer F.pad with cat of a persistent zero block in MoE prepare (vllm-project#16343) [Feature][Operator] Add DeepSeek V4.1 sparse attention operators (vllm-project#16422) [Doc][Misc] Document batch invariance scheduling limitations (vllm-project#16232) [CI][MRV2] Enable mrv2 dspark e2e test (vllm-project#16319) [BugFix] Precast MoE gate weight_fp32 to avoid aclop Cast (vllm-project#16189) [Feature][MRV2][310P] MRv2 adapting MTP on the 310P for Qwen3.5 (vllm-project#16043) [Revert] Revert "[Feature][MRV1][MRV2] Refactor Host-Side Parameter Updates for ACL Graph Replay." (vllm-project#15908) (vllm-project#16409) [Feature][Ops] Add Triton KeyPool compression and pooled indexing (vllm-project#16243) [Feature][Attention] Support NoPE in the shared SFA backend (vllm-project#16252) [Performance][Model] Reuse fused mHC operators for GLM-5.3-Flash (vllm-project#16321) [Feature][Model] Enable MiniMax-M3 FP8 MSA index score on A5 (vllm-project#15918) [Performance][KDA] Reduce preprocessing copies and redundant output masks (vllm-project#16067) [Feature][Model][MTP] Support speculative decoding for GLM-5.3-Flash (vllm-project#16214) [BugFix][Model] Skip unused hash-router bias when loading DeepSeek-V4 weights (vllm-project#16259) ...
vllm-project#16300) ### What this PR does / why we need it? With replicated PCP, the first draft stage under FULL graph replay must refresh the persistent block-table and slot-mapping buffers captured by the draft model. The MLA/GQA path reuses target attention metadata and skips `_prepare_replicated_prefill_attn`, leaving those default buffers stale between batches or draft steps. Extract the existing mapping refresh into `_refresh_replicated_prefill_mappings` and reuse it from both the preparation path and the MLA/GQA graph path. This preserves MLA/GQA metadata and its padded query layout, while DSA/SFA retain their existing metadata rebuild path with one mapping refresh. No new buffers, flags, or query-padding logic are introduced. ### Does this PR introduce _any_ user-facing change? Fixes stale KV cache mappings for replicated-PCP speculative decoding with MLA/GQA FULL graphs, which can affect draft acceptance. No CLI or configuration changes. ### How was this patch tested? - Remote isolated CPU unit tests with mocked BlockTables operations: **54 passed** across the following files: ```bash python3 -m pytest tests/ut/worker/test_mtp_pcp_speculator_v2.py tests/ut/worker/test_attn_utils_v2.py -q --tb=short ``` - The selected regression tests against the unpatched baseline produced **4 failed, 14 passed, 19 deselected**. They detect missing MLA/GQA refreshes and stale slot values. - Coverage includes all four attention architectures, request/block changes, padded slots, stable buffer addresses, metadata identity, and non-PCP/dummy/missing-batch bypasses. - Ruff lint, Ruff format check on changed files, and `git diff --check` passed. The full `format.sh ci` check was not run because Git Bash and pre-commit are unavailable in the local Windows environment. **Validation status:** These tests verify the refresh and buffer contracts using CPU tensors and mocks; they do not exercise NPU mapping kernels or graph replay. A service startup failure was reported after deployment and has not yet been diagnosed. NPU startup/capture/replay, end-to-end correctness, and acceptance-rate comparison for this exact patch remain pending. Per-request KV-length correction is outside the scope of this PR. Existing length construction and update behavior is unchanged. - vLLM main: vllm-project/vllm@a97dacb --------- Signed-off-by: Madilyn537 <zhexuanwu12@gmail.com> Signed-off-by: tianming2009 <13246728590@163.com>
vllm-project#16300) ### What this PR does / why we need it? With replicated PCP, the first draft stage under FULL graph replay must refresh the persistent block-table and slot-mapping buffers captured by the draft model. The MLA/GQA path reuses target attention metadata and skips `_prepare_replicated_prefill_attn`, leaving those default buffers stale between batches or draft steps. Extract the existing mapping refresh into `_refresh_replicated_prefill_mappings` and reuse it from both the preparation path and the MLA/GQA graph path. This preserves MLA/GQA metadata and its padded query layout, while DSA/SFA retain their existing metadata rebuild path with one mapping refresh. No new buffers, flags, or query-padding logic are introduced. ### Does this PR introduce _any_ user-facing change? Fixes stale KV cache mappings for replicated-PCP speculative decoding with MLA/GQA FULL graphs, which can affect draft acceptance. No CLI or configuration changes. ### How was this patch tested? - Remote isolated CPU unit tests with mocked BlockTables operations: **54 passed** across the following files: ```bash python3 -m pytest tests/ut/worker/test_mtp_pcp_speculator_v2.py tests/ut/worker/test_attn_utils_v2.py -q --tb=short ``` - The selected regression tests against the unpatched baseline produced **4 failed, 14 passed, 19 deselected**. They detect missing MLA/GQA refreshes and stale slot values. - Coverage includes all four attention architectures, request/block changes, padded slots, stable buffer addresses, metadata identity, and non-PCP/dummy/missing-batch bypasses. - Ruff lint, Ruff format check on changed files, and `git diff --check` passed. The full `format.sh ci` check was not run because Git Bash and pre-commit are unavailable in the local Windows environment. **Validation status:** These tests verify the refresh and buffer contracts using CPU tensors and mocks; they do not exercise NPU mapping kernels or graph replay. A service startup failure was reported after deployment and has not yet been diagnosed. NPU startup/capture/replay, end-to-end correctness, and acceptance-rate comparison for this exact patch remain pending. Per-request KV-length correction is outside the scope of this PR. Existing length construction and update behavior is unchanged. - vLLM main: vllm-project/vllm@a97dacb --------- Signed-off-by: Madilyn537 <zhexuanwu12@gmail.com> Signed-off-by: like-0517 <ithwlike@126.com>
What this PR does / why we need it?
With replicated PCP, the first draft stage under FULL graph replay must refresh the persistent block-table and slot-mapping buffers captured by the draft model. The MLA/GQA path reuses target attention metadata and skips
_prepare_replicated_prefill_attn, leaving those default buffers stale between batches or draft steps.Extract the existing mapping refresh into
_refresh_replicated_prefill_mappingsand reuse it from both the preparation path and the MLA/GQA graph path. This preserves MLA/GQA metadata and its padded query layout, while DSA/SFA retain their existing metadata rebuild path with one mapping refresh. No new buffers, flags, or query-padding logic are introduced.Does this PR introduce any user-facing change?
Fixes stale KV cache mappings for replicated-PCP speculative decoding with MLA/GQA FULL graphs, which can affect draft acceptance. No CLI or configuration changes.
How was this patch tested?
git diff --checkpassed. The fullformat.sh cicheck was not run because Git Bash and pre-commit are unavailable in the local Windows environment.Validation status: These tests verify the refresh and buffer contracts using CPU tensors and mocks; they do not exercise NPU mapping kernels or graph replay. A service startup failure was reported after deployment and has not yet been diagnosed. NPU startup/capture/replay, end-to-end correctness, and acceptance-rate comparison for this exact patch remain pending.
Per-request KV-length correction is outside the scope of this PR. Existing length construction and update behavior is unchanged.