[Misc] Make MM embedding merge interface explicit in model runner #21147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

DarkLight1337 merged 3 commits into main from mm-embeds

Jul 18, 2025

Member

ywang96 commented Jul 17, 2025 •

edited by github-actions bot

Loading

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.

Purpose

Make the get_input_embeddings() call for multimodal models explicit in model runner so that its interface will be strictly enforced.

Test Plan

Test Result

(Optional) Documentation Update

add

d8dd2d5

Signed-off-by: Roger Wang <hey@rogerw.me>

ywang96 requested review from WoosukKwon, robertgshaw2-redhat, njhill, comaniac and alexm-redhat as code owners

July 17, 2025 22:45

github-actions bot commented Jul 17, 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 can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

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.

🚀

mergify bot added the v1 label

ywang96 requested review from DarkLight1337 and removed request for comaniac, njhill, alexm-redhat and robertgshaw2-redhat

July 17, 2025 22:45

gemini-code-assist bot reviewed

View reviewed changes

Contributor

gemini-code-assist bot left a comment

Code Review

This pull request refactors the call to get_input_embeddings to use explicit keyword arguments, which improves readability. However, the change introduces a subtle alteration in behavior for empty multimodal embeddings that could lead to runtime errors or incorrect outputs in some models. I've suggested a fix to preserve the original logic while maintaining the use of explicit keyword arguments.

vllm/v1/worker/gpu_model_runner.py Outdated Show resolved Hide resolved


          Update vllm/v1/worker/gpu_model_runner.py

9f6e063

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

DarkLight1337 reviewed

View reviewed changes

Member

DarkLight1337 left a comment

Can you also update TPU model runner?


          update TPU runner

ee7af2a

Signed-off-by: Roger Wang <hey@rogerw.me>

Member Author

ywang96 commented Jul 18, 2025

Can you also update TPU model runner?

Done in ee7af2a

mergify bot added the tpu label

DarkLight1337 approved these changes

View reviewed changes

DarkLight1337 enabled auto-merge (squash)

July 18, 2025 04:12

github-actions bot added the ready label

DarkLight1337 merged commit ba2dfbb into main

63 of 65 checks passed

DarkLight1337 deleted the mm-embeds branch

July 18, 2025 07:13

WorldExplored pushed a commit to nadathurv/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

9fdfb7f

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

WorldExplored pushed a commit to nadathurv/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

e6a905d

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: WorldExplored <srreyansh.sethi@gmail.com>

WorldExplored pushed a commit to nadathurv/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

a412355

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: WorldExplored <srreyansh.sethi@gmail.com>

hj-mistral pushed a commit to hj-mistral/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

cb63b0e

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Himanshu Jaju <hj@mistral.ai>

LyrisZhong pushed a commit to LyrisZhong/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

94d4528

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

avigny pushed a commit to avigny/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

d7433c2

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: avigny <47987522+avigny@users.noreply.github.com>

x22x22 pushed a commit to x22x22/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: x22x22 <wadeking@qq.com>

Pradyun92 pushed a commit to Pradyun92/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

25f8096

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

npanpaliya pushed a commit to odh-on-pz/vllm-upstream that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

63ec1f0

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

jinzhen-lin pushed a commit to jinzhen-lin/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

611c03f

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Jinzhen Lin <linjinzhen@hotmail.com>

paulpak58 pushed a commit to paulpak58/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

1b3ba60

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Paul Pak <paulpak58@gmail.com>

taneem-ibrahim pushed a commit to taneem-ibrahim/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

b72d7a9

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

diegocastanibm pushed a commit to diegocastanibm/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

894008a

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Diego-Castan <diego.castan@ibm.com>

epwalsh pushed a commit to epwalsh/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

cb3c126

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

googlercolin pushed a commit to googlercolin/vllm that referenced this pull request


          [Misc] Make MM embedding merge interface explicit in model runner (vl…

d3083a5

…lm-project#21147)

Signed-off-by: Roger Wang <hey@rogerw.me>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels