Skip to content

Conversation

@WoosukKwon
Copy link
Collaborator

No description provided.

@WoosukKwon WoosukKwon added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 24, 2025
@mergify mergify bot added v1 tpu Related to Google TPUs labels Sep 24, 2025
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 is a great cleanup effort, removing a significant amount of deprecated V0 attention code. The changes are mostly correct and consistent. However, I found one critical issue in vllm/v1/attention/backends/pallas.py where the PallasAttentionBackend class fails to implement an abstract method from its base class, which will lead to a TypeError at runtime. Please see my comment for the details and a suggested fix.

@staticmethod
def get_metadata_cls() -> type["PallasMetadata"]:
return PallasMetadata

Copy link
Contributor

Choose a reason for hiding this comment

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

critical

The abstract method get_builder_cls is defined in the base class AttentionBackend, but it's not implemented in PallasAttentionBackend. This will cause a TypeError when trying to instantiate PallasAttentionBackend. Since the Pallas backend does not use a metadata builder, you should add an implementation that raises NotImplementedError.

    @staticmethod
    def get_builder_cls():
        raise NotImplementedError("Pallas backend does not use a metadata builder.")

Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: Woosuk Kwon <[email protected]>
@WoosukKwon WoosukKwon merged commit e6750d0 into main Sep 24, 2025
9 of 18 checks passed
@WoosukKwon WoosukKwon deleted the woosuk/cleanup-attn branch September 24, 2025 20:24
@hmellor hmellor moved this to Done in V0 Deprecation Sep 24, 2025
xuechendi added a commit to vllm-project/vllm-gaudi that referenced this pull request Sep 24, 2025
skavulya pushed a commit to skavulya/vllm-gaudi that referenced this pull request Sep 25, 2025
vllm-project/vllm#25541

---------

Signed-off-by: Chendi Xue <[email protected]>
Signed-off-by: Kavulya, Soila P <[email protected]>
iboiko-habana pushed a commit to iboiko-habana/vllm-gaudi that referenced this pull request Oct 2, 2025
vllm-project/vllm#25541

---------

Signed-off-by: Chendi Xue <[email protected]>
Signed-off-by: Iryna Boiko <[email protected]>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: yewentao256 <[email protected]>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: xuebwang-amd <[email protected]>
choprahetarth pushed a commit to Tandemn-Labs/vllm that referenced this pull request Oct 11, 2025
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: Woosuk Kwon <[email protected]>
Signed-off-by: xuebwang-amd <[email protected]>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
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 speculative-decoding tpu Related to Google TPUs v1

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants