Rename clashing method names for vLLM model protocol #27583
Conversation
Signed-off-by: Harry Mellor <[email protected]>
hmellor
requested review from
DarkLight1337,
NickLucche,
benchislett,
luccafong,
patrickvonplaten,
sighingnow and
ywang96
as code owners
|
Documentation preview: https://vllm--27583.org.readthedocs.build/en/27583/ |
mergify
bot
added
documentation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| def _check_vllm_model_embed_input_ids(model: type[object] | object) -> bool: | ||
| model_embed_input_ids = getattr(model, "embed_input_ids", None) | ||
| if not callable(model_embed_input_ids): | ||
| model_get_input_embeddings = getattr(model, "get_input_embeddings", None) | ||
| if callable(model_get_input_embeddings): | ||
| logger.warning( | ||
| "`get_input_embeddings` for vLLM models is deprecated and will be " | ||
| "removed in v0.13.0 or v1.0.0, whichever is earlier. Please rename " | ||
| "this method to `embed_input_ids`." | ||
| ) | ||
| model.embed_input_ids = model_get_input_embeddings | ||
| logger.warning( | ||
| "The model (%s) is missing the `get_input_embeddings` method.", | ||
| "The model (%s) is missing the `embed_input_ids` method.", | ||
| model, | ||
| ) | ||
| return False |
There was a problem hiding this comment.
The new _check_vllm_model_embed_input_ids always returns False when a model still exposes the deprecated get_input_embeddings method, even after binding it to embed_input_ids. As a result, is_vllm_model will reject any third‑party models that follow the old interface, contradicting the intended deprecation fallback and potentially breaking out‑of‑tree integrations until they rename their method. The check should return True when the legacy method exists and is wired up, otherwise the compatibility path never actually works.
Useful? React with 👍 / 👎.
|
As per discussion with @ywang96, I'll be pausing work on this until an ongoing new model is added as to not disrupt it. |
Signed-off-by: Harry Mellor <[email protected]>
Signed-off-by: Harry Mellor <[email protected]>
github-project-automation
bot
moved this from To Triage
to Ready
in gpt-oss Issues & Enhancements
DarkLight1337
added
the
ready
|
Failing extended pooling test also failed last night's nightly https://buildkite.com/vllm/ci/builds/38575/steps/canvas?sid=019a7670-4e2e-49b6-8137-1777aa2125f5 |
|
Failing mm extended 1 test also failed last night's nightly https://buildkite.com/vllm/ci/builds/38575/steps/canvas?sid=019a7670-4e31-45f4-867d-a91104a9c070 Failing mm extended 3 test also failed last night's nightly https://buildkite.com/vllm/ci/builds/38575/steps/canvas?sid=019a7670-4e32-4856-89cf-83eaf9f00b2c |
github-project-automation
bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
4 participants
get_input_embeddingsis a getter method of alltransformers.PreTrainedModels.This name had been reused in vLLM to call
forwardon the model's embedding layer.Since the method in vLLM is not actually a getter and causes a confusing clash with the getter in Transformers, this PR:
get_input_embeddingstoembed_input_idsget_multimodal_embeddingstoembed_multimodalfor consistencyVllmModelandSupportsMultiModalso that third party vLLM models should still work