Skip to content

fix: prevent vllm_omni from breaking Qwen3 vLLM inference - #79

Merged
timzsu merged 3 commits into
mainfrom
zsu/fix-inference-bug
Jun 21, 2026
Merged

timzsu merged 3 commits into
mainfrom
zsu/fix-inference-bug

Conversation

@timzsu

@timzsu timzsu commented Jun 19, 2026 •

Copy link
Copy Markdown
Collaborator

Purpose

When vLLM Omni 0.22.0 is installed, vLLM 0.22.0 fails to initialize an engine for models using Model Runner V2 (e.g., Qwen3-8B). This PR implements a workaround. Note that I have also fixed the root cause upstream (vllm-project/vllm-omni#4568), so we can revert the patch after we bump the versions next time.

Changes

  • src/worker/executors/__init__.py: Import vLLM Omni lazily so the plugin is not registered when we are not using the Omni executor.
  • src/worker/executors/vllm_executor.py: Removes the Omni plugin from vLLM if we are using VLLMExecutor.
  • examples/templates/inference_vllm_qwen3.yaml: A sample workflow to surface the problem.
  • The other three files are adjustments to fix lint and new unit tests.

Test Plan

End-to-end execution of examples/templates/inference_vllm_qwen3.yaml.

Test Result

The workflow completes successfully.


Pre-submission Checklist
  • I have read the contribution guidelines.
  • I have run pre-commit run --all-files and fixed any issues.
  • I have added or updated tests covering my changes (if applicable).
  • I have verified that uv run pytest tests/ passes locally.
  • If I changed shared schemas or proto definitions, I have checked downstream compatibility across Server and Worker.
  • If I changed the SDK or CLI, I have verified the affected packages work (uv sync --all-packages --group ci --frozen).
  • If this is a breaking change, I have prefixed the PR title with [BREAKING] and described migration steps above.
  • I have updated documentation or config examples if user-facing behavior changed.

@timzsu
timzsu marked this pull request as ready for review June 19, 2026 11:31
@timzsu
timzsu requested a review from kaiitunnz as a code owner June 19, 2026 11:31

@kaiitunnz kaiitunnz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments.

Comment thread src/worker/executors/vllm_executor.py Outdated
Comment thread src/worker/executors/__init__.py Outdated
Comment thread tests/worker/test_vllm_scoped_plugins.py Outdated
Comment thread tests/worker/test_vllm_scoped_plugins.py Outdated
Comment thread tests/worker/test_vllm_scoped_plugins.py Outdated
Comment thread examples/templates/inference_vllm_qwen3.yaml Outdated
Comment thread examples/templates/inference_vllm_qwen3.yaml Outdated
Comment thread examples/templates/inference_vllm_qwen3.yaml Outdated
@timzsu
timzsu requested a review from kaiitunnz June 20, 2026 10:06
timzsu added 3 commits June 20, 2026 19:36
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
Signed-off-by: Zhengyuan Su <su.zhengyuan@u.nus.edu>
@timzsu
timzsu force-pushed the zsu/fix-inference-bug branch from f7e9694 to c321535 Compare June 20, 2026 11:36

@kaiitunnz kaiitunnz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@timzsu
timzsu merged commit 302f675 into main Jun 21, 2026
11 of 13 checks passed
@timzsu
timzsu deleted the zsu/fix-inference-bug branch June 21, 2026 01:51
@kaiitunnz kaiitunnz mentioned this pull request Jun 21, 2026
4 of 8 tasks
@timzsu timzsu mentioned this pull request Jul 10, 2026
4 of 8 tasks
kaiitunnz added a commit that referenced this pull request Jul 10, 2026
#79 avoided importing vllm_omni at module load (find_spec instead of import,
lazy in-method imports, and a VLLM_PLUGINS allowlist that excluded omni from
plain vLLM init) because importing it rebinds vllm.v1.request.Request to
OmniRequest and broke plain-model warmup. vllm-omni 0.24 makes OmniRequest a
positional drop-in for Request (its extra params moved to keyword-only, and it
forwards super().__init__(*args, **kwargs)), so the rebind is now benign and
the whole workaround is obsolete. Restore eager top-level imports guarded by a
real try/except (keeping the = object fallback so annotations stay import-safe
when vllm-omni is absent) and remove the VLLM_PLUGINS exclusion.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>
kaiitunnz added a commit that referenced this pull request Jul 10, 2026
* chore: bump vllm to 0.24.0

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* chore: use vllm release package

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* chore: bump vllm-omni to 0.24.0

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* chore: restore vllm cuda 12.9 wheel

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* fix(worker): adapt omni executors to vllm-omni 0.24 output API

