Skip to content

[V1] Scatter and gather placeholders in the model runner #16076

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
merged 39 commits into from
Apr 8, 2025
Merged

[V1] Scatter and gather placeholders in the model runner #16076

merged 39 commits into from
Apr 8, 2025

Conversation

ywang96
Copy link
Member

@ywang96 ywang96 commented Apr 4, 2025 •
edited by github-actions bot
Loading

Reopened from accidentally merged #15712

This PR is an attempt to move scatter_patch_features and gather_patch_feature into the model runner (outside of the model) to avoid interfering with TPU graph compilation.

Breaking change for model developers:

  • PromptUpdateDetails.features has been replaced with PromptUpdateDetails.is_embed. You can use the newly added factories PromptUpdateDetails.select_text and PromptUpdateDetails.select_token_id to generate is_embed based on the target text/token ID.
  • BaseProcessingInfo.get_num_image_tokens should now return the equivalent of PromptUpdateDetails.is_embed.sum() instead of the number of tokens in PromptUpdateDetails.features.

Originally authored by @DarkLight1337; co-authored by @mgoin

DarkLight1337 and others added 26 commits March 31, 2025 16:41
@DarkLight1337
[V1] Scatter and gather placeholders in the model runner
9e4e312 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix placeholder token calculation
d2ad7e6 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Improve error message
776074b 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Loosen check
d8c10c8 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix Molmo
6257621 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Rename
6096406 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix gemma3
d1568ad 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Skip h2ovl for current transformers
a68228e 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix MiniCPM
cb3ccce 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix Fuyu
cff414e 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Merge branch 'main' into v1-is-embed
0cd2a54
@DarkLight1337
Fix NVLM
699c346 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Enable PlaceholderRange equality check
cd86861 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix
f9b92df 
Signed-off-by: DarkLight1337 <[email protected]>
@mgoin
Fix pos_info.is_embed
53f5507 
Signed-off-by: mgoin <[email protected]>
@DarkLight1337
Fix Idefics3
f798d89 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Merge branch 'main' into v1-is-embed
7935a8d
@DarkLight1337
Remove embed_is_patch from new models
06bf57d 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix missing replacement
df174e6 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix Pixtral token calculation and update docs
1e9fb1b 
Signed-off-by: DarkLight1337 <[email protected]>
Merge remote-tracking branch 'upstream/main' into v1-is-embed
142baf4 
Signed-off-by: Roger Wang <[email protected]>
@DarkLight1337
Fix CI
e3ec92d 
Signed-off-by: DarkLight1337 <[email protected]>
increase default timeout for testing
6d51118 
Signed-off-by: Roger Wang <[email protected]>
typo
4940d16 
Signed-off-by: Roger Wang <[email protected]>
lower max model len
522833b 
Signed-off-by: Roger Wang <[email protected]>
update pixtral test
b64983c 
Signed-off-by: Roger Wang <[email protected]>
@DarkLight1337 DarkLight1337 self-assigned this Apr 5, 2025
@DarkLight1337 DarkLight1337 moved this to In Progress in Multi-modality Core Apr 5, 2025
Roger Wang and others added 3 commits April 5, 2025 01:22
lower batch size for aya
604cb0e 
Signed-off-by: Roger Wang <[email protected]>
@DarkLight1337
Avoid OOM
fbc9345 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Skip Phi-3.5-Vision tests
2b16f91 
Signed-off-by: DarkLight1337 <[email protected]>
This was referenced Apr 5, 2025
Merged
Merged
@ywang96
Copy link
Member Author

ywang96 commented Apr 6, 2025

I'm holding on merging this PR until #16113 and #16117 are merged.

@DarkLight1337 DarkLight1337 mentioned this pull request Apr 6, 2025
Merged
DarkLight1337 added a commit to houseroad/vllm that referenced this pull request Apr 7, 2025
@DarkLight1337
Apply fixes from vllm-project#16076
d76eac1 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Merge branch 'main' into v1-is-embed
4bd0025 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Update llama4
edd66c6 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Copy link
Member

DarkLight1337 commented Apr 7, 2025 •
edited
Loading

Verifying llama4 now...

@DarkLight1337
Update tests
c0300d2 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix
f837467 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix
4071176 
Signed-off-by: DarkLight1337 <[email protected]>
@DarkLight1337
Fix
9e2b427 
Signed-off-by: DarkLight1337 <[email protected]>
@mergify Mergify
Copy link

mergify bot commented Apr 7, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @ywang96.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Apr 7, 2025
@mergify mergify bot removed the needs-rebase label Apr 7, 2025
@DarkLight1337 DarkLight1337 merged commit f2ebb6f into main Apr 8, 2025
45 checks passed
@DarkLight1337 DarkLight1337 deleted the v1-is-embed branch April 8, 2025 02:43
@github-project-automation github-project-automation bot moved this from In Progress to Done in Multi-modality Core Apr 8, 2025
@DarkLight1337 DarkLight1337 mentioned this pull request Apr 8, 2025
Draft
nishith-fujitsu pushed a commit to nishith-fujitsu/vllm that referenced this pull request Apr 9, 2025
@ywang96 @DarkLight1337
@mgoin @JenZhao @nishith-fujitsu
[V1] Scatter and gather placeholders in the model runner (vllm-projec…
a12faed 
…t#16076)

Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: mgoin <[email protected]>
Signed-off-by: Roger Wang <[email protected]>
Co-authored-by: DarkLight1337 <[email protected]>
Co-authored-by: mgoin <[email protected]>
Co-authored-by: Jennifer Zhao <[email protected]>
yangw-dev pushed a commit to yangw-dev/vllm that referenced this pull request Apr 21, 2025
@ywang96 @DarkLight1337
@mgoin @JenZhao @yangw-dev
[V1] Scatter and gather placeholders in the model runner (vllm-projec…
1084d5e 
…t#16076)

Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: mgoin <[email protected]>
Signed-off-by: Roger Wang <[email protected]>
Co-authored-by: DarkLight1337 <[email protected]>
Co-authored-by: mgoin <[email protected]>
Co-authored-by: Jennifer Zhao <[email protected]>
Signed-off-by: Yang Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DarkLight1337 DarkLight1337 DarkLight1337 approved these changes

@WoosukKwon WoosukKwon Awaiting requested review from WoosukKwon WoosukKwon is a code owner

@robertgshaw2-redhat robertgshaw2-redhat Awaiting requested review from robertgshaw2-redhat robertgshaw2-redhat is a code owner

@njhill njhill Awaiting requested review from njhill njhill is a code owner

@comaniac comaniac Awaiting requested review from comaniac comaniac is a code owner

@alexm-redhat alexm-redhat Awaiting requested review from alexm-redhat alexm-redhat is a code owner

Assignees

@DarkLight1337 DarkLight1337

Labels
documentation Improvements or additions to documentation multi-modality Related to multi-modality (#4194) ready ONLY add when PR is ready to merge/full CI is needed tpu Related to Google TPUs v1
Projects
Multi-modality Core
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ywang96 @DarkLight1337 @mgoin