Repository navigation
[Core] Add AR and diffusion backend config propagation - #8092
Conversation
|
This PR appears to belong to: docs/design/module/stage_runtime.md, docs/design/module/diffusion/index.md. Module owners: @david6666666 @Isotr0py @princepride Routing: @david6666666 via module of the changed files, module named in the PR description, CODEOWNERS; @Isotr0py via module of the changed files, module named in the PR description, CODEOWNERS; @princepride via module of the changed files, module named in the PR description @shunwendongan, 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 triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
Signed-off-by: wu kui <wukui1030660585@163.com>
Signed-off-by: wu kui <wukui1030660585@163.com>
Signed-off-by: wu kui <wukui1030660585@163.com>
750a296 to
84158a8
Compare
|
Self-review: I rechecked the rebased diff against the latest main and verified that the change is limited to Item 1 configuration/projection. I reviewed the typed and legacy AR/diffusion paths, upstream KernelConfig normalization, first-class versus engine_extras precedence, platform overlay compatibility, and the existing Higgs deploy assertions. The focused regression suite passes (819 passed, 2 existing HF-config skips), and the opt-in RTX 3080 tiny FLUX smoke passes with 15 real linear layers and 2 SDPA attention layers. No registration, selector, FishSpeech, model-default, metadata/cache/CUDA-graph, or fallback redesign is included. The local mypy warning is pre-existing and matches the rebased baseline; no suppressions were added. |
|
One more [P3], not anchored to a diff line: Reviewed at 84158a8. No correctness blockers found: the KernelConfig reuse (defaults, normalization, validation) matches upstream v0.29.0, both typed and legacy paths were traced to the terminal config, the removed worker-side MoE override has no remaining dependents, |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
This change promotes AR attention_backend, moe_backend, and linear_backend to first-class deploy fields and carries the shared MoE/linear choices through typed AR and diffusion configs. Those values are normalized with upstream KernelConfig and, on the diffusion path, written into the shared VllmConfig builder before layers are constructed. Explicit first-class fields beat the same engine_extras keys after a warning, while omitted or extras-only inputs and existing Qwen3-Omni / Higgs deploy defaults keep working.
Change flow
flowchart TD
A["[EXISTING] Deploy YAML, extras, platform overlay"]:::existing
B["[NEW] StageDeployConfig attention/moe/linear fields"]:::new
C["[CHANGED] Typed Omni configs plus KernelConfig normalize"]:::changed
D["[CHANGED] Legacy and typed engine-args projection"]:::changed
E["[CHANGED] create_base_diffusion_vllm_config kernel wiring"]:::changed
F["[EXISTING] VllmConfig.kernel_config before layer build"]:::existing
A --> B --> C --> D --> E --> F
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.
|
@shunwendongan gentle ping — this high-priority PR has seen no updates for 4 days and review feedback is waiting. Could you share a status or ETA when you get a chance? Thanks! |
|
Thanks for the detailed review. I addressed all three P3 items.
The focused source checks pass locally. The full pytest command still needs to |
Signed-off-by: shunwendongan <MacBook@MacBook-Air-6.local>
|
Addressed all three review items in commit
Validation on Apple M1 (PyTorch 2.12.1, MPS available, CUDA unavailable):
AI assistance: Codex helped draft the implementation, tests, documentation, and this summary. I reviewed the diff and validation results. |
hsliuustc0106
left a comment
There was a problem hiding this comment.
Both precedence P3s are fixed at this head — _normalize_stage_backend_fields in StageDeployConfig.__post_init__ now normalizes promoted backends and strips the duplicate legacy keys from engine_extras after nested-extras merging, which closes the platform-overlay re-entry path generally (and covers the top-level hoist case by the same mechanism). Explicit-diffusion provenance via _TrackExplicitConfigFields resolves the model-vs-diffusion silent win. Branch conflicts with main — rebase owed before merge. Thanks for the fast turnaround on the ping.
…onfig Signed-off-by: shunwendongan <MacBook@MacBook-Air-6.local> # Conflicts: # vllm_omni/engine/stage_init_utils.py
Signed-off-by: shunwendongan <MacBook@MacBook-Air-6.local>
|
@hsliuustc0106 Synced the branch with Local checks on the updated PR diff passed: Ruff check/format, compileall, The full pytest suite was not rerun locally: my M1 environment lacks vLLM and has no CUDA/NPU device. The sync includes upstream NPU/XPU vLLM 0.31 alignment (#8588/#8528) and the AMD AITER cache fix (#8527); fresh CI results are needed to confirm their effect on the earlier failures. AI assistance: Codex assisted with the upstream sync, local checks, and drafting this reply. |
|
@hsliuustc0106 The new head |
|
Current-head validation and review follow-up for
The current head is mergeable and retains approval. The listed GitHub checks are successful; Buildkite results are still absent on this head. The Oct 9 maintainer-trigger request remains pending. AI assistance: Codex performed this verification and drafted the update; the test counts above are actual command results. |
Omni ReviewBot attempt recordReview attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 120s (try 2 of 3)). |
Omni ReviewBot attempt recordReview attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 600s (try 3 of 3)). |
Omni ReviewBot attempt recordReview attempt ended as failed (failed; falling back to direct/cursor/auto). |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
Changes since the previous review
- 0 new inline finding(s); 0 finding(s) below.
CI at
d6d5ab233b13(2026-10-10T16:40:01.268490+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Note: The assigned review arm
strict/cursor/cursor-grok-4.6-highcould 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
Stage deploy config now accepts attention_backend, moe_backend, and linear_backend as real stage fields, and both the legacy engine-args builder and the typed stage projection carry MoE and linear choices through to the final config. Upstream KernelConfig normalizes and validates those two values, while a shared precedence step drops the same keys from engine_extras when a first-class value is set, including after a platform overlay merges nested extras. Diffusion no longer patches MoE onto kernel_config after worker setup; the shared VllmConfig builder receives both kernel choices before layers are built, and an explicit typed model value still beats a different explicit diffusion value with a warning.
Change flow
flowchart LR
yaml["[EXISTING] Stage deploy settings"]:::existing
deploy["[CHANGED] StageDeployConfig backend fields"]:::changed
norm["[NEW] First-class extras precedence"]:::new
builders["[CHANGED] Legacy and typed projection"]:::changed
kernel["[CHANGED] Diffusion VllmConfig kernel_config"]:::changed
worker["[CHANGED] Worker drops late MoE write"]:::changed
yaml --> deploy --> norm --> builders --> kernel --> worker
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
🤖 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!
|
Suggestions / Follow-ups
|
|
please fix conflicts |
…tions Signed-off-by: shunwendongan <MacBook@MacBook-Air-6.local>
Signed-off-by: shunwendongan <MacBook@MacBook-Air-6.local>
…ence Signed-off-by: shunwendongan <MacBook@MacBook-Air-6.local>
|
@Bounty-hunter @hsliuustc0106 Synced with main Addressed the configuration follow-ups:
Local M1 verification: 12 isolated warning regressions passed using official vLLM 0.31.0 The AR MoE startup/prefill/decode and diffusion startup/request/real MoE-forward tests remain follow-up acceptance work, as suggested. New GitHub Actions have been queued and DCO passed. AI assistance: Codex implemented the updates, ran the local verification and drafted this reply. |
Purpose
Partial implementation of #7924 Item 1. Refs #7924.
Complete the backend configuration/propagation slice discussed in the issue, without changing dispatch or registration APIs.
linear_backendthrough the AR and diffusion structured configs.KernelConfignormalization/validation for MoE and linear choices. Preserve omitted/default/explicitautobehavior and existing Qwen3-Omni defaults.VllmConfigbuilder used by engine and workers, before layers are constructed. Do not let shared model defaults overwrite explicit diffusion choices.engine_extraskeys, with a conflict warning. Preserve extras-only inputs and platform overlays, including ROCm attention overrides of legacy extras.Previously, typed AR configuration rejected
linear_backendas having no structured owner, and diffusion lacked the field and final kernel-config wiring. The regression tests follow both typed and legacy paths to the terminal config, not just the input dictionary.Out of scope: facade/registration/discovery, CUSTOM/name-registry changes, declarative model defaults, FishSpeech, selector/per-role behavior, metadata/cache/CUDA-graph lifecycle, and broader validation or fallback redesign. Diffusion retains its existing attention selector and platform hooks; AR retains upstream attention handling.
Precedence contract
engine_extraskey, including nested platform extras. Different values produce a warning; extras-only inputs remain supported.model_config.moe_backend/linear_backendwins over an explicit diffusion value, warning on different normalized values. Omitted model defaults do not overwrite explicit diffusion selections; explicitautois retained.Current-head update — 2026-10-14
Head:
cd5db54695e69cdf04cd7206d05373bdfd4793e7. Includes upstream main37f094a388ba1c79f83c129fda39797c1ebfc2a4.KernelConfignormalization before emitting a conflict warning. Equivalent case/hyphen spellings no longer warn; conflicting or invalid shadowed extras still yield to the first-class field.Local verification on Apple M1:
KernelConfigsource with a Pydantic dataclass wrapper; it does not import the full vLLM package.git diff --check, SPDX, test marks, forbidden imports and thetorch.cudagate: passed across the PR's Python diff.The AR MoE startup/prefill/decode and diffusion startup/request/real MoE-forward acceptance tests remain follow-ups, as suggested in the review. Previous-head pytest/GPU results below are historical evidence.
AI assistance: Codex implemented the sync and follow-ups, ran the local checks, and drafted this update.
Previous-head validation — 2026-10-10
Validated commit:
d6d5ab233b13aa798a38b006211211c3ac4f5c5a, after merging main874b8b3a.Environment: Ubuntu/WSL2, Python 3.12.3, vLLM 0.31.0 (matching the updated CI Dockerfile), PyTorch 2.13.0+cu130, transformers 5.14.1, diffusers 0.40.0, pydantic 2.13.5. CUDA smoke ran on one RTX 3080 10 GB, compute capability 8.6; Windows NVIDIA driver 616.56.
The official vLLM 0.31.0 package was installed in a separate target directory and used via
PYTHONPATH; these are real pytest runs, not a stub harness. The commands below used the existing Python environment withPYTHONPATH=/home/shunwendongan/.venvs/vllm-omni-pr8092-031.Commands and results
The two regression skips are existing Qwen3-Omni cases requiring an HF config. The runs emit upstream deprecation warnings and the checkout's missing generated
_version.pywarning.The CUDA component smoke executes 15 real vLLM linear layers, observes
torchreaching the real GEMM selector, checksdefault_unquantized_gemmand twoSDPAImplattention layers, and checks finite output of shape(1, 4, 64). It uses a tiny native FLUX transformer with random weights and synthetic inputs; no checkpoint download or numerical/dispatch implementation replacement.At
d6d5ab23on 2026-10-10, both precedence review threads were resolved and the PR retained approval. GitHub reportedMERGEABLE; Python 3.11/3.12 wheel builds, pre-commit, DCO and Read the Docs passed on that head. Buildkite results were absent. These results apply to that previous head.These checks validate configuration propagation and a CUDA component execution path, not full AR generation, image generation/quality, performance, or NPU/ROCm runtime support. AI assistance: Codex assisted implementation, review, test execution, and this PR update.
Historical validation on vLLM 0.29.0 (not current-head evidence)
These results predate the latest precedence fixes and main sync. They are retained as historical evidence; current-head results are listed above.
Test Plan
vLLM Version: 0.29.0
vLLM-Omni base:
67aa30c9(rebased before validation)Environment: Ubuntu/WSL2, Python 3.12.3, PyTorch 2.13.0+cu130 (CUDA 13.0), transformers 5.14.1, diffusers 0.40.0, pydantic 2.13.5. GPU: one RTX 3080 10 GB, SM86.
Configuration/engine regression, existing defaults, platform overlays, and affected Higgs deploy coverage:
The new config tests use
core_model+cpuand are collected by the existing L1 sweep. Quick focused command:pytest -q tests/config/test_backend_selection.py -m 'core_model and cpu'Opt-in, offline CUDA component smoke (no checkpoint download):
pytest -s -q tests/diffusion/models/flux/test_backend_selection.py -m 'local_model and cuda'Test Result
default_unquantized_gemm, and both attention layers selectSDPAImpl; output(1, 4, 64)is finite. A spy observestorchreaching the real GEMM selector without replacing its behavior, sincetorchandautomay resolve identically on Ampere.mypy-3.10. On Windows, both the unmodified base and this branch produce the same 88 diagnostics in the affected existing files (compared after removing line numbers); no suppressions or allowlist changes were added.git diff --checkpasses. Full diff-scopedprecheck-prreview completed with this baseline warning.This is not an AR generation, full image-generation, image-quality, or performance benchmark. No new backend or faster-kernel claim is made. NPU/ROCm runtime behavior is not hardware-validated here; upstream linear filtering/fallback remains in effect (including unquantized ROCm dispatch not consulting this flag in vLLM 0.29).
AI assistance: Codex assisted implementation, test execution, code review, and documentation/PR drafting. The results above are actual command outputs, with limitations called out explicitly.