[Bugfix] handle alignment of encoder_seq_lens in mllama.py #14784

tjohnson31415 · 2025-03-13T22:26:00Z

Fix for the crash repro reported in this comment. The bug and fix are pretty similar to #12347 in that the problem arises in a batch of mixed text and image requests leading to the attn metadata having some lists with an element for each sequence and others with an element only for each sequence-with-images.

Another fix that came out of #10648

FIX #10648

github-actions · 2025-03-13T22:26:11Z

👋 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.

🚀

tjohnson31415 · 2025-03-24T20:20:21Z

vllm/model_executor/models/mllama.py

input_processor_for_mllama has been removed and I don't see our use of the Transformers processor doing this same trick to only process the last image group.

I'm working to undersatnd this better, but I think we either should re-implement the "cheat" or remove the extra complexity (i.e. this check comparing to num tiles and remove kv_range_for_decode).

I'm checking with the author of PR #11427 which removes the previous input processor. See the discussion here.
https://vllm-dev.slack.com/archives/C07QCGVDNUF/p1743001000770969

This cheat is implementing here #15564

mergify · 2025-03-28T15:16:25Z

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

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

heheda12345 · 2025-04-01T06:49:09Z

In general this PR looks good. Thanks for the fix. Can you sync this PR with main to get in recent bug fixes for mllama #15564 and #14883 and test it again?

Signed-off-by: Travis Johnson <[email protected]>

tjohnson31415 · 2025-04-10T06:53:28Z

@heheda12345 I rebased and updated the branch.
tests/models/encoder_decoder/vision_language/test_mllama.py tests all pass for me.

Ready for another look!

heheda12345

LGTM in general. Only some nits.

vllm/model_executor/models/mllama.py

Signed-off-by: Travis Johnson <[email protected]>

tjohnson31415 · 2025-04-11T16:56:58Z

@heheda12345 Thanks for the review! I pushed the suggested changes (the linter didn't like l as a var name so used x instead). PTAL

heheda12345

Thanks for the bug fix!

…ect#14784) Signed-off-by: Travis Johnson <[email protected]> Signed-off-by: Yang Wang <[email protected]>

…ect#14784) Signed-off-by: Travis Johnson <[email protected]>

…ect#14784) Signed-off-by: Travis Johnson <[email protected]> Signed-off-by: Mu Huai <[email protected]>

tjohnson31415 force-pushed the fix-mllama-crash branch from d289902 to 8e6903c Compare March 13, 2025 22:35

tjohnson31415 mentioned this pull request Mar 13, 2025

[Bug]: Llama 3.2 90b crash #10648

Closed

1 task

tjohnson31415 force-pushed the fix-mllama-crash branch from 8e6903c to fb1d347 Compare March 21, 2025 22:44

tjohnson31415 marked this pull request as ready for review March 24, 2025 19:38

tjohnson31415 requested review from DarkLight1337 and ywang96 as code owners March 24, 2025 19:38

tjohnson31415 commented Mar 24, 2025

View reviewed changes

DarkLight1337 requested a review from heheda12345 March 25, 2025 04:30

mergify bot added the needs-rebase label Mar 28, 2025

tjohnson31415 force-pushed the fix-mllama-crash branch from fb1d347 to 582552e Compare April 7, 2025 23:37

mergify bot removed the needs-rebase label Apr 7, 2025

tjohnson31415 force-pushed the fix-mllama-crash branch from 582552e to 5b257d9 Compare April 9, 2025 05:37

tjohnson31415 added 4 commits April 10, 2025 00:20

test: add another mllama regression case

7ac9b76

Signed-off-by: Travis Johnson <[email protected]>

fix mllama assertion check

f9582a1

Signed-off-by: Travis Johnson <[email protected]>

little refactor and add CI test

62b7772

Signed-off-by: Travis Johnson <[email protected]>

fix: get test_mllama.py passing

aa6d40d

Signed-off-by: Travis Johnson <[email protected]>

tjohnson31415 force-pushed the fix-mllama-crash branch from 5b257d9 to aa6d40d Compare April 10, 2025 06:40

heheda12345 reviewed Apr 10, 2025

View reviewed changes

vllm/model_executor/models/mllama.py Outdated Show resolved Hide resolved

vllm/model_executor/models/mllama.py Outdated Show resolved Hide resolved

commit review suggestions

c0d2885

Signed-off-by: Travis Johnson <[email protected]>

heheda12345 approved these changes Apr 11, 2025

View reviewed changes

heheda12345 enabled auto-merge (squash) April 11, 2025 18:03

github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Apr 11, 2025

heheda12345 merged commit 71b9cde into vllm-project:main Apr 11, 2025
61 checks passed

tjohnson31415 deleted the fix-mllama-crash branch April 11, 2025 22:08

yangw-dev pushed a commit to yangw-dev/vllm that referenced this pull request Apr 21, 2025

[Bugfix] handle alignment of encoder_seq_lens in mllama.py (vllm-proj…

a1412eb

…ect#14784) Signed-off-by: Travis Johnson <[email protected]> Signed-off-by: Yang Wang <[email protected]>

jikunshang pushed a commit to jikunshang/vllm that referenced this pull request Apr 29, 2025

[Bugfix] handle alignment of encoder_seq_lens in mllama.py (vllm-proj…

69a8219

…ect#14784) Signed-off-by: Travis Johnson <[email protected]>

lk-chen pushed a commit to lk-chen/vllm that referenced this pull request Apr 29, 2025

[Bugfix] handle alignment of encoder_seq_lens in mllama.py (vllm-proj…

300c23b

…ect#14784) Signed-off-by: Travis Johnson <[email protected]>

RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request May 12, 2025

[Bugfix] handle alignment of encoder_seq_lens in mllama.py (vllm-proj…

abdd700

…ect#14784) Signed-off-by: Travis Johnson <[email protected]> Signed-off-by: Mu Huai <[email protected]>

Uh oh!

[Bugfix] handle alignment of encoder_seq_lens in mllama.py #14784

[Bugfix] handle alignment of encoder_seq_lens in mllama.py #14784

Uh oh!

Conversation

tjohnson31415 commented Mar 13, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 13, 2025

Uh oh!

tjohnson31415 Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

heheda12345 Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

heheda12345 Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Mar 28, 2025

Uh oh!

heheda12345 commented Apr 1, 2025

Uh oh!

tjohnson31415 commented Apr 10, 2025

Uh oh!

heheda12345 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tjohnson31415 commented Apr 11, 2025

Uh oh!

heheda12345 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tjohnson31415 commented Mar 13, 2025 •

edited by github-actions bot

Loading