Repository navigation
Don't compile vision encoder for Transformers backend - #30518
Merged
hmellor merged 40 commits intoApr 2, 2026
Merged
Conversation
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request aims to prevent the vision encoder from being compiled when using the Transformers backend. The current implementation modifies the encoder class globally, which can lead to unintended side effects. My review suggests a safer approach by creating a subclass for the specific encoder instance to avoid global state modification.
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
…ompile` Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
…e `Base` Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
hmellor
marked this pull request as ready for review
January 2, 2026 18:25
hmellor
requested review from
WoosukKwon,
mgoin,
robertgshaw2-redhat,
tlrmchlsmth and
youkaichao
as code owners
January 2, 2026 18:25
5 tasks
HenryTangDev
pushed a commit
to HenryTangMain/vllm
that referenced
this pull request
Apr 6, 2026
…0518) Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
puririshi98
pushed a commit
to puririshi98/vllm
that referenced
this pull request
Apr 7, 2026
…0518) Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> Signed-off-by: Rishi Puri <riship@nvidia.com>
mtparet
pushed a commit
to blackfuel-ai/vllm
that referenced
this pull request
Apr 9, 2026
…0518) Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
khluu
pushed a commit
that referenced
this pull request
Apr 16, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> (cherry picked from commit dfe5e31)
khluu
pushed a commit
that referenced
this pull request
Apr 17, 2026
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> Signed-off-by: khluu <khluu000@gmail.com>
mystous
pushed a commit
to mystous/vllm_hybrid
that referenced
this pull request
May 10, 2026
…0518) Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
my-other-github-account
pushed a commit
to my-other-github-account/vllm
that referenced
this pull request
May 15, 2026
…0518) Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
my-other-github-account
pushed a commit
to my-other-github-account/vllm
that referenced
this pull request
May 15, 2026
…0518) Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> Signed-off-by: khluu <khluu000@gmail.com>
vadiklyutiy
added a commit
to vadiklyutiy/vllm
that referenced
this pull request
Oct 8, 2026
…ile positional arg check <!-- markdownlint-disable --> ## Overview Fixes vllm-project/vllm issue 60202. `@support_torch_compile` wraps `__init__` and checks its positional arguments (the check came in with vllm-project#30518). That check compared each argument with the annotation of the parameter before it, and it crashed inside `isinstance` on generic annotations. Now each argument is checked against its own parameter. The check runs only when the annotation is a plain class that is not a config class. vllm-project#60214 is also open for this issue. See "Relation to vllm-project#60214" under Details for how this PR differs. ## Claims - `Block(8, "gelu")` with `__init__(self, dim: int, act: str)` no longer raises the wrapper's `TypeError`. - Annotations that `isinstance` cannot take are now skipped, so positional arguments for them no longer make `isinstance` raise. These are parameterized generics (`Callable[[torch.Tensor], torch.Tensor]`, `list[int]`), unions and string annotations. - A wrong type for a plain class annotation is still refused with the same message, for example `"8"` for `dim: int`. - Config annotations (subclasses of `PreTrainedConfig`) are not checked, so Transformers-backend models that build today still build. A sweep of all Transformers 5.18 model types found none that is newly refused. One encoder (`t5gemma2`) was refused by this bug, and it now builds. ## Validation **Reproducer.** The issue's script, unchanged (it is in the collapsed block below), run as `python -P repro.py`. Setup: 1× NVIDIA B200 (the script runs no CUDA code), vLLM `43b4aaea3`, Python 3.12, torch 2.13.0+cu130, transformers 5.18.0. Before the fix there were 2 runs, both with the same output. One was in a venv at `43b4aaea3`. The other was in the `vllm/vllm-openai` nightly image at the same commit. ``` Block(dim=8, act='gelu'): passed the positional check (then the stub: 'NoneType' object has no attribute 'mode') Block(8, 'gelu'): TypeError: 'Block.__init__' received a positional argument of type 'str', but no parameter of that type was found in the method signature. Please either annotate 'Block.__init__' or pass it as a keyword argument. Block3(torch.relu, torch.tanh): TypeError: isinstance() argument 2 cannot be a parameterized generic ``` After the fix there were 2 runs on this branch: mine, and one in a separate check of the patch. Both exited with code 0 and printed the same output: ``` Block(dim=8, act='gelu'): passed the positional check (then the stub: 'NoneType' object has no attribute 'mode') Block(8, 'gelu'): passed the positional check (then the stub: 'NoneType' object has no attribute 'mode') Block3(torch.relu, torch.tanh): passed the positional check (then the stub: 'NoneType' object has no attribute 'mode') ``` The `AttributeError` comes from the script's own config stub (`compilation_config=None`), after the positional check has passed. The keyword call stopped at the same point before the fix too. <details> <summary>Reproducer script (from the issue)</summary> ```python import types from collections.abc import Callable import torch import torch.nn as nn import vllm.compilation.decorators as deco from vllm.compilation.decorators import support_torch_compile # Stub the current config; the positional check runs before the config is used. deco.get_current_vllm_config = lambda: types.SimpleNamespace(compilation_config=None) @support_torch_compile(dynamic_arg_dims={"x": 0}) class Block(nn.Module): def __init__(self, dim: int, act: str = "silu"): super().__init__() def forward(self, x: torch.Tensor) -> torch.Tensor: return x @support_torch_compile(dynamic_arg_dims={"x": 0}) class Block3(nn.Module): def __init__(self, act: Callable[[torch.Tensor], torch.Tensor], gate: Callable[[torch.Tensor], torch.Tensor]): super().__init__() def forward(self, x: torch.Tensor) -> torch.Tensor: return x for label, make in [ ("Block(dim=8, act='gelu')", lambda: Block(dim=8, act="gelu")), ("Block(8, 'gelu')", lambda: Block(8, "gelu")), ("Block3(torch.relu, torch.tanh)", lambda: Block3(torch.relu, torch.tanh)), ]: try: make() print(f"{label}: constructed") except TypeError as e: print(f"{label}: TypeError: {e}") except AttributeError as e: # past the positional check; the stub has no compilation settings print(f"{label}: passed the positional check (then the stub: {e})") ``` </details> **New test** `tests/compile/test_decorator.py::test_positional_args_type_check`: ``` python -m pytest --noconftest tests/compile/test_decorator.py -k test_positional_args_type_check ``` - This branch: `1 passed`. - With the original `decorators.py`, an earlier version of the test (the same test without the config parameter) fails: `TypeError: 'M.__init__' received a positional argument of type 'builtin_function_or_method' ...`. - With only the `PreTrainedConfig` skip removed, it fails: `TypeError: 'M.__init__' received a positional argument of type 'Qwen2Config' ...`. - I used `--noconftest` because the root `tests/conftest.py` needs `tblib`, which was not installed in the test venv. - I did not run the other 4 tests in this file. CUDA on the test machine failed with `CUDA-capable device(s) is/are busy or unavailable`. Those tests pass only keyword arguments, so the changed loop does not run for them. **Python 3.10 to 3.14.** vLLM itself ran only on Python 3.12. I cut the final check block out of `decorators.py`, with a stand-in `PreTrainedConfig`, and ran it on CPython 3.10.20, 3.11.15, 3.12.3, 3.13.13 and 3.14.4. The result was the same on every version. The script's calls, `list[int]`, `type[int]`, `dict[str, int]`, and a `Qwen2Config` for a `LlamaConfig` parameter all pass. `"8"` for `dim: int` is refused. **Transformers backend.** These probes ran on CPU on the `meta` device, with `CompilationMode.NONE`. Each probe decorates the decoder class and the encoder classes the way the Transformers backend does, then builds them with `AutoModel.from_config`. This tests the build step only. I did not run a full `vllm serve`. | Build probe | `43b4aaea3` | Without the config skip | This PR | |---|---|---|---| | All 557 model types in Transformers 5.18 `MODEL_MAPPING_NAMES`, default configs, decoder and each `get_encoder(modality=...)` | 502 build, **1 refused** (`t5gemma2`) | not run | 503 build, 0 refused | | vLLM's own config classes: `nemotron`, `olmo_hybrid`, `qwen3_5_text`, `qwen3_5_moe_text`, `muse_glimmer_text` | build | **refused** | build | | Local remote code (2-layer copy of `hmellor/Ilama-3.2-1B`), with the config loaded in the build process, or cloudpickled to it as in a spawned engine | build | **refused** both ways | build | | Hub `internlm/internlm2_5-1_8b-chat`, the same two ways | build | **refused** when cloudpickled | build | The other 54 model types do not build from a default config even without the decorator. In the sweep, `t5gemma2` builds `T5Gemma2Encoder(config.encoder, config.eoi_token_index)`, and the old check compared the `int` with `config: T5Gemma2EncoderConfig`. Encoders are only decorated when `compile_mm_encoder` is set. **pre-commit** on both changed files: passed. ## Details **Root cause.** vllm-project#30518 (`dfe5e3168`) changed the wrapper to `__init__(self, *args, vllm_config=None, prefix="", **kwargs)`, so that Hugging Face classes built as `cls(config)` can be decorated. It also added a check of `args` against the annotations of `old_init`. The check has two faults: 1. `old_init` is the unbound `cls.__init__`, so its signature starts with `self`, but `args` does not include `self`. `zip(args, annotations)` therefore checks each argument against the parameter before it. A debug print showed `8` checked against the empty annotation of `self`, and `'gelu'` checked against `int`. In `cls(config)` the only argument always met `self` and was skipped. So the check never ran on the Transformers backend, which is why nobody noticed the bug. 2. `isinstance(arg, annotation)` raises its own `TypeError` for parameterized generics and for string annotations. **Fix.** Skip `self`. Call `isinstance` only when all three of these hold: - `inspect.isclass(annotation)`. This skips string annotations, unions, and (on Python 3.11+) generics. - `get_origin(annotation) is None`. On Python 3.10, `inspect.isclass` is `True` for `list[int]`, `type[int]` and `collections.abc.Callable[[int], int]`. - The annotation is not a subclass of `PreTrainedConfig`. **Why configs are skipped.** With fault 1 fixed, the check now runs on Transformers-backend configs for the first time. The config that vLLM passes is the right one, but it is often not an instance of the annotated class. There are three cases: - vLLM registers its own config class for some model types (for example `nemotron` and `qwen3_5_text`). That class is not a subclass of the Transformers class with the same name. - For local remote code, Transformers 5.18 loads the config module, and the modeling module's copy of it, into two different hash directories. - For Hub remote code in a spawned engine process, the config is cloudpickled by value and comes back as a new class. In each case the error would point at Transformers or Hub code that the user cannot change. Skipping configs keeps them unchecked, as they always were in practice. **Tradeoffs and limits.** - Unions (`Foo | None`) are now skipped instead of checked. This makes the check a little looser, but it never refuses a correct call. - Some annotations still reach `isinstance` and raise: `typing.Any` (on Python 3.11+), a `Protocol` that is not `runtime_checkable`, and a `TypedDict`. No class decorated with `support_torch_compile` in `vllm/` has a positional parameter annotated like that, so I did not add a case for them. - The message says "no parameter of that type was found in the method signature". This is now a little off, because the check compares by position. I left the message unchanged. - Removing the check altogether would also fix the issue, because it never checked a call correctly. This PR keeps the check so that wrong types for plain class annotations are still caught. - `from transformers import PreTrainedConfig` adds no start-up cost. Importing `vllm.compilation.decorators` already loads `transformers.configuration_utils`. - CI runs the new test, because `.buildkite/test_areas/pytorch.yaml` runs every `compile/test_*.py`. CI has no Python 3.10 job and no Transformers-backend run that covers the config cases above. I checked those outside CI, as shown in Validation. **Relation to vllm-project#60214.** vllm-project#60214 also drops `self`, and it skips annotations that make `isinstance` raise. This PR also skips `PreTrainedConfig` annotations. Without that skip, the corrected check refuses the Transformers-backend builds marked "refused" in the table above. I have not run vllm-project#60214's branch, so I do not know whether it hits these cases. --- <details> <summary> Pull Request Checklist </summary> - [x] I used vLLM's `/pr-checklist` skill. (Mandatory for agents, optional for humans). - [x] AI assistance was used during the creation of this PR. - [ ] **Design Fit:** Minimizes impact on core components, reuses existing functionality, and justifies added complexity. - [ ] **Testing and Validation:** Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI. - [ ] **Code Quality and Style:** Keeps code and comments clear and concise, and updates relevant documentation and examples. - [ ] **Pull Request Contents:** Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide. </details> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Vadim Gimpelson <vadim.gimpelson@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
vLLM doesn't compile the vision encoder for multimodal models, but when models are loaded with the Transformers modeling backend, the entire model is decorated for compilation.
This PR fixes this so that:
-cc.compile-mm-encoder trueis passedThis will make using vision models with vLLM via the Transformers modeling backend behave as they would if they were loaded with the vLLM modeling backend.
Other useful PRs:
set_model_tag@can_return_tuplefrom Transformers