Skip to content

Conversation

@pisceskkk
Copy link
Contributor

@pisceskkk pisceskkk commented Dec 4, 2025

Recently, vllm-ascend has been advancing the main-to-main adaptation work with the vllm repository. We have now fully adapted PCP and other features on vllm-ascend, so we believe it is necessary to add essential feature adaptation flags for the attention backend and modify the logic of the PCP feature check code to ensure that the functional switches for GPU and NPU do not interfere with each other.

Modifications:​

  • Consolidate all PCP-related feature check code in the same location as DCP checks. As backend functionalities are progressively completed, the relevant code can be deleted based on the situation.
  • Add feature adaptation flags for the attention backend. Subsequent PRs for backend adaptation can enable or disable the corresponding flags based on development progress.

CC @LucasWilkinson @FENP @LookAround0301 @zhenwenqi2024

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively relocates the feature checks for Prefill Context Parallelism (PCP) to a more consolidated and appropriate location, improving code organization and clarity. The introduction of new feature flags in the attention backend's abstract class is a good step towards more granular control over backend capabilities. While the refactoring is well-executed, I've identified one high-severity issue where a compatibility check was incorrectly scoped, which could lead to runtime errors in specific distributed configurations.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@pisceskkk pisceskkk force-pushed the pcp-check branch 2 times, most recently from 55ba613 to 0a76876 Compare December 5, 2025 03:23
@pisceskkk pisceskkk force-pushed the pcp-check branch 2 times, most recently from e2ebf25 to 38a6494 Compare December 11, 2025 01:55
@mergify
Copy link

mergify bot commented Dec 11, 2025

Hi @pisceskkk, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy or markdownlint failing?
mypy and markdownlint are run differently in CI. If the failure is related to either of these checks, please use the following commands to run them locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10
# For markdownlint
pre-commit run --hook-stage manual markdownlint

Copy link
Collaborator

@LucasWilkinson LucasWilkinson left a comment

Choose a reason for hiding this comment

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

LGTM

@LucasWilkinson LucasWilkinson enabled auto-merge (squash) December 11, 2025 04:58
@LucasWilkinson LucasWilkinson added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 11, 2025
@vllm-bot vllm-bot merged commit a11f4a8 into vllm-project:main Dec 11, 2025
53 of 55 checks passed
@pisceskkk pisceskkk deleted the pcp-check branch December 11, 2025 11:38
TheCodeWrangler pushed a commit to TheCodeWrangler/vllm that referenced this pull request Dec 12, 2025
Signed-off-by: QiuChunshuo <[email protected]>
Co-authored-by: Cyrus Leung <[email protected]>
Signed-off-by: Nathan Price <[email protected]>
TheCodeWrangler pushed a commit to TheCodeWrangler/vllm that referenced this pull request Dec 12, 2025
Signed-off-by: QiuChunshuo <[email protected]>
Co-authored-by: Cyrus Leung <[email protected]>
Signed-off-by: Nathan Price <[email protected]>

Signed-off-by: Nathan Price <[email protected]>
TheCodeWrangler pushed a commit to TheCodeWrangler/vllm that referenced this pull request Dec 12, 2025
Signed-off-by: QiuChunshuo <[email protected]>
Co-authored-by: Cyrus Leung <[email protected]>
Signed-off-by: Nathan Price <[email protected]>
weiyu0824 pushed a commit to weiyu0824/vllm that referenced this pull request Dec 16, 2025
quanliu1991 pushed a commit to quanliu1991/vllm that referenced this pull request Dec 22, 2025
Majid-Taheri pushed a commit to Majid-Taheri/vllm that referenced this pull request Dec 23, 2025
Signed-off-by: QiuChunshuo <[email protected]>
Co-authored-by: Cyrus Leung <[email protected]>
Signed-off-by: Ubuntu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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