Repository navigation
[Refactor][Worker] Reuse upstream PCP speculative and dummy batch handling - #17632
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 refactors the Ascend PCP (Persistent Context Parallelism) implementation to align with updated upstream vLLM capabilities. By removing temporary compatibility shims for speculative batch partitioning and dummy attention handling, the codebase now leverages native upstream logic, simplifying maintenance and reducing technical debt. 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
|
|
👋 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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Suggested PR Title:\n\nmarkdown\n[Worker][Misc] Remove temporary speculative decoding compatibility workarounds\n\n\nSuggested PR Summary:\n\nmarkdown\n### What this PR does / why we need it?\nThis PR removes temporary speculative decoding compatibility workarounds and dummy attention preparation overrides in `vllm_ascend/worker/v2/model_runner.py` and `vllm_ascend/worker/v2/pcp_manager.py`. Specifically, it deletes `_partition_speculative_batch_compat` and `prepare_dummy_attn` methods, simplifies batch partitioning, and preserves global attention state for decode-only requests. These cleanups are made possible as upstream support has been integrated.\n\nI have no feedback to provide on the code changes themselves as there are no review comments.\n\n### Does this PR introduce _any_ user-facing change?\nNo.\n\n### How was this patch tested?\nCI passed with existing tests.\n
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Preserve decode attention state and delegate dummy attention preparation to upstream while retaining V4.1 ring-state initialization. Align CPU regression tests with upstream partitioning and dummy staging. Signed-off-by: wzx0726 <278573478+wzx0726@users.noreply.github.com>
8aaae09 to
e59e0b6
Compare
…dling (vllm-project#17632) ### What this PR does / why we need it? The paired vLLM (`ced6857afa0ea7b2e3f0846a62e1394e90f15607`, v0.30.0) now supports speculative PCP partitioning and stages dummy inputs into persistent PCP buffers. Remove the compatibility paths introduced for these upstream gaps: - Delete `_partition_speculative_batch_compat` and call upstream `partition_batch` directly. For decode-only batches, preserve the attention state computed before partitioning, after refreshing local sequence lengths and graph padding. Upstream clears local draft metadata; recomputing the state from that metadata can turn an MTP verification batch into `PrefillCacheHit` when chunked prefill is disabled. - Delete `AscendPCPManager.prepare_dummy_attn` and the `NPUModelRunner.prepare_dummy_attn` override. Inherit the upstream runner implementation and retain the existing Ascend PCP block-table and attention-context extensions. Upstream `prepare_inputs_to_capture` already refreshes the persistent device input buffers. This follows up on vllm-project#14960 and vllm-project#15969. Only two production files change; the separate E2E changes in vllm-project#17150 are excluded. No new helpers, state, flags, or parallel implementations are introduced. ### Does this PR introduce _any_ user-facing change? No new API or configuration. PCP + MTP continues to use the existing speculative attention state while relying on the paired upstream implementation. ### How was this patch tested? - Static checks: Python AST parsing and `git diff --check` passed. The isolated PR patch matches the reviewed production changes exactly. - The speculative-partition/state-preservation change was previously compared against the original compatibility helper on Ascend A5: MRV2, TP=1, PCP=2, DCP=1, MTP=3, eager, block size 128, a five-layer DeepSeek model, chunked prefill both enabled and disabled. Single-request and two-request batches were repeated twice with temperature 0 and seed 7; all 768 compared output tokens matched exactly. The previously failing non-chunked case retained `SpecDecoding`. - The subsequent dummy-attention cleanup has **not** completed runtime validation. The DP-idle/FULL_DECODE_ONLY comparison stopped at an environment import failure before inference; further runs were paused at the author's request. The earlier eager results do not validate this cleanup or the final combined patch. - No test files were changed. DP real/idle/real transitions, FULL_DECODE_ONLY replay, mixed prefill/decode traffic, and full-model coverage remain pending. This PR is intentionally a draft. - vLLM main: vllm-project/vllm@ced6857 Signed-off-by: wzx0726 <278573478+wzx0726@users.noreply.github.com> Co-authored-by: wzx0726 <278573478+wzx0726@users.noreply.github.com>
… call sites The three call sites of _get_full_attention_dcp_sizes referenced remote_pcp_size, a parameter that exists in upstream signatures after the PCP refactor (vllm-project#17632 lineage) but not in ours. Our tree pins pcp_size==1 at worker init (base_worker assert), and the per-PP-rank metadata has no pcp field, so derive it via getattr(remote_metadata, 'pcp_size', 1) — identical to upstream behavior under our pcp_size==1 constraint. Verified: AST name-resolution scan finds no remaining unresolved free names in pull_worker.py (2 reported hits are except-scoped 'exc', false positives). Signed-off-by: xiangyongzh <ascend-operator@localhost>
4 participants
What this PR does / why we need it?
The paired vLLM (
ced6857afa0ea7b2e3f0846a62e1394e90f15607, v0.30.0) now supports speculative PCP partitioning and stages dummy inputs into persistent PCP buffers. Remove the compatibility paths introduced for these upstream gaps:_partition_speculative_batch_compatand call upstreampartition_batchdirectly. For decode-only batches, preserve the attention state computed before partitioning, after refreshing local sequence lengths and graph padding. Upstream clears local draft metadata; recomputing the state from that metadata can turn an MTP verification batch intoPrefillCacheHitwhen chunked prefill is disabled.AscendPCPManager.prepare_dummy_attnand theNPUModelRunner.prepare_dummy_attnoverride. Inherit the upstream runner implementation and retain the existing Ascend PCP block-table and attention-context extensions. Upstreamprepare_inputs_to_capturealready refreshes the persistent device input buffers.This follows up on #14960 and #15969. Only two production files change; the separate E2E changes in #17150 are excluded. No new helpers, state, flags, or parallel implementations are introduced.
Does this PR introduce any user-facing change?
No new API or configuration. PCP + MTP continues to use the existing speculative attention state while relying on the paired upstream implementation.
How was this patch tested?
Static checks: Python AST parsing and
git diff --checkpassed. The isolated PR patch matches the reviewed production changes exactly.The speculative-partition/state-preservation change was previously compared against the original compatibility helper on Ascend A5: MRV2, TP=1, PCP=2, DCP=1, MTP=3, eager, block size 128, a five-layer DeepSeek model, chunked prefill both enabled and disabled. Single-request and two-request batches were repeated twice with temperature 0 and seed 7; all 768 compared output tokens matched exactly. The previously failing non-chunked case retained
SpecDecoding.The subsequent dummy-attention cleanup has not completed runtime validation. The DP-idle/FULL_DECODE_ONLY comparison stopped at an environment import failure before inference; further runs were paused at the author's request. The earlier eager results do not validate this cleanup or the final combined patch.
No test files were changed. DP real/idle/real transitions, FULL_DECODE_ONLY replay, mixed prefill/decode traffic, and full-model coverage remain pending. This PR is intentionally a draft.
vLLM main: vllm-project/vllm@ced6857