Skip to content

Better weight tying check for multimodal models - #38035

Merged
DarkLight1337 merged 3 commits into
vllm-project:mainfrom
hmellor:fix-weight-tying
Mar 25, 2026
Merged

DarkLight1337 merged 3 commits into
vllm-project:mainfrom
hmellor:fix-weight-tying

Conversation

@hmellor

@hmellor hmellor commented Mar 24, 2026

Copy link
Copy Markdown
Member

For some models, SomeVLTextConfig may also have a tie_word_embeddings field. This is only the case if SomeVLTextConfig is also used for a text only version of the same model. For example:

SomeVLModelForCausalLM:
    self.model = SomeLanguageModel(SomeVLTextConfig)
    self.lm_head = nn.Linear()

Therefore, the presence of tie_word_embeddings in SomeVLTextConfig cannot be used as a signal for whether tie_word_embeddings should be copied from hf_config to the language_model config.

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request refines the logic within the with_hf_config method in vllm/config/vllm.py by simplifying the conditional check for tie_word_embeddings in multimodal models. The change removes a redundant check for the absence of tie_word_embeddings in the text configuration. Additionally, the accompanying comments have been significantly expanded to provide a more detailed and accurate explanation of how tie_word_embeddings should be handled across different multimodal model architectures, clarifying its relationship between the main model configuration and the language model's specific configuration. There is no feedback to provide from the review comments.

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) March 25, 2026 02:33
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 25, 2026
hmellor added 2 commits March 25, 2026 11:25
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@DarkLight1337
DarkLight1337 merged commit 1e673a4 into vllm-project:main Mar 25, 2026
51 checks passed
@hmellor
hmellor deleted the fix-weight-tying branch March 25, 2026 12:16
RhizoNymph pushed a commit to RhizoNymph/vllm that referenced this pull request Mar 26, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
nithinvc pushed a commit to nithinvc/vllm that referenced this pull request Mar 27, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>

Signed-off-by: Nithin Chalapathi <nithin.ch10@gmail.com>
JiantaoXu pushed a commit to JiantaoXu/vllm that referenced this pull request Mar 28, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
puririshi98 pushed a commit to puririshi98/vllm that referenced this pull request Apr 7, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
mtparet pushed a commit to blackfuel-ai/vllm that referenced this pull request Apr 9, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
mystous pushed a commit to mystous/vllm_hybrid that referenced this pull request May 10, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
my-other-github-account pushed a commit to my-other-github-account/vllm that referenced this pull request May 15, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants