Repository navigation
[hw-agnostic] Implemented plumbing to support OOT plugins - #57715
Closed
bohnstingl wants to merge 9 commits into
Closed
bohnstingl wants to merge 9 commits into
bohnstingl wants to merge 9 commits into
Conversation
…hw-agnostic_p2-monkey-patch Conflict in .buildkite/test_areas/models_basic.yaml: both sides added a test file to the Basic Models (Other) step. Resolved as a union -- main's test_deepseek_v4_vl_rocm.py and this branch's transformers/test_layer_registry.py both run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
`_LayerNameScope.__enter__` rebound every public name a hw-agnostic layer module defines itself, functions included. `activation` defines `get_act_and_mul_fn` on both paths and the two are not interchangeable: the in-tree one takes a `compile_native` keyword and raises `ValueError` for an unsupported activation, the hw-agnostic one takes neither and raises `KeyError`. So for the duration of plugin loading, an in-tree caller passing `compile_native=` got a `TypeError`. Rebinding a function buys nothing to offset that. The mechanism exists so a plugin's `from vllm.model_executor.layers.<mod> import X` yields the class it should subclass and `register_oot` writes into the registry that path reads; a function is neither subclassed nor registered. It was also unprotected -- `validate_registered_overrides` inspects classes, so the swap was wider than the backstop that checks it. Filter to classes in `_own_classes` and share it with `_published_classes`, so the rebinding and the validation cannot come apart again. Also build the override in `test_registration_follows_the_imported_class` with `type()` instead of a `class` statement over a variable base, which mypy rejects as an invalid base class; this matches the idiom `test_plugin_import_pattern_reaches_the_entry_point` already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
…hw-agnostic_p2-monkey-patch Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
bohnstingl
requested review from
Harry-Chen,
hmellor and
khluu
as code owners
September 19, 2026 20:39
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
3 of 4 tasks
…hw-agnostic_oot-rebinding Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
5 tasks
…hw-agnostic_oot-rebinding Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
…hw-agnostic_oot-rebinding Signed-off-by: Thomas Ortner <boh@zurich.ibm.com>
tdoublep
self-requested a review
October 6, 2026 08:53
tdoublep
reviewed
Oct 6, 2026
tdoublep
left a comment
tdoublep
left a comment
Member
There was a problem hiding this comment.
In general I'm concerned that this machinery to rebind stuff creates confusion.
We already have a mechanism for injecting HW agnostic classes via _resolve (here: https://github.com/vllm-project/vllm/blob/main/vllm/model_executor/models/transformers/layers.py#L23-L35).
I think a cleaner approach would be to expose this function publicly so that OOT plugins can call it before inheriting. This way we keep a single mechanism for selecting the classes. I think having OOT plugins opt-in via code change is fine at this point.
6 tasks done
Contributor
Author
|
Closing in favor of #60218 |
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
2 participants
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.
Purpose
This PR is an alternative to #53153, addressing the same problem:
vllm/model_executor/hw_agnostic/custom_op.pycurrently owns its ownop_registry_oot, so an OOT plugin that registers against the public in-tree name lands in a registry the hw-agnostic layers never read.CustomOp,PluggableLayerand both registries from the in-tree module, leaving a single shared registry.hw_agnostic_layer_names()(new,vllm/model_executor/hw_agnostic/layers/_layer_names.py) rebinds each mirrored in-tree layer name to its hw-agnostic counterpart for the duration of_run_general_plugins(). Through this,from vllm.model_executor.layers.layernorm import RMSNormresolves to the hw-agnostic class andregister_ootwrites the hw-agnostic registry.The future of this feature depends a bit on the decision whether the existing CustomOp infrastructure will be solely used for the hardware-agnostic path or not, see #44219 (comment) and following. If the in-tree
custom_op.pyand its layers have to be removed, they can simply be moved over to the hardware-agnostic path with this lightweight rebinding infrastructure removed. As with #53153, existing OOT plugins can remain unmodified. If not, this infrastructure can simply stay.Test Plan
Test Result
31 passed, 14 warnings in 196.28s (0:03:16)
cc @hmellor @tdoublep