Remove mm_prefix_lm patch because vllm==0.18.0 already support#2047
Conversation
Signed-off-by: princepride <wangzhipeng628@gmail.com>
|
let's wait for the upgrate alignment with upsteam |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ad494b103
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| import sys | ||
| from functools import cached_property as _cached_property | ||
|
|
||
| from aenum import extend_enum |
There was a problem hiding this comment.
Keep Bagel mm_prefix_lm patch for vLLM 0.17 users
This change removes the ModelConfig.is_mm_prefix_lm override in vllm_omni.patch, but the repo still documents and builds against vLLM 0.17 (docs/getting_started/quickstart.md:22-25, docker/Dockerfile.ci:2). In those environments Bagel is not in upstream MM_PREFIX_LM_MODELS, so dropping this fallback disables the bidirectional-prefix behavior Bagel needs and can break Bagel inference paths. Either keep the patch for pre-0.18 versions or gate this removal behind an enforced minimum vLLM version.
Useful? React with 👍 / 👎.
Purpose
In vllm, I already add "bagel" in MM_PREFIX_LM_MODELS list: vllm-project/vllm#36316, we can find it in vllm branch v0.18.0: https://github.com/vllm-project/vllm/blob/bcf2be96120005e9aea171927f85055a6a5c0cf6/vllm/config/model.py#L1145, so I revert the patch.