Add a vllm_llm_kwargs passthrough for colocate vLLM generation - #6816
Open
behroozazarkhalili wants to merge 12 commits into
Open
Add a vllm_llm_kwargs passthrough for colocate vLLM generation#6816behroozazarkhalili wants to merge 12 commits into
behroozazarkhalili wants to merge 12 commits into
Conversation
…ructor The colocate path builds `LLM(...)` from a fixed set of explicit kwargs, so any engine argument TRL does not expose a field for is unreachable. Reported in #6776: text-only Gemma 4 checkpoints declare a multimodal architecture, so vLLM takes the multimodal path and fails on a missing preprocessor_config.json. The documented workaround is `hf_overrides`, which the trainer had no way to pass, leaving users to monkeypatch `vllm.LLM.__init__`. `vllm_llm_kwargs` is a dict merged over the explicit kwargs, mirroring how `generation_kwargs` merges over the SamplingParams defaults, with the same rule that conflicting keys override. It is annotated `dict[str, Any] | str | None` and listed in `_VALID_DICT_FIELDS`: both are required for a dict field to be settable from the command line, since argparse resolves the field before `__post_init__` gets a chance to json-load it. The knob lands on `VLLMGeneration`, which is shared by 8 trainers, plus the two paths that build `LLM(...)` themselves: the Online DPO trainer, which never migrated to `VLLMGeneration`, and `vllm_serve.py`, where it takes a JSON string matching the existing `speculative_config` flag. `tensor_parallel_size` and `enable_sleep_mode` are rejected rather than merged. Both are read back after construction: the former builds the TP process group and drives prompt gathering and output slicing, the latter drives the sleep/wake cycle and its bookkeeping. Overriding only the engine side would leave the two out of step, and in the tensor-parallel case that is silent rather than fatal, with each rank submitting its own prompts to a shared engine. The server rejects `tensor_parallel_size` for the same reason, as its weight-sync group size is derived from it.
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
The GOLD trainer now reads `args.vllm_llm_kwargs` when constructing VLLMGeneration, but three `SimpleNamespace` stubs in the test file enumerate the attributes a fake args object carries, so two vLLM init tests raised AttributeError. Two of the three stubs are what the failing tests use. The third lives in `_make_vlm_trainer_args`, whose four callers all take the `use_vllm=False` default and so never reach the vLLM path today; it gets the attribute anyway, since the helper already accepts `use_vllm=True` and the same break returns the moment a caller passes it.
Three conflicts, all the same shape: `_VALID_DICT_FIELDS` in `grpo_config.py`, `rloo_config.py` and `distillation_config.py`, where main added `generation_kwargs` and `chat_template_kwargs` while this branch had added `vllm_llm_kwargs`. Resolved as a union with `vllm_llm_kwargs` last, matching the list's existing convention: it is append-as-added rather than declaration ordered, since `model_init_kwargs` is declared at line 435 while `transformers_continuous_batching_config` is declared at 1038.
The passthrough had no test. Adding the field to a config is not evidence that a user-supplied key survives to `LLM()`, and two independent code-review passes read the config diff and concluded the plumbing was missing, which is the shape of feedback a missing test invites. `TestVLLMGenerationLLMKwargs` drives `VLLMGeneration.__init__` in colocate mode with `is_vllm_available` and `LLM` patched, so it runs without vLLM installed while the production merge and precedence logic execute unchanged. Three behaviors are asserted: a new key (`hf_overrides`) arrives at the constructor, a user value beats the default TRL sets for the same key, and `tensor_parallel_size` or `enable_sleep_mode` raises rather than silently desynchronizing the engine from the trainer. The tests are non-vacuous. With `llm_kwargs.update(self.llm_kwargs)` disabled in `vllm_generation.py`, the first two fail with `KeyError: 'hf_overrides'` and `assert 0.3 == 0.55`; the reserved-key cases keep passing, since they exercise the `__init__` guard rather than the merge.
trl/scripts/vllm_serve.py no longer builds the engine in process. It now shells out to `vllm serve` through build_command, and forwards unrecognized arguments verbatim, so `--hf-overrides` and friends already reach the engine without a JSON flag. The server half of this branch is therefore redundant and is dropped. VLLMGeneration still constructs LLM() directly for colocate mode, and nothing on the command line reaches that call, so vllm_llm_kwargs is kept for the eleven trainer configs that feed it.
Open
…able `vllm_llm_kwargs` refused two keys, `tensor_parallel_size` and `enable_sleep_mode`, and let everything else override TRL's defaults. Five more of those defaults are read back after the engine is built or relied on by the weight sync, so overriding them on the engine side alone breaks training silently: `model` (the sync still pushes the training model's weights), `distributed_executor_backend` (TRL drives the colocated driver worker directly), `seed` (one value per tensor-parallel group), `logprobs_mode` (the importance-sampling correction expects processed log probabilities) and `quantization` (derived from the training model). The reserved keys now live in one table, `RESERVED_LLM_KWARGS`, with the reason next to each key, and a single `_check_llm_kwargs` enforces it. `VLLMGeneration` and OnlineDPO both call it, OnlineDPO before the mode branch so a reserved key is refused in server mode too, where its inline copy never looked. The test is parametrized over the table, so a new reserved key is covered the moment it is added. The nine config docstrings and help strings now use the repository's `or` form for the union, say the field applies only where TRL builds the engine, and name the keys that raise instead of promising that every conflicting key overrides.
…oracles `llm_kwargs` was inserted between `trust_remote_code` and `repetition_penalty`, so a caller that passed the generation parameters by position now had one of them land in the new slot. It is now the last parameter, after `generation_kwargs`, and the docstring entry moves with it. The passthrough tests each checked one value, so a constructor that hardcoded exactly that value passed them; each now runs over two values. The reserved-key test drew its cases from `RESERVED_LLM_KWARGS`, so deleting a reservation deleted its test; the list is now spelled out in the test and a separate test pins the table to it. `VLLMGeneration` writes `RANK`, `LOCAL_RANK` and `WORLD_SIZE` straight into the environment and the fixture never restored them; the fixture registers them with monkeypatch first, and a test checks that a build leaves the environment as it found it.
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.
What this does
Closes #6776.
VLLMGenerationbuilds the vLLM engine itself in colocate mode, and the constructor call attrl/generation/vllm_generation.py:347only receives the arguments TRL happens to expose. Engine settings TRL has no flag for,hf_overridesbeing the case in #6776, cannot be reached at all.This adds a
vllm_llm_kwargsdict that is merged into those constructor kwargs, with keys in the dict overriding TRL's defaults. It is wired through the eleven trainer configs that drive colocate generation.Two keys are rejected rather than forwarded:
TRL reads both back to build the tensor-parallel process group, slice generation outputs, and drive the sleep and wake cycle. Setting only the engine side would leave the two out of step, and the failure would surface far from the cause.
Scope change after merging main
This PR originally covered two paths. The server-side one is gone.
trl/scripts/vllm_serve.pyused to build the engine in process throughllm_worker, and this branch added an--llm_kwargsJSON flag to it. Main has since replaced that withbuild_command, which shells out tovllm serve. Unrecognized arguments are already forwarded verbatim:So
trl vllm-serve --model Qwen/Qwen3-0.6B --hf-overrides '{...}'reaches the engine today with no flag from this PR. Adding a JSON knob alongside that would be a second, worse spelling of the same thing, so the server half is dropped.vllm_serve.pyis no longer touched by this branch.The colocate path has no such route.
LLM()is still called directly there and nothing on the command line reaches it, so the config field is the only way in and that half is kept.Tests
Five tests in
tests/test_vllm_client_server.py, all againstVLLMGenerationwithLLMreplaced by a recorder:test_llm_kwargs_reach_the_llm_constructor, over twohf_overridesvaluestest_llm_kwargs_override_the_defaults_trl_sets, over twogpu_memory_utilizationvaluestest_reserved_table_matches_the_pinned_list, which pinsRESERVED_LLM_KWARGSto the seven keys spelled out in the testtest_reserved_keys_are_rejected, parametrized over those seven keystest_build_leaves_the_distributed_environment_unchanged, since the constructor writesRANK,LOCAL_RANKandWORLD_SIZETwo values per passthrough test because a constructor that hardcodes the one value a single case checks would pass that case. The reserved list is spelled out rather than read from the table because a test derived from the table loses its case when a reservation is deleted instead of failing.
llm_kwargsis the last parameter ofVLLMGeneration.__init__, aftergeneration_kwargs, so callers that pass the older parameters by position are unaffected.Verification
ruff checkandruff format --checkat the CI-pinned 0.13.3, and the pinned doc-builder at--max-len 119, pass on all 21 changed files. The tests above pass, 13 passed with 40 deselected.Not verified here: no live vLLM engine was started, so the tests assert the kwargs reach the constructor rather than that vLLM accepts any particular key. Whether a given override is valid remains vLLM's contract.
Note
Medium Risk
Touches colocate vLLM engine construction across many trainers; incorrect merges could break generation or weight sync, though reserved keys and tests reduce that risk.
Overview
Adds
vllm_llm_kwargsso colocate training can pass extra arguments into vLLM’sLLM()constructor (e.g.hf_overrides) when TRL builds the engine in-process.VLLMGenerationgains anllm_kwargsparameter: TRL’s defaults are built first, then user kwargs are merged on top._check_llm_kwargsandRESERVED_LLM_KWARGSblock seven keys TRL must own (model,tensor_parallel_size,distributed_executor_backend,seed,logprobs_mode,quantization,enable_sleep_mode) so engine and trainer stay in sync.The same config field is wired through trainer configs (GOLD, GRPO, RLOO, distillation, Online DPO, SDFT, SDPO, SSD, IW OPD, etc.) into
VLLMGeneration; Online DPO also validates and merges kwargs on its directLLM(**vllm_kwargs)path.New unit tests mock
LLMto assert passthrough, override behavior, reserved-key rejection, and that distributed env vars are restored after init.Reviewed by Cursor Bugbot for commit a7cc4fc. Bugbot is set up for automated code reviews on this repo. Configure here.