Replace the getattr/dict-probing output extraction with direct access to
vllm-omni 0.24's typed OmniRequestOutput (multimodal_output property, request_output,
images), accept the Qwen3-TTS model_outputs payload key, and build the Qwen3-TTS
talker prompt (placeholder token ids sized to the model estimate, cache_salt,
tts_local_seed) the way vllm-omni's own serving path does. Type _omni as Omni with
object-valued TYPE_CHECKING fallbacks so annotations resolve without importing
vllm-omni at module load.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* test(worker): cover omni multimodal output extraction

Exercise extract_multimodal_output / extract_audio_from_mm and the text2audio /
text2general audio extractors against fakes that mirror the vllm-omni 0.24
OmniRequestOutput contract, including the Qwen3-TTS model_outputs payload and
non-dict Mapping payloads.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* chore(examples): trim omni templates to lightweight smoke tests

Shrink the omni example workflows into quick vllm-omni bring-up smokes: lower
GPU memory / inference steps / audio length, shorten prompts, and drop the
inline single-GPU stage_configs from the text2general template so it uses the
model's built-in pipeline.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* refactor(worker): extract Qwen3-TTS handling into an omni package

Move the Qwen3-TTS talker-prompt and sampling-param construction out of the TTS
executor into worker.executors.omni.qwen3_tts, behind a centralized is_qwen3_tts()
check, so other TTS families can gain their own adapter modules later. The
tts_local_seed sampling override is now applied only for Qwen3-TTS; non-Qwen3
models fall back to raw text and default sampling params, with a TODO pointing at
the per-family builders vllm-omni's serving path still needs ported.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* fix(worker): forward async_chunk to the Omni constructor

build_omni_init_kwargs now passes a cfg-level async_chunk through to Omni(...),
and the reuse spec includes it so flipping the flag rebuilds the instance. The
TTS executor's async_chunk=False for Qwen3-TTS was previously set on the config
but never forwarded, so it was silently dropped; the Omni constructor accepts the
kwarg. This lives in the base, so any omni executor's cfg async_chunk is honored.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* test(worker): sort imports in omni output extraction tests

Order the omni_executor_base import group so OmniRequestOutput precedes the
lowercase helpers, matching isort.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* chore(examples): restore omni templates to canonical form

Revert the e2e smoke trims (f077539); the shipped examples stay full-fidelity
references. The trimmed, GPU-capped smoke variants used for the dependency-bump
run live with the e2e harness instead of in the published examples.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* refactor(worker): give OmniTextPrompt a runtime = object fallback

The TYPE_CHECKING-only import is safe today because its sole use is a local
variable annotation (never evaluated), but a bare deferred import is a trap: any
future use in a signature, cast, or isinstance would NameError. Mirror the
= object fallback the other omni modules use for deferred vllm-omni names.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* fix(worker): keep omni_text2audio importable without numpy

Quote the module-level np.ndarray signatures so they are not evaluated at
import; the module's try/except sets np=None when numpy is absent, and an
evaluated np.ndarray annotation would AttributeError, defeating that guard.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* refactor(worker): drop unreachable Mapping branch in _extract_request_audio

req is always an OmniRequestOutput under the 0.24 typed API, never a Mapping,
so the dict-fallback branch was dead.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* test(worker): cover qwen3-tts prompt/config helpers

Unit-test the reusable pure helpers (is_qwen3_tts, task-type resolution,
max-new-tokens precedence/floor, cache-salt determinism).

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* refactor(worker): drop the vllm_omni import-isolation workaround

#79 avoided importing vllm_omni at module load (find_spec instead of import,
lazy in-method imports, and a VLLM_PLUGINS allowlist that excluded omni from
plain vLLM init) because importing it rebinds vllm.v1.request.Request to
OmniRequest and broke plain-model warmup. vllm-omni 0.24 makes OmniRequest a
positional drop-in for Request (its extra params moved to keyword-only, and it
forwards super().__init__(*args, **kwargs)), so the rebind is now benign and
the whole workaround is obsolete. Restore eager top-level imports guarded by a
real try/except (keeping the = object fallback so annotations stay import-safe
when vllm-omni is absent) and remove the VLLM_PLUGINS exclusion.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

* refactor(worker): reuse shared multimodal extraction helpers in omni_text2general

_extract_request_audio reimplemented a subset of extract_multimodal_output and
extract_audio_from_mm from the base module. The 0.24 multimodal_output property
already descends into the first completion output (and falls back to the request
level), so the manual outputs[0].multimodal_output access was redundant, as was
the audio/model_outputs guard that extract_audio_from_mm already applies. Call
the shared helpers at the site and drop the bespoke reimplementation.

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>

---------

Signed-off-by: Noppanat Wadlom <noppanat.wad@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants