Skip to content

[Bugfix] Make siglip/clip compatible with transformers v5 #37200

Merged
hmellor merged 1 commit intovllm-project:mainfrom
zucchini-nlp:fix-clip-models
Mar 16, 2026
Merged

[Bugfix] Make siglip/clip compatible with transformers v5 #37200
hmellor merged 1 commit intovllm-project:mainfrom
zucchini-nlp:fix-clip-models

Conversation

@zucchini-nlp
Copy link
Copy Markdown
Contributor

@zucchini-nlp zucchini-nlp commented Mar 16, 2026

From v5 we are returning a BaseModelOutputWithPooling from get_xxx_features. This PR fixes the test to be compatible with v4-format tensor output and v5-format dict output

cc @hmellor

Signed-off-by: raushan <raushan@huggingface.co>
@zucchini-nlp zucchini-nlp requested a review from noooop as a code owner March 16, 2026 15:17
@mergify mergify bot added multi-modality Related to multi-modality (#4194) bug Something isn't working labels Mar 16, 2026
Copy link
Copy Markdown
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

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

LGTM!

@hmellor hmellor enabled auto-merge (squash) March 16, 2026 15:22
Copy link
Copy Markdown
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 addresses a compatibility issue with transformers v5 in the siglip and clip model tests. The changes adapt the tests to handle the new BaseModelOutputWithPooling return type from feature extraction methods. My review includes suggestions to make the implementation more concise and idiomatic by using getattr for accessing the pooler_output, which simplifies the logic for handling different transformers versions.

Comment on lines +61 to +62
if not isinstance(pooled_output, torch.Tensor):
pooled_output = pooled_output.pooler_output
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

For better readability and conciseness, you can use getattr to handle both cases (tensor output from older transformers versions and BaseModelOutputWithPooling from newer versions). This avoids the explicit type check and makes the intent clearer.

            pooled_output = getattr(pooled_output, 'pooler_output', pooled_output)

Comment on lines +78 to +79
if not isinstance(pooled_output, torch.Tensor):
pooled_output = pooled_output.pooler_output
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Similar to the comment on test_clip.py, using getattr here would make the code more concise and idiomatic for handling different return types.

            pooled_output = getattr(pooled_output, 'pooler_output', pooled_output)

@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 16, 2026
@hmellor hmellor merged commit 55e6d3d into vllm-project:main Mar 16, 2026
21 checks passed
Lucaskabela pushed a commit to Lucaskabela/vllm that referenced this pull request Mar 17, 2026
wendyliu235 pushed a commit to wendyliu235/vllm-public that referenced this pull request Mar 18, 2026
fxdawnn pushed a commit to fxdawnn/vllm that referenced this pull request Mar 19, 2026
khairulkabir1661 pushed a commit to khairulkabir1661/vllm that referenced this pull request Mar 27, 2026
Monishver11 pushed a commit to Monishver11/vllm that referenced this pull request Mar 27, 2026
…ct#37200)

Signed-off-by: raushan <raushan@huggingface.co>
Signed-off-by: Monishver Chandrasekaran <monishverchandrasekaran@gmail.com>
JiantaoXu pushed a commit to JiantaoXu/vllm that referenced this pull request Mar 28, 2026
vrdn-23 pushed a commit to vrdn-23/vllm that referenced this pull request Mar 30, 2026
…ct#37200)

Signed-off-by: raushan <raushan@huggingface.co>
Signed-off-by: Vinay Damodaran <vrdn@hey.com>
EricccYang pushed a commit to EricccYang/vllm that referenced this pull request Apr 1, 2026
…ct#37200)

Signed-off-by: raushan <raushan@huggingface.co>
Signed-off-by: EricccYang <yangyang4991@gmail.com>
liuchenbing2026 pushed a commit to liuchenbing2026/vllm that referenced this pull request Apr 4, 2026
big-yellow-duck pushed a commit to EmbeddedLLM/vllm that referenced this pull request Apr 8, 2026
mtparet pushed a commit to blackfuel-ai/vllm that referenced this pull request Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working multi-modality Related to multi-modality (#4194) ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants