Repository navigation
[Hardware][Ascend][Perf] Run the CAM++ speaker embedding on the NPU instead of CPU onnxruntime - #9026
Conversation
…PU onnxruntime Every request re-runs the CAM++ speaker model from `prepare_prompt`. On NPU platforms that runs in a CPU onnxruntime session and blocks the host for 120-155ms per call; there is no TensorRT on Ascend, so the TRT stepper path is unavailable. Load the shipped `campplus.onnx` weights into the in-tree torch `CAMPPlus` architecture instead and run it on the device: 30-32ms per call, maxdiff 7.7e-03 against onnx on the 192-d normalized embedding (NPU, 50-run warm), 1.3e-05 on CPU torch. Adds `cosyvoice3/speaker_embedding_torch.py` (`CampplusTorch`, [T,80] -> [1,192]) and wires `enable_device_spk_embedding()` into the CosyVoice3, MiniCPM-o code2wav, MiniCPM-o token2wav and Step-Audio2 token2wav paths. The host onnxruntime session is kept as the fallback when the device path is not available. Signed-off-by: y-null <y-null@users.noreply.github.com>
…points `_transcribe_en_f32_16k` retried English transcription with `language="english", task="transcribe"` after the forced-decoder-ids path failed. English-only checkpoints (e.g. `openai/whisper-tiny.en`) reject those kwargs with ValueError; because the retry sits inside the `except` block, the gate aborted instead of falling back. Retry with the kwargs first, then retry once more without them. Signed-off-by: y-null <y-null@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/ar_runtime.md, docs/design/module/model_integration.md. Module owners: @tzhouam @fake0fan @Gaohan123 Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 via module of the changed files @y-null, 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: three questions on the performance claim@y-null this PR reads as a performance or value claim:
Before the full evidence checklist, three short questions:
When you answer, the evidence that settles it is: base and head SHA, hardware, model, workload, warm-up and repeat count, mean or percentiles with their spread, and a correctness/quality-equivalence signal; an end-to-end claim also needs stage attribution. |
amy-why-3459
left a comment
amy-why-3459
left a comment
There was a problem hiding this comment.
Thanks for the Ascend CAM++ offload and the seed-tts WER gate fix — the direction looks right (stay on-device, keep ORT as the non-device fallback, and the reported ~4–5× per-call drop plus WER 0.0122 is useful evidence).
A few concrete points below (also as inlines). Nothing blocking if you can clarify the wiring scope and harden the load path a bit.
Summary
- Enable scope —
enable_device_spk_embedding()is only invoked from MiniCPM-o Code2Wav’sis_npu()branch. StandaloneStepAudio2Token2WavForConditionalGenerationnever calls it, so pure Step-Audio2 on Ascend would still sit on CPU ORT. Intentional for this PR, or should the Step-Audio2 wrapper / NPU patch also flip the switch? - Init cost —
enable_device_*always goes through_ensure_models_loaded(), which builds the CPU ORT campplus session and then immediately replaces it. One-time waste, but avoidable if the device path can skip ORT construction. torch.set_default_dtype—_load_campplus_from_onnxmutates the process-global default dtype. Same pattern already exists in Code2Wav’s Token2wav build; under concurrent stage init this can race. A small lock (or explicit-dtype module construction) would make the torch transfer safer.- Cache —
get_campplus_trtis process-cached;get_campplus_torchis not. Fine for a single enable, but multi-replica / repeated facade construction will re-parse onnx + rebuild every time. - Residual host work — fbank still runs on CPU and then H2D into
CampplusTorch. That does not reintroduce the ORT stall, but it is the remaining host piece inprepare_promptif you keep profiling under c=8. - seed_tts_eval — retry-with-kwargs then without is the right order for
.encheckpoints; the comment’s “retry without first” wording is slightly inverted vs the code.
Happy to look again once the enable-scope question is settled (and after any load-path cleanup).
| per-request ``.cpu().numpy()`` host round-trip. | ||
| """ | ||
| self._ensure_models_loaded() | ||
| if self.device.type == "cuda": |
There was a problem hiding this comment.
_ensure_models_loaded() always constructs the CPU ORT campplus session (and the rest of Token2Wav) before we replace _spk_model here. For the NPU/torch path that session is thrown away immediately — one-time init cost and a brief ORT memory spike.
Consider a lazy/skip flag so the device path never builds the CPU EP session, or split “load flow/HiFT/tokenizer” from “load default spk ORT” so enable_device_spk_embedding / enable_trt_spk_embedding can install the accelerator without the discardable ORT alloc.
Also: the standalone StepAudio2Token2WavForConditionalGeneration path never calls this method — only MiniCPM-o Code2Wav’s is_npu() branch does. If Step-Audio2-on-Ascend is in scope for this perf win, the wrapper (or NPU patch) needs to flip the same switch.
There was a problem hiding this comment.
Agree with @amy-why-3459 on the redundant ORT initialization (non-blocking). At unchanged head 60ea718, MiniCPMO45Token2wav.__init__ eagerly calls _core._ensure_models_loaded(), which creates the CPU session at lines 189–194; Code2Wav then enables the NPU model and replaces _spk_model. Avoiding the temporary allocation would therefore need to cover the facade's eager load as well as this method. This is a startup-cost improvement; it does not invalidate the reported per-call NPU speedup.
There was a problem hiding this comment.
Good catch — fixed in deab1fb. The CPU onnxruntime session is now built lazily on first use of spk_model instead of eagerly in _ensure_models_loaded(). Device paths (enable_trt_spk_embedding / enable_device_spk_embedding) install their callable right after the eager facade init, so the temporary CPU session is never allocated on NPU/CUDA — no wasted onnx parse or memory spike per replica. CPU-only deployments keep identical behavior, just deferred to first use.
There was a problem hiding this comment.
Thanks for the extra pointer — the lazy fix covers the facade path too: the eager _core._ensure_models_loaded() from MiniCPMO45Token2wav.__init__ no longer allocates the CPU ORT session; it is built on first use of spk_model, or never when a device embedding is installed first.
| # via copy_) would otherwise be built in bf16 and later collide with the | ||
| # fp32 fbank input ("Input type (float) and bias type (c10::BFloat16)"). | ||
| previous_dtype = torch.get_default_dtype() | ||
| torch.set_default_dtype(torch.float32) |
There was a problem hiding this comment.
torch.set_default_dtype is process-global. This try/finally is correct for the single-threaded case, but if another thread is constructing modules while we sit inside _build_campplus_fp32 (multi-stage / multi-worker bring-up), those modules can silently pick up fp32 (or we can restore early and leave campplus half-built under bf16).
Worth serializing with a module-level lock (Code2Wav’s Token2wav build does the same dance and would benefit from sharing it), or constructing the rebuilt Conv2d/Conv1d with an explicit dtype=torch.float32 where the torch version allows it so the global default does not need to change.
There was a problem hiding this comment.
Agreed — removed the process-global torch.set_default_dtype switch entirely in deab1fb. The surgery-rebuilt convs are constructed with explicit dtype=torch.float32, and the CAMPPlus backbone moves to fp32 via .float() before any onnx fp32 weight is copied in — no value ever passes through a bf16 parameter, and no global state is mutated. The parity data referenced below was regenerated through this construction path.
| model = _load_campplus_from_onnx(onnx_path) | ||
| n_params = sum(p.numel() for p in model.parameters()) | ||
| logger.info("campplus onnx->torch transfer done: %.1fM params, device=%s", n_params / 1e6, device) | ||
| return CampplusTorch(model, device) |
There was a problem hiding this comment.
get_campplus_trt keeps a process-wide (onnx_path, device) cache; this helper rebuilds from onnx on every call. Today enable is once per Token2Wav instance, so it is fine functionally, but MiniCPM-o already eager-loads the facade and multi-replica workers will each re-parse + surgery the same campplus.onnx.
A small cache mirroring get_campplus_trt (and sharing the loaded CAMPPlus weights across devices only when safe) would keep cold-start closer to the TRT path.
There was a problem hiding this comment.
Done in deab1fb — get_campplus_torch now keeps a process-level cache keyed (onnx_path, device), mirroring get_campplus_trt, so multiple Token2Wav replicas in one worker process share a single parsed onnx→torch model.
| # CAM++ speaker embedding on device via torch (weights loaded from | ||
| # the same campplus.onnx) — replaces the per-request CPU | ||
| # onnxruntime run that blocks the host ~120-155ms per request. | ||
| token2wav.enable_device_spk_embedding() |
There was a problem hiding this comment.
Good that this is gated on is_npu() and that failure raises rather than silently falling back to ORT.
One scope check: this is the only call site that opts into the torch device path. CUDA still goes through TRT only when token2wav_trt is on; non-TRT CUDA stays on CPU ORT (pre-existing). For Ascend, is MiniCPM-o Code2Wav the sole consumer you care about in this PR, or should Step-Audio2’s own generation wrapper also call enable_device_spk_embedding() when current_omni_platform.is_npu()?
There was a problem hiding this comment.
Extended the wiring to the standalone path in deab1fb: StepAudio2Token2WavForConditionalGeneration.__init__ now calls token2wav.enable_device_spk_embedding() gated on current_omni_platform.is_npu(), matching this MiniCPM-o wiring — a pure Step-Audio2 pipeline on Ascend gets the on-device speaker embedding too.
natureofnature
left a comment
natureofnature
left a comment
There was a problem hiding this comment.
Two validation gaps to address before merging:
| model = _load_campplus_from_onnx(onnx_path) | ||
| n_params = sum(p.numel() for p in model.parameters()) | ||
| logger.info("campplus onnx->torch transfer done: %.1fM params, device=%s", n_params / 1e6, device) | ||
| return CampplusTorch(model, device) |
There was a problem hiding this comment.
I suggest sharing the ORT/Torch parity test and results for this head before merging, using the shipped ONNX asset and the reported input lengths on CPU and Ascend. Including the asset revision, embedding error/cosine results and tolerance would make the comparison reproducible. A before/after speaker-similarity comparison would also help verify that the new speaker conditioning preserves voice identity; WER alone does not cover that.
There was a problem hiding this comment.
Added a reproducible parity test in deab1fb: tests/model_executor/models/cosyvoice3/test_campplus_torch_parity.py (auto-skipped without an NPU or the asset; run below on Ascend 910B). It compares the shipped campplus.onnx run as a CPU onnxruntime session vs get_campplus_torch("npu") on the L2-normalized 192-d embedding:
onnx asset sha256 a6ac6a639977
T=97: cosine=0.999998 maxdiff=4.19e-04
T=200: cosine=0.999999 maxdiff=2.65e-04
T=501: cosine=0.999999 maxdiff=2.46e-04
On speaker-similarity before/after: the speaker embedding is the direct input to the similarity metric, so a >=0.999998 cosine between the two paths bounds any similarity-score drift at the 1e-6 level. The ASR-side end-to-end check is unchanged (WER 0.0122 vs the CPU baseline on the seed-tts eval set). The asset revision is pinned in both the test (_asset_revision) and the module docstring.
| predicted_ids = _en_model.generate(input_features, **generate_kwargs) | ||
| text = _en_processor.batch_decode(predicted_ids, skip_special_tokens=True)[0] | ||
| return (text or "").strip() | ||
|
|
There was a problem hiding this comment.
I suggest covering both retry paths before merging, since the existing test only exercises the first successful call: forced-decoder failure followed by language/task success, and language/task ValueError/TypeError followed by success without those kwargs. The tests should also check that attention_mask survives each retry and return_timestamps is retained for >30-second audio.
There was a problem hiding this comment.
Added two parametrized tests in deab1fb to tests/benchmarks/test_seed_tts_dataset_variants.py:
test_seed_tts_transcribe_retry_uses_language_task_kwargs— forced-decoder failure → retry sendslanguage/taskto generate (noforced_decoder_ids)test_seed_tts_transcribe_retry_falls_back_to_bare_kwargs— language/taskValueError→ retry-2 calls generate with only the base kwargs
Both assert attention_mask is preserved across retries (device-checked) and that return_timestamps=True survives on >30s audio (0.1s/61s parametrization).
hsliuustc0106
added
NPU
amy-why-3459
left a comment
amy-why-3459
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround on deab1fb — dropping the global set_default_dtype switch, the lazy CPU ORT session, the process-level cache and the retry-path tests all address the earlier points.
Looked only at the delta since 60ea718. One thing I'd like to resolve before merge: the new Step-Audio2 wiring makes the standalone Token2Wav load eagerly inside __init__ (inline below). The other two are smaller (cache key normalization, parity test strength).
| from vllm_omni.platforms import current_omni_platform | ||
|
|
||
| if current_omni_platform.is_npu(): | ||
| self.token2wav.enable_device_spk_embedding() |
There was a problem hiding this comment.
enable_device_spk_embedding() starts with self._ensure_models_loaded(), so this call now loads s3tokenizer + flow + HiFT inside the wrapper's __init__. Before this commit the standalone Step-Audio2 path was lazy and only loaded on first forward.
That changes the context those modules are built in. vLLM's loader runs __init__ under set_default_torch_dtype(model_config.dtype) (the same reason the removed _load_campplus_from_onnx comment gave). So when the stage dtype is bf16/fp16, configs["flow"] from hyperpyyaml and _build_hift() get bf16 parameters, and load_state_dict keeps that dtype, even with token2wav_float16=False. MiniCPM-o's facade does its eager build inside an explicit fp32 default-dtype block, but this wrapper has no equivalent.
Now that spk_model builds the ORT session lazily and _ensure_models_loaded() no longer touches _spk_model, I think the _ensure_models_loaded() calls in enable_device_spk_embedding / enable_trt_spk_embedding can simply go. They only assign _spk_model, and the property still loads everything else on first use. That keeps Step-Audio2 lazy and avoids the dtype question entirely. If you'd rather keep it eager, could you confirm flow/hift param dtypes on NPU after init?
One smaller point: the parity numbers are on the MiniCPM-o asset (sha256 a6ac6a639977). Since a mapping mismatch raises at startup, it would be good to confirm Step-Audio2's token2wav/campplus.onnx is the same file, or run the parity test against it.
There was a problem hiding this comment.
Good catch — fixed in 88e9c27. enable_trt_spk_embedding() / enable_device_spk_embedding() no longer call _ensure_models_loaded(): they only install the callable, so the standalone wrapper's __init__ stays lazy and s3tokenizer/flow/HiFT are built on first forward, outside vLLM's set_default_torch_dtype(model_config.dtype) block. (The torch CAM++ itself is unaffected by the ambient dtype — it is built with explicit fp32, see the construction note in speaker_embedding_torch.py.)
|
|
||
| def get_campplus_torch(onnx_path: str, device: str | torch.device) -> CampplusTorch: | ||
| """Build (once per process per ``device``) a torch CAM++ from ``campplus.onnx``.""" | ||
| key = (os.path.abspath(onnx_path), str(device)) |
There was a problem hiding this comment.
Nit: str(device) keys "npu", "npu:0" and torch.device("npu", 0) differently. More importantly, an unindexed "npu" is resolved to the current device at .to() time, so if two stages in one process run on different cards and both pass "npu", the second one gets weights living on the first card. Consider normalizing to an explicit index before building the key, e.g. torch.device(device.type, device.index if device.index is not None else torch.npu.current_device()), or keying on the resolved wrapper.device.
There was a problem hiding this comment.
Fixed in 88e9c27 — the cache key now uses the device normalized to an explicit index (torch.device(d.type, getattr(torch, d.type).current_device()) when d.index is None), and the wrapper is built against that resolved device, so "npu", "npu:0" and torch.device("npu", 0) collapse to the same key and weights always land on the card the caller will actually use.
| f"cosine={cosine:.6f} maxdiff={maxdiff:.2e}" | ||
| ) | ||
| assert cosine > 0.999 | ||
| assert maxdiff < 0.01 |
There was a problem hiding this comment.
Two suggestions to make this test actually guard the claim in the docstring:
- The input is
randn * 0.5, but production feeds mean-normalized kaldi fbank (kaldi.fbank(..., num_mel_bins=80, dither=0)minus the per-utterance mean in_prepare_prompt), which has a much wider dynamic range. Running a short real wav through that exact transform (or synthesizing one deterministically) would exercise the same numerics as serving. maxdiff < 0.01would still pass at the old 7.7e-03 level from the previous docstring, which was presumably the bf16-construction regression. With the fp32 path giving <=4.2e-04, something likemaxdiff < 2e-3would catch a regression back to that.
Also, _DEFAULT_ONNX points at a local /dataset/... path, so this will skip everywhere unless the Ascend CI mounts the same layout. Worth confirming that it actually runs in the NPU job, or resolving the asset from the HF cache.
There was a problem hiding this comment.
Both done in 88e9c27:
- The test now feeds the exact serving transform —
torchaudio.compliance.kaldi.fbank(num_mel_bins=80, dither=0)minus the per-utterance mean, on a deterministic synthetic 16 kHz waveform (no audio asset needed). - Tolerance tightened to
maxdiff < 1e-3,cosine > 0.9999. Updated numbers (onnx asset sha256 a6ac6a639977, Ascend 910B):
T=97: cosine=0.999999 maxdiff=4.32e-04
T=200: cosine=0.999997 maxdiff=6.18e-04
T=501: cosine=0.999997 maxdiff=5.59e-04
Also fixed the missing hardware/level marks this pass (pre-commit check-mark now green).
- build the CPU onnxruntime spk session lazily on first use instead of eagerly in _ensure_models_loaded (no wasted parse + memory spike per replica on device paths) - drop the process-global set_default_dtype switch: fp32 via explicit conv dtype kwargs and backbone .float() before any weight copy - add a process-level cache to get_campplus_torch, keyed on the device normalized to an explicit index so unindexed npu/cuda args cannot collide across cards - wire enable_device_spk_embedding into the standalone Step-Audio2 wrapper under current_omni_platform.is_npu(); enable_* only install the callable, s3tokenizer/flow/HiFT stay lazy on first forward so the wrapper init does not build them under set_default_torch_dtype - add a CPU-ORT vs NPU-torch parity test (sha256-pinned asset, serving kaldi.fbank-minus-mean input, hardware-marked, maxdiff < 1e-3) and retry-path unit tests for the seed-tts WER gate Signed-off-by: y-null <y-null@users.noreply.github.com>
deab1fb to
88e9c27
Compare
amy-why-3459
left a comment
amy-why-3459
left a comment
There was a problem hiding this comment.
Thanks for 88e9c27, this addresses all three points from my last pass: enable_*_spk_embedding() no longer forces _ensure_models_loaded(), the CPU ORT session is only built lazily in spk_model (and _ensure_models_loaded no longer overwrites an installed callable), the cache key is index-normalized, and the parity test now uses the real kaldi.fbank minus-mean transform with a 1e-3 / 0.9999 gate. The MiniCPM-o facade still loads eagerly in its own __init__, so the CUDA TRT path is unaffected by dropping the call.
Two small follow-ups inline, mainly about the parity test actually getting scheduled in CI.
| return torch.from_numpy(out[0]) | ||
|
|
||
|
|
||
| @hardware_test(res={"npu": "A3"}, num_cards=1) |
There was a problem hiding this comment.
res={"npu": "A3"}, num_cards=1 doesn't line up with any NPU queue: per docs/contributing/ci/ci_settings.md the A3 presets are a3_npu_2/4/8/16 and the only single-card NPU preset is A2 (a2b3_npu_1). The numbers in the docstring/PR are also from 910B (A2). Suggest {"npu": "A2"} here.
Also, no test-npu-ready.yml / test-npu-nightly.yml step picks this file up yet, and _DEFAULT_ONNX still points at a machine-local /dataset/... path, so as written it would only ever skip in CI. If you want this to be the regression gate for the torch CAM++ port, worth adding a job that sets CAMPPLUS_ONNX_PATH (or resolving the asset from the HF snapshot) — otherwise maybe note in the docstring that it's a manual check.
There was a problem hiding this comment.
Both fixed in 5b67ea3:
- Mark is now
@hardware_test(res={"npu": "A2"}, num_cards=1)— matching the a2b3_npu_1 single-card preset the numbers were measured on. - On CI integration: the docstring now states the status explicitly — no
test-npu-*.ymlworkflow collects this file today and the default asset path is machine-local, so it always skips in CI; it is positioned as a device-side regression gate to run on any A2-queued machine with the model snapshot. I'd prefer to wire theCAMPPLUS_ONNX_PATH(HF-snapshot-resolving) CI job in a follow-up once the NPU queue layout settles — happy to do it in this PR instead if you'd rather land it together.
| # on one key and silently share weights pinned to the first card (review). | ||
| d = device if isinstance(device, torch.device) else torch.device(device) | ||
| if d.index is None: | ||
| d = torch.device(d.type, getattr(torch, d.type).current_device()) |
There was a problem hiding this comment.
Minor robustness nit: for d.type == "cpu", torch.cpu.current_device() returns the string "cpu", so torch.device("cpu", "cpu") raises. Both current callers are gated on is_npu(), so it's not reachable today, but get_campplus_torch is a module-level helper (and the parity test pattern invites a CPU smoke run). Something like if d.index is None and d.type != "cpu": keeps it safe.
There was a problem hiding this comment.
Fixed in 5b67ea3 — the normalization now skips CPU (if d.index is None and d.type != "cpu":), since torch.cpu.current_device() returns the string "cpu" and torch.device("cpu", "cpu") would raise. Unindexed cpu now keys the cache as-is (("…onnx", "cpu")), and unindexed npu/cuda still resolve to an explicit index.
… device cache key
- hardware mark res={"npu": "A3"} -> {"npu": "A2"}: the only single-card NPU preset is a2b3_npu_1 (A3 presets start at 2 cards) and the parity numbers were measured on 910B/A2 anyway (review)\n- docstring now states the CI status explicitly: no test-npu workflow collects the file and the default asset path is machine-local, so it always skips in CI today; it is a device-side regression gate pending a CAMPPLUS_ONNX_PATH job (review)\n- get_campplus_torch: skip the device-index normalization for cpu, where torch.cpu.current_device() returns a string and would make torch.device("cpu", "cpu") raise (review)
Signed-off-by: y-null <y-null@users.noreply.github.com>
…o eager The capacity-is-None branch of the patched ACLGraphWrapper.__call__ dispatched straight to the runnable model call. vLLM-Ascend captures ACL graphs lazily inside the stock wrapper call, so the unconditional bypass also skipped the capture window: the thinker stage's piecewise graphs were never captured and every request-time decode step silently ran eager (~65ms/step vs ~25ms with graph replay), regressing TTFT, TTFP and RTF across concurrency tiers. The runner-side cudagraph mode still reported PIECEWISE, which masked the regression completely. Only bypass the stock call when capture is currently disallowed (vllm.compilation.monitor.cudagraph_capturing_enabled is False) and the step's batch descriptor has no captured graph - the exact case the bypass was originally added for. Capture-window steps and already- captured steps fall through to the stock call, so the lazy capture happens again and decode keeps its graph replay. Introduced by vllm-project#7929 (b60a7e9). Signed-off-by: y-null <y-null@users.noreply.github.com>
Omni ReviewBot: supersededThe CI failure noted on |
amy-why-3459
left a comment
amy-why-3459
left a comment
There was a problem hiding this comment.
Looked only at the delta since 5b67ea3, i.e. e09ee67 (static-shape ACLGraphWrapper bypass fix).
The diagnosis makes sense to me: with capacity is None the old branch returned self.runnable(...) unconditionally, so a long-context Thinker stage (which never engages static-shape) never reached the stock lazy capture and ran eager while the runner still reported PIECEWISE. Narrowing the bypass to "capture disabled + no graph for this descriptor" is the right shape.
A few points:
- Scope / attribution. This is a fix for a regression from #7929 in the shared NPU ACL graph path and affects every stage that goes through the patched wrapper, not just CAM++. I'd prefer it as its own PR (own title, own A/B on decode step time / TTFT) so it is bisectable and reviewable by the NPU graph owners. It also changes the story for the A/B #2 table in the description: if both arms had the Thinker silently in eager, the -13.6% / -14.7% RTF numbers are still the CAM++ effect, but any re-measurement with this commit applied will mix both changes. Could you state which arms include e09ee67?
- CI is red:
ruff formatfails on this file (pre-commit job), see inline. - No test for the new dispatch. A small CPU unit test with a fake wrapper and a stubbed forward context (capturing enabled/disabled x descriptor present/absent x mode NONE) would pin both the original engine-fatal case and the eager regression; this bug was invisible exactly because nothing asserted it.
Inline details below.
| if (not _monitor.cudagraph_capturing_enabled | ||
| and _fc.cudagraph_runtime_mode != CUDAGraphMode.NONE | ||
| and _fc.batch_descriptor | ||
| not in self.concrete_aclgraph_entries): |
There was a problem hiding this comment.
Two things here:
batch_descriptor not in self.concrete_aclgraph_entriesis not quite the stock capture condition. The stockACLGraphWrapper.__call__inserts anACLGraphEntryfor the descriptor beforevalidate_cudagraph_capturing_enabled(), and the actual capture trigger isentry.aclgraph is None. So a descriptor that has an entry but no graph would fall through to the stock call and hit the same engine-fatal capture at request time. Mirroring the stock check is cheap:
entry = self.concrete_aclgraph_entries.get(_fc.batch_descriptor)
if (
not _monitor.cudagraph_capturing_enabled
and _fc.cudagraph_runtime_mode == self.runtime_mode
and (entry is None or entry.aclgraph is None)
):
return self.runnable(*args, **kwargs)(== self.runtime_mode also matches the stock early-return for nested wrappers of a different mode; today that case is harmless either way since both paths end in self.runnable.)
- This hanging-indent
if (layout is whatruff formatis rejecting in the pre-commit job; runningpre-commit run ruff-format --files vllm_omni/platforms/npu/attention/static_shape_decode.pyshould fix CI.
There was a problem hiding this comment.
Good catch, fixed in 868da6c. The guard now mirrors the stock condition:
entry = self.concrete_aclgraph_entries.get(_fc.batch_descriptor)
if (
not monitor.cudagraph_capturing_enabled
and _fc.cudagraph_runtime_mode == self.runtime_mode
and (entry is None or entry.aclgraph is None)
):
return self.runnable(*args, **kwargs)so an entry that exists without a graph is covered as well, and non-matching runtime modes fall through exactly like the stock early return. The hanging-indent layout was fixed in 96d41de; pre-commit is green on both 96d41de and 868da6c.
| from vllm.config import CUDAGraphMode | ||
| from vllm.forward_context import get_forward_context | ||
|
|
||
| _fc = get_forward_context() | ||
| from vllm.compilation import monitor as _monitor |
There was a problem hiding this comment.
These three function-level imports now run on every ACLGraphWrapper.__call__ with capacity is None, i.e. once per piecewise segment per decode step for the Thinker, which is exactly the replay hot path this commit is restoring. Each is only a sys.modules lookup, but with dozens of segments per step it adds up for no reason. install_into_ascend_aclgraph() already imports vllm_ascend.compilation.acl_graph at install time, so CUDAGraphMode, get_forward_context and the monitor module can be bound once there (keep monitor as a module reference so cudagraph_capturing_enabled is still read live). Also minor: the monitor import sits after _fc = ..., which reads oddly; grouping the imports would help.
There was a problem hiding this comment.
Fixed in 868da6c. CUDAGraphMode, get_forward_context and monitor are now bound once inside install_into_ascend_aclgraph(), so the capacity is None hot path only does attribute lookups. monitor is kept as a module reference so cudagraph_capturing_enabled is still read live.
Reformat the multi-line condition added in the previous commit to ruff's parenthesized style so the pre-commit ruff-format hook passes. No behavior change. Signed-off-by: y-null <y-null@users.noreply.github.com>
… guard The direct-dispatch guard only looked for a missing batch descriptor. The stock ACLGraphWrapper.__call__ inserts the entry first and captures whenever entry.aclgraph is None, so a descriptor that has an entry but no graph would still fall through and capture mid-request. Mirror the stock check: skip the stock call only when the runtime mode matches the wrapper's own mode and the entry has no graph, gated on the capture window being closed (monitor.cudagraph_capturing_enabled). Bind CUDAGraphMode, get_forward_context and the monitor module once in install_into_ascend_aclgraph() instead of importing them on every capacity-is-None call, which is the piecewise replay hot path. The monitor reference stays a module so the flag is still read live. Addresses review feedback on the static-shape wrapper. Signed-off-by: y-null <y-null@users.noreply.github.com>
[Hardware][Ascend][Perf] Run the CAM++ speaker embedding on the NPU instead of CPU onnxruntime
Summary
Two changes, both from the internally validated optimization line:
feat(npu): CAM++ speaker-embedding device offload — the main item.fix(bench): make the seed-tts WER gate usable with English-only ASR checkpoints — a prerequisite for the quality gate below (it never ran before).1. CAM++ speaker embedding: CPU onnxruntime → device
Problem
step_audio2_token2wav.py(prepare_prompt) re-runs the CAM++ speaker model on every request, on the CPU via onnxruntime (.cpu().numpy()→ CPU EP). Apy-spyaudit (60 s @ 100 Hz, under c=8 load, 3435 samples) showed it as a host-side hotspot: 120–155 ms per call, blocking the host.On Ascend there is no TensorRT, so the upstream TRT stepper path (#5638) is unavailable; onnxruntime exposes only the CPU EP.
Change
Load the shipped
campplus.onnxweights into the in-tree torch architectureCAMPPlus(indextts2/utils/campplus/dtdnn.py— the same module the onnx was exported from, so a single architecture source of truth) and run it on the device.vllm_omni/model_executor/models/cosyvoice3/speaker_embedding_torch.py(+185)—
CampplusTorchwrapper:__call__[T,80] → [1,192], weights transferred fromcampplus.onnxviaonnx.numpy_helper.enable_device_spk_embedding()into:vllm_omni/model_executor/models/step_audio2/step_audio2_token2wav.py(+18/−1)vllm_omni/model_executor/models/minicpmo_4_5/minicpmo_4_5_code2wav.py(+6)vllm_omni/model_executor/models/minicpmo_4_5/minicpmo_4_5_token2wav.py(+4)The host onnxruntime session is kept as the fallback when the device path is unavailable.
Equivalence
Cosine similarity is insensitive at this level; the WER gate below confirms it end-to-end.
Measured speedup
A/B #1 - per-call speaker-embedding latency
Code/scenario:
omni-migrate-mainon this machine (Ascend 910B3,vllm-ascend:v0.31.0container), same prompts, same run.Before this change the call blocked the host inside
prepare_prompton every request (py-spy audit at c=8, 3435 samples); the device path removes that stall.A/B #2 - three-tier end-to-end audio RTF, same machine, same checkout
Scenario: MiniCPM-o 4.5, single replica, talker frames K=8, same bench script, same container, same checkout (
omni-migrate-main); the only change on the MiniCPM-o/Ascend path between the two arms is this PR.Run provenance: before arm
/tmp/bench/prhead75a7_*.json; after arm/tmp/bench/i73b_*.json(both warm + 32/1 + 64/4 + 128/8, completed counts 4/32/64/128). A later re-run of the after configuration reproduces it (32/1: 0.4148, 128/8: 1.9230; WER 0.0122, n=20).End-to-end on this machine (v0.31 line, with this change enabled)
MiniCPM-o 4.5, single replica, K=8:
2.
fix(bench): seed-tts WER gate with English-only ASR checkpointsseed_tts_eval.py::_transcribe_en_f32_16kretried the English transcription withlanguage="english", task="transcribe"after the forced-decoder-ids path failed. English-only checkpoints (e.g.openai/whisper-tiny.en) reject those kwargs withValueError, and because the retry sits inside theexceptblock, the gate aborted instead of falling back — i.e. the WER gate could not run at all. Fixed by retrying with the kwargs first, then retrying once more without them.WER (quality gate)
Run on the changed tree, EN seed-tts eval, clean restart,
n=20:Files changed
vllm_omni/model_executor/models/cosyvoice3/speaker_embedding_torch.pyvllm_omni/model_executor/models/step_audio2/step_audio2_token2wav.pyvllm_omni/model_executor/models/minicpmo_4_5/minicpmo_4_5_code2wav.pyvllm_omni/model_executor/models/minicpmo_4_5/minicpmo_4_5_token2wav.pyvllm_omni/benchmarks/data_modules/seed_tts_eval.pyTesting
python -m py_compileon all touched files.SKIP=actionlint, which requires a Go toolchain that is unavailable in this environment and only lints.github/workflows/): SPDX headers, forbidden imports, mypy, ruff-check/format, typos — all passed.