Skip to content

[rocm][V0] fix selection logic for custom PA in V0 #16426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 17, 2025
Merged

[rocm][V0] fix selection logic for custom PA in V0 #16426

merged 2 commits into from
Apr 17, 2025

Conversation

divakar-amd
Copy link
Contributor

@divakar-amd divakar-amd commented Apr 10, 2025 •
edited by github-actions bot
Loading

This PR enables custom Paged Attention for vLLM-V0
When measured on Mistral-7B-v0.1-FP8-KV for input=2048, output=128, bs=128, the e2e latency difference is about 2-3x before and after this PR.
This extends the previous PR: #15982

@divakar-amd
enable custom PA for V0
545cd06 
Signed-off-by: Divakar Verma <[email protected]>
@github-actions GitHub Actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

@divakar-amd divakar-amd changed the title enable custom PA for V0 [rocm][V0] fix selection logic for custom PA in V0 Apr 10, 2025
SageMoore
SageMoore approved these changes Apr 11, 2025
View reviewed changes
Copy link
Contributor

@SageMoore SageMoore left a comment

Choose a reason for hiding this comment

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

LGTM. Let's make sure that we run the AMD CI on this PR before merging

@Alexei-V-Ivanov-AMD
Copy link
Collaborator

/ready

ProExpertProg
ProExpertProg reviewed Apr 13, 2025
View reviewed changes
Copy link
Collaborator

@ProExpertProg ProExpertProg left a comment

Choose a reason for hiding this comment

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

Could we just improve the comment as the logic is a little obscure?

vllm/platforms/rocm.py
Comment on lines +113 to +114
and (not envs.VLLM_USE_V1 or sliding_window == 0
or sliding_window == (-1, -1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind adding a comment here? Something like: "custom paged attn always supported on V0, only [with(out)...] sliding window on V1" - where the [...] is a short description of why the sliding window checks are there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@robertgshaw2-redhat robertgshaw2-redhat enabled auto-merge (squash) April 15, 2025 16:30
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Apr 15, 2025
@vllm-bot vllm-bot merged commit 95aca28 into vllm-project:main Apr 17, 2025
56 of 58 checks passed
lionelvillard pushed a commit to lionelvillard/vllm that referenced this pull request Apr 17, 2025
@divakar-amd @lionelvillard
[rocm][V0] fix selection logic for custom PA in V0 (vllm-project#16426)
dc1f31f 
Signed-off-by: Divakar Verma <[email protected]>
yangw-dev pushed a commit to yangw-dev/vllm that referenced this pull request Apr 21, 2025
@divakar-amd @yangw-dev
[rocm][V0] fix selection logic for custom PA in V0 (vllm-project#16426)
0b6c05d 
Signed-off-by: Divakar Verma <[email protected]>
Signed-off-by: Yang Wang <[email protected]>
jikunshang pushed a commit to jikunshang/vllm that referenced this pull request Apr 29, 2025
@divakar-amd @dbyoung18
[rocm][V0] fix selection logic for custom PA in V0 (vllm-project#16426)
b35dda9 
Signed-off-by: Divakar Verma <[email protected]>
lk-chen pushed a commit to lk-chen/vllm that referenced this pull request Apr 29, 2025
@divakar-amd @lk-chen
[rocm][V0] fix selection logic for custom PA in V0 (vllm-project#16426)
1bbf332 
Signed-off-by: Divakar Verma <[email protected]>
adobrzyn pushed a commit to HabanaAI/vllm-fork that referenced this pull request Apr 30, 2025
@divakar-amd @adobrzyn
[rocm][V0] fix selection logic for custom PA in V0 (vllm-project#16426)
69647a3 
Signed-off-by: Divakar Verma <[email protected]>
Signed-off-by: Agata Dobrzyniewicz <[email protected]>
RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request May 12, 2025
@divakar-amd
[rocm][V0] fix selection logic for custom PA in V0 (vllm-project#16426)
28fc3f0 
Signed-off-by: Divakar Verma <[email protected]>
Signed-off-by: Mu Huai <[email protected]>
@ckhordiasma ckhordiasma mentioned this pull request May 14, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ProExpertProg ProExpertProg ProExpertProg approved these changes

@robertgshaw2-redhat robertgshaw2-redhat robertgshaw2-redhat approved these changes

+1 more reviewer

@SageMoore SageMoore SageMoore approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@divakar-amd @Alexei-V-Ivanov-AMD @SageMoore @ProExpertProg @robertgshaw2-redhat @vllm-bot