[bugfix] add supports_v1 platform interface #15417
Conversation
Signed-off-by: Joe Runde <Joseph.Runde@ibm.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 🚀 |
|
cc @youkaichao are the ascend folks okay with this? |
vllm/engine/arg_utils.py
Outdated
| # TPU (experimental) so far. Out-of-tree device support plugins can | ||
| # maintain their own v1 compatibility checks. | ||
| if not (current_platform.is_cuda_alike() or current_platform.is_tpu() | ||
| or current_platform.is_out_of_tree()): |
There was a problem hiding this comment.
Maybe we should add a supports_v1() to Platform (defaulting to false) instead of this, so we don't break fallbacks for other out-of-tree platforms
There was a problem hiding this comment.
Yeah I'd be fine with that as well. I didn't do that here at first because I didn't want to push this centralized bit of logic back out to the different platforms in-tree, but that's not a huge deal.
so we don't break fallbacks
Yeah... I get that we don't want to turn on V1 where it won't work, but this explicitly denies it where in our case it does work! 😅 It should be the responsibility of the plugin to determine compatibility, so I'm fine with either this or a platform.supports_v1() flag. I'l leave some time for more comments and check back on this tomorrow morning
There was a problem hiding this comment.
I have a comment here #15263 (comment)
we should add platform.supports_v1()
There was a problem hiding this comment.
in addition, to give oot platforms the same ability of falling back, we should pass the config object to the supports_v1 function.
There was a problem hiding this comment.
in addition, to give oot platforms the same ability of falling back, we should pass the config object to the
supports_v1function.
Agree 👍
vllm-ascend has a initial support for V1 Engine, we'll test this pr on v1 with vllm-ascend. Thanks for this pr! |
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com>
joerunde
changed the title
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com>
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com>
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com>
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com> Signed-off-by: Wes Medford <wryanmedford@gmail.com>
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com> Signed-off-by: Louis Ulmer <ulmerlouis@gmail.com>
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com>
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com>
Signed-off-by: Joe Runde <Joseph.Runde@ibm.com> Signed-off-by: Mu Huai <tianbowen.tbw@antgroup.com>
5 participants
This PR re-allows platform plugins to run on V1. I'm assuming that all out-of-tree platforms are responsible for all of their own validation at startup to ensure the provided configuration will work, so vllm shouldn't disallow them from running.
Our plugin currently doesn't boot with v0.8.1 (and presumably won't with the upcoming v0.8.2), this little fix allows it to start again.
FIX vllm-project/vllm-spyre#38