Repository navigation
[BugFix] Fix speculative decoding graph replay - #16221
Conversation
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 the speculative decoding graph replay mechanism. By refining the logic for rebuilding draft prefill metadata to only trigger for DSA-based architectures, the system avoids incorrect metadata states that previously caused query-length mismatches. These changes ensure more reliable speculative decoding performance across different model configurations. 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
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
👋 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! |
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Attention][Feature] Support MLA architecture in draft attention metadata building for speculatorSuggested PR Summary:
### What this PR does / why we need it?
This PR updates `build_draft_attn_metadatas` in `AscendMTPSpeculator` to conditionally prepare replicated prefill attention only when the attention architecture is "DSA". For other architectures like "MLA", it bypasses this preparation and directly returns the original attention metadata. This is necessary to support different attention architectures during draft model prefill.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
The changes were tested by updating the unit test `test_graph_prefill_builds_draft_metadata` in `tests/ut/worker/test_mtp_pcp_speculator_v2.py` to cover different combinations of `attn_architecture`, `replicated_pcp`, and `rebuild_metadata`.No review comments were provided, so there is no additional feedback to provide.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Fixes PCP speculative decoding graph replay failures caused by query-length mismatches by conditionally rebuilding draft prefill attention metadata only for DSA.
Changes:
- Gate replicated prefill attention metadata preparation behind
attn_architecture == "DSA"during draft-model prefill. - Expand the unit test matrix to cover DSA/MLA + replicated/non-replicated PCP combinations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vllm_ascend/worker/v2/spec_decode/autoregressive/speculator.py | Restricts replicated prefill attention-metadata preparation to DSA to prevent query-length mismatches during graph replay. |
| tests/ut/worker/test_mtp_pcp_speculator_v2.py | Updates UT to parameterize behavior across attention architectures and PCP replication modes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
2a2ca6d to
16120cb
Compare
Restrict draft prefill metadata rebuilding to DSA and SFA. Reuse existing metadata for other attention backends to preserve padded query lengths during speculative graph replay. Signed-off-by: leolee <yihao.li@huawei.com>
16120cb to
52360b1
Compare
### What this PR does / why we need it? Fix a query-length mismatch during PCP speculative graph replay by rebuilding draft prefill metadata only for DSA and SFA. ### Does this PR introduce _any_ user-facing change? Yes, fixes speculative decoding graph replay failures. ### How was this patch tested? - vLLM main: vllm-project/vllm@b2f6858 Signed-off-by: leolee <yihao.li@huawei.com>
### What this PR does / why we need it? Fix a query-length mismatch during PCP speculative graph replay by rebuilding draft prefill metadata only for DSA and SFA. ### Does this PR introduce _any_ user-facing change? Yes, fixes speculative decoding graph replay failures. ### How was this patch tested? - vLLM main: vllm-project/vllm@b2f6858 Signed-off-by: leolee <yihao.li@huawei.com> Signed-off-by: tianming2009 <13246728590@163.com>
### What this PR does / why we need it? Fix a query-length mismatch during PCP speculative graph replay by rebuilding draft prefill metadata only for DSA and SFA. ### Does this PR introduce _any_ user-facing change? Yes, fixes speculative decoding graph replay failures. ### How was this patch tested? - vLLM main: vllm-project/vllm@b2f6858 Signed-off-by: leolee <yihao.li@huawei.com> Signed-off-by: like-0517 <ithwlike@126.com>
What this PR does / why we need it?
Fix a query-length mismatch during PCP speculative graph replay by rebuilding draft prefill metadata only for DSA and SFA.
Does this PR introduce any user-facing change?
Yes, fixes speculative decoding graph replay failures.
How was this patch tested?