Skip to content

[Core] Enable inputs_embeds_size separate from hidden_size#29741

Merged
DarkLight1337 merged 13 commits intovllm-project:mainfrom
DarkLight1337:inputs-embeds-size
Nov 30, 2025
Merged

[Core] Enable inputs_embeds_size separate from hidden_size#29741
DarkLight1337 merged 13 commits intovllm-project:mainfrom
DarkLight1337:inputs-embeds-size

Conversation

@DarkLight1337
Copy link
Copy Markdown
Member

@DarkLight1337 DarkLight1337 commented Nov 30, 2025

Purpose

  • Add a new method ModelConfig.get_inputs_embeds_size to distinguish the hidden dimension of inputs_embeds from ModelConfig.get_hidden_size.
  • Update model runners to use ModelConfig.get_inputs_embeds_size. (Notable for OOT model runners)
  • Update CLIP and SigLIP to handle the case when get_inputs_embeds_size and get_hidden_size return different values.

FIX #27566 (comment)

cc @piood @patrickvonplaten @wangxiyuan

Test Plan

Add google/siglip2-giant-opt-patch16-384 to SigLIP tests

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@mergify mergify bot added multi-modality Related to multi-modality (#4194) speculative-decoding v1 tpu Related to Google TPUs labels Nov 30, 2025
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
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 introduces support for models where inputs_embeds_size is different from hidden_size, which is a great addition for supporting more multimodal models. The changes are mostly about propagating this new configuration. However, I found a critical issue in the implementation of get_inputs_embeds_size which could lead to incorrect behavior for the intended models. My review includes a fix for this issue.

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 removed the ready ONLY add when PR is ready to merge/full CI is needed label Nov 30, 2025
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 30, 2025
@DarkLight1337
Copy link
Copy Markdown
Member Author

OK the test passes locally now.

Copy link
Copy Markdown
Member Author

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

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

/gemini review

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
@DarkLight1337 DarkLight1337 merged commit 64bc09b into vllm-project:main Nov 30, 2025
57 checks passed
@DarkLight1337 DarkLight1337 deleted the inputs-embeds-size branch November 30, 2025 09:31
@github-project-automation github-project-automation bot moved this from In Progress to Done in Multi-modality Core Nov 30, 2025
@wangxiyuan
Copy link
Copy Markdown
Contributor

Thanks for reminding. @Potabk Please consider this change when upgrading vLLM

@piood
Copy link
Copy Markdown
Contributor

piood commented Dec 1, 2025

Thanks for fix that!

kitaekatt pushed a commit to kitaekatt/vllm that referenced this pull request Dec 1, 2025
…project#29741)

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
amd-hhashemi pushed a commit to amd-hhashemi/vllm that referenced this pull request Dec 2, 2025
…project#29741)

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: Hashem Hashemi <hashem.hashemi@amd.com>
dsuhinin pushed a commit to dsuhinin/vllm that referenced this pull request Jan 21, 2026
…project#29741)

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: dsuhinin <suhinin.dmitriy@gmail.com>
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) 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.

4 participants