[Misc][MRV2] Revert obsolete vLLM 0.28 PCP+DP adapters - #17174
Conversation
|
👋 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. |
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 performs a cleanup by removing compatibility code and workarounds specifically introduced for vLLM 0.28.0. With the upgrade to vLLM 0.29.0, these adapters are no longer necessary as the newer version provides native support for PCP+DP configurations. The changes focus on removing shims, context wrappers, and associated tests, while preserving necessary v0.29.0-specific patches. 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
|
![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
This pull request removes obsolete vLLM 0.28.0 PCP+DP compatibility adapters and workarounds across several files, including model runner, PCP manager, and parallel configuration patches, as well as their associated unit tests, following the upgrade to v0.29.0. The review feedback correctly identifies that the PR title and summary should be updated to comply with the Repository Style Guide, providing a well-structured markdown template for the title and description.
wzx0726
changed the title
3227f2d to
990fdd5
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
990fdd5 to
5034869
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Revert the obsolete compatibility portions of 8f2e3fe (vllm-project#16853). Both supported vLLM pins provide PCP-local dispatch counting. Remove the 0.28 validator shim, dispatch context and count override, and their release-only tests and documentation. Keep the separate 0.29 validator, generic PCP count/feature coverage, and later slot-buffer dtype tests. Resolve conflicts preserving 0.29 argument gating and nullcontext. Validation: targeted Ruff lint/format, AST syntax, unchanged-function comparison and git diff --check passed. Ascend runtime UT and smoke validation were not run. Signed-off-by: wzx0726 <278573478+wzx0726@users.noreply.github.com>
5034869 to
d6b9f29
Compare
…17174) ### What this PR does / why we need it? Remove the obsolete vLLM 0.28.0 PCP+DP compatibility introduced by vllm-project#16853 (`8f2e3fed73328148ea603ccfe5764542fa7cbbd2`) after vllm-project#17004 upgraded the supported release to v0.29.0. Both v0.29.0 and the paired vLLM main provide native PCP-local token counting before dispatch. This is a conflict-resolved revert with targeted cleanup: - Remove the 0.28-only ParallelConfig validator shim, dispatch ContextVar/wrapper, the legacy dispatch block inside gather_batch_req_state, and PCP token-count override. - Preserve the independent v0.29.0 patch_parallel_config.py: the release still rejects PCP+DP without that patch. - Preserve vllm-project#17128's PD decode recompute logic in gather_batch_req_state and its regression tests. - Preserve subsequent changes, including nullcontext and the v0.29.0 argument gates, generic PCP count/feature tests, and slot-buffer dtype coverage. - Remove obsolete adapter-specific tests and patch documentation. Remove imports made unused by the cleanup, including vllm_version_is in pcp_manager.py; that import predates vllm-project#16853 but has no remaining references after this revert and would trigger Ruff F401. No new tests or unrelated formatting changes. The execute_model body is unindented only to remove its obsolete context wrapper. ### Does this PR introduce _any_ user-facing change? No intended behavior change on the supported v0.29.0 and fixed-main lanes. The retired v0.28.0 compatibility is removed; v0.29.0 PCP+DP configuration support remains provided by the independent validator patch. ### How was this patch tested? On rebased commit `d6b9f29e01ee2daa2d594b8c5c828e94dac0b128`: - Ruff lint/format, AST syntax, conflict-marker and git diff --check checks passed for the seven changed Python files. - Preserve the new upstream step_eplb_after import, PD decode recompute logic, and o_proj TP graph checks. Range-diff against 5034869 shows only upstream context changes; the cleanup scope is unchanged. - DCO Signed-off-by is retained. Fresh CI is pending; no local Ascend runtime tests were run on this revision. - Previous revision 5034869 passed [E2E run 35741354764](https://github.com/vllm-project/vllm-ascend/actions/runs/35741354764), including pre-commit, CPU UT, selected NPU jobs, upstream tests and ci-gate. Skipped jobs are not passes; these results do not establish validation of this rebased revision. Base: vllm-ascend main `972fcd5c974d55a7970ebe630329daa4b4049c49`. Release: vLLM v0.29.0 (`98dff2a81d747d1dba01a47f939f48c3526d4206`). vLLM main: vllm-project/vllm@84030bb - vLLM main: vllm-project/vllm@84030bb Signed-off-by: wzx0726 <278573478+wzx0726@users.noreply.github.com> Co-authored-by: wzx0726 <278573478+wzx0726@users.noreply.github.com>
4 participants
What this PR does / why we need it?
Remove the obsolete vLLM 0.28.0 PCP+DP compatibility introduced by #16853 (
8f2e3fed73328148ea603ccfe5764542fa7cbbd2) after #17004 upgraded the supported release to v0.29.0. Both v0.29.0 and the paired vLLM main provide native PCP-local token counting before dispatch.This is a conflict-resolved revert with targeted cleanup:
Remove the 0.28-only ParallelConfig validator shim, dispatch ContextVar/wrapper, the legacy dispatch block inside gather_batch_req_state, and PCP token-count override.
Preserve the independent v0.29.0 patch_parallel_config.py: the release still rejects PCP+DP without that patch.
Preserve [Bugfix][MRV2][P/D] Preserve decode graph with recompute scheduler #17128's PD decode recompute logic in gather_batch_req_state and its regression tests.
Preserve subsequent changes, including nullcontext and the v0.29.0 argument gates, generic PCP count/feature tests, and slot-buffer dtype coverage.
Remove obsolete adapter-specific tests and patch documentation. Remove imports made unused by the cleanup, including vllm_version_is in pcp_manager.py; that import predates [BugFix][MRV2] Adapt PCP+DP dispatch for vLLM 0.28.0 #16853 but has no remaining references after this revert and would trigger Ruff F401.
No new tests or unrelated formatting changes. The execute_model body is unindented only to remove its obsolete context wrapper.
Does this PR introduce any user-facing change?
No intended behavior change on the supported v0.29.0 and fixed-main lanes. The retired v0.28.0 compatibility is removed; v0.29.0 PCP+DP configuration support remains provided by the independent validator patch.
How was this patch tested?
On rebased commit
d6b9f29e01ee2daa2d594b8c5c828e94dac0b128:Base: vllm-ascend main
972fcd5c974d55a7970ebe630329daa4b4049c49.Release: vLLM v0.29.0 (
98dff2a81d747d1dba01a47f939f48c3526d4206).vLLM main: vllm-project/vllm@84030bb