[Bugfix] Implement multimodal_cpu_fields in model runner#5196
[Bugfix] Implement multimodal_cpu_fields in model runner#5196wangxiyuan merged 1 commit intovllm-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to ensure specific multimodal data fields are on the CPU before being used by the model, which is a good refactoring of model-specific logic into a more general component. My main feedback is to make the list of CPU-bound fields configurable rather than hardcoded to improve maintainability and support for future models.
0c72cb7 to
a57d5f8
Compare
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
|
Please delete the whole file |
a57d5f8 to
70a2ddc
Compare
|
According to vllm-project/vllm#28168, I suppose this Besides, we should avoid letting users to manually configure this by passing an |
|
Currently, this |
70a2ddc to
451667a
Compare
Signed-off-by: hfadzxy <starmoon_zhang@163.com>
451667a to
721287c
Compare
|
LGTM if all CI passed. Note that we need to remove this logic after this field is deprecated in vllm. |
done |
Yes, I deleted multimodal_cpu_fields from additional_config. |
|
CC @wangxiyuan All CI passed. |
…t#5196) ### What this PR does / why we need it? Related to vllm-project#4084 Implement multimodal_cpu_fields in model runner - vLLM version: v0.12.0 - vLLM main: vllm-project/vllm@ad32e3e Signed-off-by: hfadzxy <starmoon_zhang@163.com> Signed-off-by: zrj026 <zhangrunjiang026@gmail.com>
…t#5196) ### What this PR does / why we need it? Related to vllm-project#4084 Implement multimodal_cpu_fields in model runner - vLLM version: v0.12.0 - vLLM main: vllm-project/vllm@ad32e3e Signed-off-by: hfadzxy <starmoon_zhang@163.com> Signed-off-by: zrj026 <zhangrunjiang026@gmail.com>
What this PR does / why we need it?
Related to #4084 Implement multimodal_cpu_fields in model runner
Does this PR introduce any user-facing change?
How was this patch tested?