Repository navigation
[Bugfix] Give LLM stages typed owners for vLLM LoRA engine args - #8574
MinhaoLi0318 wants to merge 4 commits into
Conversation
Since vllm-project#5140 made the typed VllmOmniConfig resolver the runtime path (first released in v0.29.0rc1), vLLM's LoRA engine args (enable_lora, max_lora_rank, max_loras, max_cpu_loras, lora_target_modules, ...) have no structured config owner on AR/generation stages. Every input path (top-level kwarg, stage_overrides, stage_N_* keys, deploy YAML stage keys, serve CLI) fails with "no structured config owner: enable_lora", so vLLM LoRA cannot be enabled on any LLM stage and a lora_request fails with "LoRA is not enabled!". The legacy path in v0.28.0 passed these through. Add OmniStageLoRAConfig, which inherits vllm.config.LoRAConfig, and an optional BaseVllmOmniStageConfig.lora_config. The owned fields are enable_lora plus a field map generated from LoRAConfig with target_modules aliased to lora_target_modules, following the existing load/cache/scheduler/parallel owners. As in vLLM, the stage carries a LoRA config only when enable_lora is set; other LoRA options are ignored otherwise. The construction validator keeps only the max_cpu_loras >= max_loras check and leaves platform checks to the engine process. _project_omni_stage_engine_args projects enable_lora and the explicitly set fields back to EngineArgs. Global vs stage-scoped: - On a single-stage pipeline, top-level LoRA args apply to that stage. - On a multi-stage pipeline, top-level vLLM LoRA args are rejected with a message pointing to stage-scoped keys, because broadcasting would enable LoRA on talker/codec stages that do not support it. - On a pipeline with a diffusion stage, a global max_cpu_loras keeps its existing diffusion meaning and is not applied to LLM stages; an LLM stage sets its value with a stage-scoped key. Diffusion stages are unchanged: they still reject enable_lora and keep their own LoRA fields on diffusion_config. Tests that used enable_lora as the example of an unowned field now use cpu_offload_gb. Docs cover the new stage option and the owner mapping. Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
|
This PR appears to belong to: docs/design/module/stage_runtime.md, docs/design/module/vllm_omni_config.md. Module owners: @lishunyang12 @alex-jw-brooks @tzhouam Routing: @lishunyang12 via module named in the PR description, semantic router, CODEOWNERS; @alex-jw-brooks via module named in the PR description, semantic router; @tzhouam via module of the changed files, CODEOWNERS @MinhaoLi0318, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot routing recordAssigned Strict on zcode (GLM-5.3-Flash) under experiment |
A lora_request sent to a stage-0 engine started without LoRA fails in
vLLM's input processor ("LoRA is not enabled!"). That happens inside
generate's try block, so it also closed the engine. Check stage 0's
lora_config before the try block and raise a ValueError that shows the
stage-scoped way to enable it.
Update the LLM/AR stage section of the LoRA guide to enable LoRA with
stage_overrides, which vllm-project#8574 makes possible.
Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
|
Verified on a GPU together with #8196 (vLLM 0.31.0, 1x A100 80GB): |
|
Self-review (author). What I checked before asking for review:
@lishunyang12 @tzhouam, as config owners, could you take a look when you have time? The main open question is the one from #7352: does a typed AI assistance: drafted with Claude Code; I reviewed the diff and re-ran the checks above before posting. |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: c806bc69-904d-435e-a5d6-5e6e69619dfb) — check |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: 6fa3de45-a7dd-4c72-8e08-c53acffe447f) — check |
Omni ReviewBot attempt recordReview attempt ended as failed (step 'review' (agent.review_diff): unhandled error: RuntimeError: zcode exited 1 without a result event: statusCode: undefined } Error: Turn execution failed (traceId: 729536bc-fd89-42ac-9898-126f6b74f458) — check |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
0 actionable finding(s).
CI at
1e1a77be86f8(2026-10-10T06:41:21.431901+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Note: The assigned review arm
strict/zcode/GLM-5.3-Flashcould not complete this review, so it was produced by the fallback armdirect/cursor/auto. It is excluded from the routing experiment.
Full review analysis
PR description
LLM stages can take vLLM LoRA engine arguments again. OmniStageLoRAConfig owns enable_lora and the LoRAConfig fields that also exist on EngineArgs (target_modules aliased to lora_target_modules). A stage stores that config only when enable_lora is set, and projection writes enable_lora=True plus the explicitly set fields back into that stage's engine args. On a multi-stage pipeline, top-level vLLM LoRA arguments are rejected so they are not broadcast onto talker or codec stages; a global max_cpu_loras next to a diffusion stage still sizes only the diffusion cache. Diffusion stages still reject enable_lora and keep their own LoRA fields.
Change flow
flowchart LR
A["[EXISTING] CLI, stage overrides, deploy YAML"]:::existing
B["[CHANGED] Stage ownership filter"]:::changed
C["[NEW] OmniStageLoRAConfig"]:::new
D["[CHANGED] EngineArgs projection"]:::changed
E["[EXISTING] Diffusion LoRA config"]:::existing
A --> B
B --> C
C --> D
B --> E
classDef existing fill:#e5e7eb,stroke:#6b7280,color:#111827
classDef changed fill:#fef3c7,stroke:#d97706,color:#451a03,stroke-width:2px
classDef new fill:#dcfce7,stroke:#16a34a,color:#052e16,stroke-width:2px
classDef removed fill:#fee2e2,stroke:#dc2626,color:#450a0a,stroke-width:2px
No actionable findings.
🤖 This review was generated by InferMatrix Copilot, an open-source repo-maintenance agent for PR review, CI debugging and issue triage. Try it on your own repo, and ⭐ star it if it helped!
|
@lishunyang12 Could you review this one when you have time? It fixes LoRA config on LLM stages: on main,
Thanks! AI assistance: drafted with Claude Code; I reviewed it before posting. |
Resolve the conflict in vllm_omni/config/omni_config.py by keeping this branch's _LORA_ENGINE_FIELDS and taking main's three-field _DIRECT_VLLM_CONFIG_ENGINE_FIELDS (speculative_config from vllm-project#7929). Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
hsliuustc0106
left a comment
There was a problem hiding this comment.
@Acerak01-fy PTAL
| @@ -47,10 +47,12 @@ schema source for AR and generation stages: | |||
| - `OmniStageCacheConfig` inherits `vllm.config.CacheConfig`. | |||
There was a problem hiding this comment.
can you add or update the lora feature design docs?
|
LGTM. |
|
Regarding the revision of the doc, it is recommended to update the supported range and boundaries of LORA. |
Describe the supported scope and boundaries of the vLLM LoRA path on LLM stages and the diffusion LoRA path, how vLLM LoRA options reach each LLM stage through OmniStageLoRAConfig, where global and stage-scoped options are accepted, which stage a request-level LoRARequest is applied to, and what is not supported. Link it from the config module page and the stage config guide. Signed-off-by: MinhaoLi0318 <lminhao039@gmail.com>
|
@hsliuustc0106 @Acerak01-fy Added
It is linked from AI assistance: drafted with Claude Code; I reviewed the doc change and this reply. |
Purpose
vLLM LoRA cannot be enabled on any AR/generation stage on main. This is a regression since #5140 (first released in v0.29.0rc1), which made the typed
VllmOmniConfigresolver the runtime path.What broke
Every input path for vLLM's LoRA engine args (
enable_lora,max_lora_rank,max_loras,max_cpu_loras,lora_target_modules,fully_sharded_loras,default_mm_loras, ...) is rejected during config resolution: the top-levelOmni/AsyncOmnikwarg,stage_overrides,stage_N_*keys, deploy YAML stage keys /engine_extras, and the serve CLI.Because a stage cannot enable LoRA, a
lora_requestpassed toAsyncOmni.generate(forwarded since #5374) fails in vLLM withLoRA is not enabled!. On v0.28.0 the legacy stage-config path passed these keys through, so existing deploy YAMLs that setenable_lora/max_lora_rankon stage 0 (for example in downstream projects such as UniRL, which pins v0.28.0) are rejected on main.Repro (CPU, no model weights)
ValueErrorabove.OmniStageLoRAConfig(max_lora_rank=64, max_loras=1, ...).Root cause
These fields live in
vllm.config.LoRAConfig, notModelConfig, and no structured stage config owns them, so_validate_stage_engine_override_ownershipand_validate_global_stage_cli_ownershipreject them. #7390 gave typed owners to other CLI flags and leftenable_lora/max_lorasfor a follow-up because they need their own owner. The question of how to handle them is still open in #7352. This PR is that follow-up. It is also needed for #8196 (lora_requestonOmni.generate).Changes
OmniStageLoRAConfiginheritsvllm.config.LoRAConfig, like the existing load/cache/scheduler/parallel stage configs.BaseVllmOmniStageConfiggets an optionallora_config.enable_loraplus a field map generated fromLoRAConfigwith_upstream_engine_field_map, aliasingtarget_modulestolora_target_modules. They are added to_LLM_STAGE_ENGINE_FIELDSonly.lora_configonly whenenable_lorais set; other LoRA options are ignored otherwise. The construction validator only checksmax_cpu_loras >= max_loras(non-positive treated as unset); platform-dependent checks stay in the engine process. Like the other inherited stage configs, it stays msgpack transport-safe (covered by the existing transport test)._project_omni_stage_engine_args(vllm_omni/engine/stage_init_utils.py) projectsenable_lora=Trueand the explicitly set LoRA fields back toEngineArgs.--stage-overrides. Broadcasting would also enable LoRA on talker/codec stages that do not support it, while request LoRA only reaches stage 0.max_cpu_loraskeeps its existing diffusion meaning and is not applied to LLM stages. An LLM stage sets its value with a stage-scoped key.enable_loraand keeplora_path/lora_scale/lora_backend/max_cpu_lorasondiffusion_config.docs/configuration/stage_configs.md, the owner/alias notes indocs/design/module/vllm_omni_config.md, and a LoRA design page,docs/design/feature/lora.md(supported scope and boundaries, per-stageLoRAConfig, global vs. stage-scoped options, which stage a request adapter reaches, what is not supported), linked from the docs nav and both pages.vllm_omni/config/__init__.pyexportsOmniStageLoRAConfig; the SPDX header was added by thecheck-spdx-headerpre-commit hook because the file had none.Usage
Out of scope
AsyncOmni.add_lora/remove_lorastill send the RPC to every stage. With LoRA enabled on one stage only, targeting those calls at specific stages is a separate change.Notes for merging
_ModelEngineOverrides/ stage config region inomni_config.py; expect a small textual conflict.test_stage_cli_field_selection_defers_ownership_validation_until_sources_are_mergedintests/config/test_omni_config.pyconflicts textually with [Core] Remove legacy stage config runtime paths #8077. When resolving, keep thecpu_offload_gbexample;enable_lorais now owned and no longer an example of an unowned field. The new engine test only uses_project_omni_stage_engine_args, which [Core] Remove legacy stage config runtime paths #8077 keeps.AI assistance: I used Claude Code to help investigate the regression, draft the code, tests and docs, review the diff, and draft this description. I reviewed every change and ran the checks below myself.
Test Plan
vLLM Version: 0.31.0 (source on
PYTHONPATH, CPU,VLLM_TARGET_DEVICE=cpu), torch 2.13 CPU, macOS. No GPU was available.vLLM-Omni Commit: based on c2cd13c
New tests:
tests/config/test_omni_config.py: stage-scoped LoRA from stage CLI keys, deployengine_extrasand deploy YAML onqwen3_ttsandbagel; global args on a single-stage pipeline; global args rejected on multi-stage pipelines; globalmax_cpu_lorason a mixed AR + DiT pipeline; options ignored withoutenable_lora; vLLM LoRA rejected on a diffusion stage; the validator does not resolve deferred fields. Existing export / public-field / keyword-only / msgpack tests now includeOmniStageLoRAConfig.tests/engine/test_stage_engine_args.py:test_stage_lora_reaches_projected_engine_args_only_on_owning_stage, plus projection test updates.Commands:
I also ran a local CPU script that goes through
prepare_stage_config_inputs->load_and_resolve_stage_configs->_project_omni_stage_engine_args-> vLLM 0.31.0create_engine_config, with a tiny Qwen2 config standing in for the model.Test Result
test_injected_aligner_survives_typed_config_and_runtime_projections,test_frontend_log_stats_flag_is_not_an_unowned_stage_argument[True]/[False],test_mps_stays_in_runtime_instead_of_engine_arguments.mypy-3.10hook and the local hooks listed above pass (mypy: 0 errors on both this branch and c2cd13c).create_engine_config:qwen2_5_omniwithstage_overrides={"0": {"enable_lora": True, "max_lora_rank": 64, "max_loras": 2}}: only stage 0 getsLoRAConfig(max_lora_rank=64, max_loras=2, max_cpu_loras=2); stages 1 and 2 have no LoRA config.bagelwith stage 0enable_lora: AR stage 0 gets aLoRAConfig; DiT stage 1 has noenable_loraand keepsmax_cpu_loras=1.qwen2_5_omni_thinker_onlywith a globalenable_loraworks.enable_lora/max_lora_rankonqwen2_5_omniandbagelraise the new error; on single-stage diffusionwan2_2_ti2vit is still rejected.max_lora_rank: 32,lora_target_modules: [o_proj]) work; a deployenable_lora: trueoverridden tofalsebystage_overridesgives no LoRA config.Nothing was run on GPU. I will do the end-to-end GPU check (serving a LoRA adapter on stage 0 and sending a
lora_request) together with #8196.BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
Note
To avoid agent spam, vLLM-Omni limits contributors without write access to 5 open PRs. If this blocks well-intentioned critical work, please contact a committer to request bypass list access.
(anything written below this line will be removed by GitHub Actions)