Skip to content

Support using SigLIP2 text and image embedding as standalone model#24027

Closed
duc-ph wants to merge 11 commits intovllm-project:mainfrom
duc-ph:feature/siglip2_pooling
Closed

Support using SigLIP2 text and image embedding as standalone model#24027
duc-ph wants to merge 11 commits intovllm-project:mainfrom
duc-ph:feature/siglip2_pooling

Conversation

@duc-ph
Copy link
Copy Markdown

@duc-ph duc-ph commented Sep 1, 2025

WIP

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 refactors the SigLIP2 implementation to support using the text and image embedding models as standalone components. The changes are generally in the right direction, but there are several critical issues in the new text model implementation that will cause runtime errors. These include incorrect loop iteration, a method call with missing arguments, and an incorrect call to a superclass constructor. Additionally, there's a potential bug in how vision inputs are checked, which could lead to incorrect behavior for empty inputs. These issues need to be addressed before this PR can be merged.

@duc-ph duc-ph marked this pull request as draft September 1, 2025 06:53
@duc-ph duc-ph changed the title (draft) Support using SigLIP2 text and image embedding as standalone model Support using SigLIP2 text and image embedding as standalone model Sep 1, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 1, 2025

👋 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 ask your reviewers to trigger select CI tests on top of fastcheck CI.

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.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

🚀

DP and others added 2 commits September 2, 2025 14:37
Signed-off-by: “DP” <you@example.com>
@mergify mergify bot added multi-modality Related to multi-modality (#4194) new-model Requests to new models labels Sep 3, 2025
“DP” added 2 commits September 8, 2025 00:15
Signed-off-by: “DP” <you@example.com>
Signed-off-by: “DP” <you@example.com>
@hmellor
Copy link
Copy Markdown
Member

hmellor commented Mar 6, 2026

Closing this PR as the functionality has been superseded by merged work. PR #27324 added SiglipEmbeddingModel which already supports SigLIP2 text and image embeddings as a standalone model.

@hmellor hmellor closed this Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-modality Related to multi-modality (#4194) new-model Requests to new models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants