[amy-why-3459]

Summary

Stage 0 of Drama 3 (fish_omni) samples one whole audio frame per stream per
decode step: 40 Qwen3.5-MoE backbone layers plus 13 codebook-tower layers of
small kernels, whose sampled codes feed the next step's embedding. Decode is
the steady state of a stream, so per-kernel launch cost dominates the
arithmetic. The NPU override added in #9106 asked for PIECEWISE capture,
which leaves the pure decode path eager.

This asks for FULL_AND_PIECEWISE with capture sizes up to max_num_seqs.
Mixed steps (a new request's prompt rows) still replay piecewise backbone
graphs; decode steps replay one graph per captured batch size.

Performance

2x Ascend 910B1 (A2), stage 0 TP2 + expert parallel on 64 streams, vLLM-Omni
0.32.0 nightly (2026-10-22 image), vLLM-Ascend 0.19.1rc2, CANN 9.1.0.
Workload: streamed /v1/audio/speech, Seed-TTS-Eval en, one fixed
pre-encoded reference, max_tokens=500, 64 prompts, 1 warmup. Audio seconds
come from the SSE PCM payloads; RTF = (end - start) / audio_s, so queueing
and prefill are inside the number.

Single stream (C1), two clean runs of each arm:

	frames/s	RTF mean	RTF p95	TTFA p50
PIECEWISE (before)	17.31	1.237	1.301	0.91 s
FULL_AND_PIECEWISE (after)	22.40 / 22.91	0.962 / 0.947	1.141 / 1.036	0.80 / 0.78 s

That is +30% single-stream throughput and -23% RTF, with lower TTFA
because the first decode step after prefill already replays.

Batching on the same build (best of repeats): 133 frames/s at 16 streams,
137 at 32, 305 at 64. The host is a shared 8-card box (load average 15-20
during runs), so single readings of C8-C32 move by tens of percent; the two
C1 repeats above are the attribution.

What was rejected (and why it is not in this PR)

• Binding fla_npu.ops.ascendc.causal_conv1d_update into the GDN update.
  vLLM-Ascend logs that the portable update syncs per request and "stalls ACL
  graph capture at decode-FULL", which suggested it was a prerequisite for
  this change. Measured, it is not: with the binding disabled, FULL graphs
  capture and the numbers match (22.57 frames/s, RTF 0.952 at C1). Dropped.
• Keeping the checkpoint's block-FP8 experts as FP8 on A2. Not a config
  change that exists today: vLLM-Ascend builds its MXFP8 MoE method only on
  950 (is_950()), so A2 expands every expert to bf16 at load. A native A2
  block-FP8 grouped GEMM would be a separate, larger change.
• mamba_ssm_cache_dtype: bfloat16 is already set in this deploy config.

For context on the targets this was chasing (RTF < 0.1 at C1, < 1 at C64):
neither is met (C1 is ~0.95, C64 2-5 depending on host load). One frame is 40
MoE layers plus 13 tower layers and 21.5 frames/s at RTF 0.1 allows 4.65 ms
per frame, while each rank holds ~32 GiB of expanded expert weights, so the
budget sits below the bandwidth floor before KV, tower and sampling. This PR
takes the launch-overhead win that is available now; it does not claim the
target.

Phase timing behind that split (eager, one stream): backbone ~195 ms,
tower + sampling ~72 ms, writeback ~0.1 ms per frame, versus ~44 ms per frame
once decode replays as one graph.

Testing

• pytest tests/model_executor/models/fish_omni/ -q — 85 passed, 24 skipped.
• New CPU test test_npu_deploy_captures_full_decode_graphs pins the override
  (a PIECEWISE-only NPU config would silently revert decode to eager) and
  checks the capture sizes stop at max_num_seqs.
• End-to-end: the numbers above come from this exact commit serving
  /v1/audio/speech on two 910B1 cards; generation content is unchanged (same
  sampling params, same graph-independent output path).

Notes for reviewers

The capture sizes top out at max_num_seqs (64) because the graph captures
the sample rows of its batch size. Mamba/GDN state, tower KV pages and the
per-step frame buffers live at fixed addresses outside the replayed region,
which is what makes capture legal here.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-23T18:03:56.252734Z We couldn’t complete this review. Comment @codex review to try again.	8ed13c2	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[vllm-omni-review-bot]

This PR appears to belong to: docs/design/module/vllm_omni_config.md, docs/design/module/entrypoints.md.

Module owners: @alex-jw-brooks @lishunyang12 @NickCao

Routing: @alex-jw-brooks via module of the changed files, CODEOWNERS; @lishunyang12 via module of the changed files, CODEOWNERS; @NickCao via module of the changed files, CODEOWNERS

@amy-why-3459, 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.

[hsliuustc0106]

@amy-why-3459 — two evidence TODOs for 8ed13c296930b37c246cef26676b2349cf9e5b61:

• Pin the PIECEWISE baseline SHA and attach the runnable benchmark command/input plus per-run logs for both arms on the stated 2×910B1 environment. Keep model revision, workload, warmup, repetitions and measurement scope matched; retain the shared-host variability rather than reporting best runs as a general result.
• Supply paired seeded output quality/completeness results and a decode→mixed/padded-batch transition check for FULL replay. The new test verifies the YAML setting, not capture/replay behavior; unchanged sampling parameters alone do not prove equivalent output.

Acceptance: the reproducible matched runs support the stated throughput/RTF change and show complete, correct output across those replay transitions. No static defect established; I inspected source but did not execute NPU tests.

[amy-why-3459]

@yenuo26 @FayeSpica PTAL

[FayeSpica]

Validated 780efad6e689 on 2× Ascend A3: all 5 online tests passed. No other concerns from this review.

[vllm-omni-review-bot]

Omni ReviewBot: superseded

The CI failure noted on 5ca7ba93d7fc refers to an earlier head; the pull request now points at 04a315900d31.

[natureofnature]

Agree with @hsliuustc0106 on the remaining benchmark reproducibility and replay-transition checks before merging, particularly seeded output comparisons across padded batches and decode→mixed→decode transitions. The A3 online-test results remain applicable to 04a315900, since the relevant code is unchanged. The rest looks good from static review.

[natureofnature]

Rechecked all five changed files and the NPU input preparation → FULL replay → frame writeback path. The capture sizes stay within the 64 request slots, and the ready job selects the smoke test while loading real weights. Current-head NPU CI, main and AMD CI report success; Intel CI is still pending. The Drama 3 code and online tests are unchanged from the revision covered by @FayeSpica's five-test A3 run. I did not run NPU tests in this review.

The previous validation request remains open: the online tests do not compare PIECEWISE against FULL replay or force padded-batch and decode→mixed→decode transitions. Those checks matter because replay reuses request-state buffers while batch membership changes.

【Conclusion: Static review found no blocking code issues in commit 04a3159.】
【Action recommended: Before merging, provide matched PIECEWISE/FULL benchmark logs on the stated 2×910B1 setup and seeded output comparisons across padded batches and decode→mixed→decode transitions, showing complete output without quality regression.】

[yenuo26]

One suggestion on test_npu_deploy_captures_full_decode_graphs: the hand-rolled repo-root resolution (Path(__file__).resolve().parents[4] + yaml.safe_load + manual stage lookup) can reuse the existing deploy-config helpers — tests/config/test_lingbot_world_stepwise_deploy.py pins deploy fields the same way:

from tests.helpers.stage_config import get_deploy_config_path
from vllm_omni.config.stage_config import load_deploy_config

deploy = load_deploy_config(get_deploy_config_path("fish_omni.yaml"))
npu = deploy.platforms["npu"]
stage0 = next(s for s in npu["stages"] if s["stage_id"] == 0)

load_deploy_config keeps the raw platforms block on the parsed object, so the three assertions work unchanged, and the path math (which breaks silently if the test file ever moves) goes away.

Rest verified on my side: the num_cards={"cuda": 1, "npu": 2} dict is the supported per-platform split; the READY job's -m 'core_model …' --run-level 'advanced_model' combination does what the comment says (smoke selection with real weights, since the core_model dummy loader's FP16→FP8 device cast is unsupported on A3) while nightly takes the full advanced_model selection; tts_drama_3_function in the dependency map already covers deploy/fish_omni.yaml, so the new jobs trigger on the right source changes.

[amy-why-3459]

@yenuo26 Thanks for the suggestion in your review. I'll update test_npu_deploy_captures_full_decode_graphs in a follow-up PR to use get_deploy_config_path and load_deploy_config, replacing the hand-written repo-root resolution and YAML parsing while keeping the existing assertions unchanged.

[yenuo26]

LGTM

[amy-why-3459]

@yenuo26 Follow-up to your review: implemented in #9386, commit 2ca0b43. The test now uses get_deploy_config_path and load_deploy_config; all three assertions are unchanged. Validation: all 11 tests in test_fish_omni_config.py passed, and changed-file pre-commit checks passed. This addresses the config-helper cleanup only; NPU benchmark and replay-transition validation were not run as part of this change.