[Refactor] Define MultiModalKwargsItems separate from MultiModalKwargs #23053
Conversation
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
DarkLight1337
added
the
ready
DarkLight1337
requested review from
WoosukKwon,
alexm-redhat,
comaniac,
hmellor,
njhill,
patrickvonplaten,
robertgshaw2-redhat,
sighingnow and
ywang96
as code owners
|
👋 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 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 🚀 |
mergify
bot
added
documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors MultiModalKwargs into two distinct classes, MultiModalKwargsItems and MultiModalKwargs, to better separate structured, per-item data from batched tensor data. The changes are consistently applied throughout the codebase, improving clarity and maintainability. I've found one critical issue in a test case where a copy-paste error makes a check ineffective.
|
/gemini |
|
Hi @DarkLight1337! It looks like you've invoked me without a specific command. If you'd like me to perform a code review, please use |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors MultiModalKwargs into two distinct classes, MultiModalKwargsItems and MultiModalKwargs, to improve clarity and simplify the data structures for handling multi-modal inputs. MultiModalKwargsItems now represents a modality-keyed dictionary of items, while MultiModalKwargs holds the final batched tensor data for the model. The changes are propagated throughout the codebase, including numerous model implementations and testing utilities, to adopt the new get_data() method for converting between these types. The refactoring is well-executed and significantly enhances the maintainability of the multi-modal components. My review includes one high-severity suggestion to restore a removed assertion, ensuring robustness against potential runtime errors.
DarkLight1337
force-pushed
the
separate-mm-kwargs
branch
from
c365514 to
b6b0ff8
Compare
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
force-pushed
the
separate-mm-kwargs
branch
from
a56a1e8 to
946958a
Compare
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk> Signed-off-by: Duncan Moss <djm.moss@gmail.com>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk> Signed-off-by: Xiao Yu <xiao.yu@amd.com>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
vllm-project#23053) Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
2 participants
Purpose
More refactoring to enable #23018
MultiModalKwargsItems(a modality-keyed dictionary ofMultiModalKwargsItem) andMultiModalKwargs(a model argument-keyed dictionary of tensor data), in order to avoid confusion. As a result of this change, these classes can now be greatly simplified to be little more than a thin wrapper over a regular dictionary.MultiModalKwargsItems.get_data()is used to constructMultiModalKwargsget_dataexplicitly in_get_prompt_updatesFix
NestedTensorsnot defined from #23070Test Plan
Unblocked all MM tests to verify that the models still work correctly.
Test Result
(Optional) Documentation Update
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.