Repository navigation
Add embedding input functionality for disabled modalities - #31506
reaganjlee wants to merge 4 commits into
Conversation
|
👋 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 You ask your reviewers to trigger select CI tests on top of 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 If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. 🚀 |
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to allow providing pre-computed embeddings for a multimodal model even when the corresponding modality's encoder is disabled. This is a useful feature for saving memory. The changes span across the multimodal registry, processing logic, cache management, and model runners to correctly handle this new "embeddings-only" mode. The tests have also been updated to cover this new behavior.
My review found one critical issue in a test utility function that could lead to incorrect test behavior. Otherwise, the core logic and implementation appear sound.
5aed201 to
a5d6cb1
Compare
|
Hi @reaganjlee, the pre-commit checks have failed. Please run: uv pip install pre-commit
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
There was a problem hiding this comment.
💡 Codex Review
https://github.com/vllm-project/vllm/blob/66acef20fc59a3d50ef7ca811f463b571aff0dee/v1/core/encoder_cache_manager.py#L324-L337
Avoid returning a 1-token budget for embedding-only mode
When all modality limits are 0 but enable_mm_embeds=True, this returns an encoder compute budget/cache size of 1. The scheduler uses encoder_compute_budget to decide whether it can schedule a multimodal item based on the number of placeholder embeds (e.g., image/video embeds are typically far >1). That means any real embedding input will fail can_allocate, so the encoder step is never scheduled and the request can stall before the placeholder or proceed without inserting the provided embeddings. To support embedding-only mode, the budget needs to allow the actual number of embeds per item (or bypass the encoder budget check for embedding inputs) rather than forcing 1.
ℹ️ 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".
|
This pull request has merge conflicts that must be resolved before it can be |
|
Migrating to #32493 |
Purpose
status: found bug in implementation
Resolves part of #21943. Previously, a change was made to implicitly make a model language-model only (#22299) when all modalities had limits set to 0. However, some users may want to input embeddings as an input while still saving on memory from the unused encoder. This changes behavior to allow this.
Test Plan
Running Qwen2.5VL-7B on L40S
Command:
vllm serve Qwen/Qwen2.5-VL-7B-InstructCommand:
vllm serve Qwen/Qwen2.5-VL-7B-Instruct --limit-mm-per-prompt '{"image": 0, "video": 0}'Command:
vllm serve Qwen/Qwen2.5-VL-7B-Instruct --limit-mm-per-prompt '{"image": 0, "video": 0}' --enable-mm-embedsTest Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.