Support embedding models in V1 #16188
Conversation
Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
Encoder-only models can also benefit from the prefix caching that is enabled by the kv cache Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
This is only passing mypy, it hasn't been tested yet Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
... and disable cuda graphs for these models. Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
|
👋 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 🚀 |
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
There was a problem hiding this comment.
I'm done with my initial review. Will test later. Can @njhill @robertgshaw2-redhat also take a look at this?
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
Signed-off-by: Max de Bayser <maxdebayser@gmail.com>
| # chunked prefill has to be enabled explicitly to allow | ||
| # pooling requests to be chunked | ||
| if not self.scheduler_config.chunked_prefill_enabled and \ | ||
| num_new_tokens > token_budget: | ||
| self.waiting.popleft() | ||
| skipped_waiting_requests.appendleft(request) | ||
| continue |
There was a problem hiding this comment.
I'm not sure this could potentially cause a starvation problem, but it seems to be ok at least for the pooling/embedding models
There was a problem hiding this comment.
I'm not sure, because the requests that skip ahead of the one that doesn't fit in the budget are processed and leave the waiting queue, right? So in theory, in the next call to schedule the skipped request will be in front of the queue and should be processed unless it was to big to fit max_model_len or max_batch_tokens to begin with. And if chunked prefill is disabled, there will never be running requests so the token budget should still be 100% when schedule reaches the processing of the waiting queue.
Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
|
This pull request has merge conflicts that must be resolved before it can be |
There was a problem hiding this comment.
@maxdebayser @22quinn LGTM overall. Thanks again for all the efforts in this PR!
Left some minor comments. Please take a look.
Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
|
@WoosukKwon , one of the failing tests depends on this fix: #19686 . |
regarding the error from #19446, here's the follow-up: #19715 that should address cases that weren't caught in the CI run on #19446 ... |
|
Thanks, @russellb! |
Signed-off-by: Max de Bayser <mbayser@br.ibm.com>
|
Thanks for your hard work! Can you update the V1 guide with the latest status? |
|
Thanks @DarkLight1337 and @22quinn for all your help. Is the doc update by @22quinn ok for now? |
| # No Embedding Models so far. | ||
| if model_config.task not in ["generate"]: | ||
| _raise_or_fallback(feature_name=f"--task {model_config.task}", | ||
| recommend_to_remove=False) | ||
| return False |
There was a problem hiding this comment.
Did you mean to enable all tasks here?
TaskOption = Literal["auto", "generate", "embedding", "embed", "classify",
"score", "reward", "transcription"]
There was a problem hiding this comment.
I'll double check the "transcription" task, but the others yes. Is this causing a problem?
There was a problem hiding this comment.
Nope, it just caused a conflcit in my branch where I had enabled "transcription" and thought maybe it had enabled more than you intended. It's fine if you mean to!
I'm not sure about transcription, either. I know it wouldn't work with whisper, but that'll still get blocked because the model is marked as v0-only. Since all models should have the V0-only marker where needed, this check probably isn't necessary.
Yeah it looks good! |
Signed-off-by: Max de Bayser <mbayser@br.ibm.com> Signed-off-by: Max de Bayser <maxdebayser@gmail.com> Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: 22quinn <33176974+22quinn@users.noreply.github.com>
Signed-off-by: Max de Bayser <mbayser@br.ibm.com> Signed-off-by: Max de Bayser <maxdebayser@gmail.com> Signed-off-by: 22quinn <33176974+22quinn@users.noreply.github.com> Co-authored-by: 22quinn <33176974+22quinn@users.noreply.github.com> Signed-off-by: minpeter <kali2005611@gmail.com>
This PR proposes an implementation path to support embedding models in V1. The basic idea is to take a different path than in V0 where there were completely different code bases for decoder models and encoder only models. Since the difference between decoder and encoder only models boils down to the attention mask and what is done to the hidden states at the end, in this PR I'm trying to add the minimum amount of changes required to the engine, the scheduler and the model runner to get embeddings working end-to-end.
In the current shape if this PR, Bert models are already running. The next steps are to enable the other embedding models in our unit tests.
The goal of this PR is to get V1 in a shape from where we can refactor the code base towards the a generic hidden states processor implementation: #12249
cc: @DarkLight1337