Repository navigation
[Feature][TTS] Seed MOSS-TTS residual sampling from the deploy seed - #8078
jingchengtian wants to merge 1 commit into
Conversation
|
This PR appears to belong to: docs/design/module/entrypoints.md. Module owners: @alex-jw-brooks @linyueqian @NickCao Routing: @alex-jw-brooks via module of the changed files, CODEOWNERS; @linyueqian via module of the changed files; @NickCao via module of the changed files @jingchengtian, 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. |
8a5ee81 to
4197479
Compare
Omni ReviewBot routing recordAssigned Direct under experiment |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
This PR opts MOSS-TTS Local/Realtime streaming into the shared Qwen3 chunk-ramp and adaptive-chunk helpers inside talker2codec_raw_async_chunk, while leaving the original 1-then-15 emit path as the default when those keys are unset. Because MOSS deletes emitted rows instead of keeping a full frame history, the processor adds a per-request _ramp_total_emitted counter and tries to feed a reconstructed cumulative length into compute_ramp_emit. Serving also copies the Qwen3 tts_local_seed deploy-default fallback onto MOSS Speech API requests so bench runs without request.seed stop using global RNG; the Local YAML only documents the new keys as comments.
Change flow
flowchart TD
A["[EXISTING] Stage-0 talker async frames"]:::existing
B["[CHANGED] talker2codec_raw_async_chunk"]:::changed
C["[NEW] _ramp_total_emitted + ramp/adaptive emit"]:::new
D["[EXISTING] chunk_size_utils parse/emit helpers"]:::existing
E["[CHANGED] serving_speech tts_local_seed fallback"]:::changed
F["[CHANGED] moss_tts_local.yaml commented keys"]:::changed
G["[EXISTING] Stage-1 codec streaming"]:::existing
A --> B
B --> C
C --> D
D --> G
E --> G
F --> B
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
See inline comments below.
| transfer_manager.ramp_chunk_count.pop(req_id, 0) | ||
|
|
||
| # Track chunk count for ramp / adaptive indexing. | ||
| if hasattr(transfer_manager, "ramp_chunk_count"): |
There was a problem hiding this comment.
[P1] Processor double-increments adapter-owned ramp_chunk_count
talker2codec_raw_async_chunk increments transfer_manager.ramp_chunk_count[req_id] after every emit, including after the finished pop on the previous lines. The production async sender already increments that same dict after a successful put (OmniChunkTransferAdapter._send_single_request_for_generation and mixin send_chunk). Qwen3's processor only reads the counter; test_qwen3_tts_async_chunk.py advances it in the test harness to mimic the adapter. With codec_chunk_ramp: [4, 4, 8, 15], chunk 0 can still emit 4 frames, but the next call sees chunk_index=2 and waits for ramp_cumulative(2)=16 instead of 8, so the advertised ramp is not the schedule that runs. A failed put is worse: the processor has already stepped the index and the adapter has not. Remove the processor-side increment and leave ownership with the adapter.
| ) | ||
|
|
||
| elif ramp is not None: | ||
| emit, emit_frames = compute_ramp_emit(cumulative_length, chunk_index, ramp, chunk_frames, is_finished) |
There was a problem hiding this comment.
[P1] Ramp emit window is treated as a delete-buffer slice length
compute_ramp_emit returns length - prev_threshold, the Qwen3 keep-all-frames window from the previous cumulative boundary. This processor deletes emitted rows from pending_frames, then slices pending_frames[:emit_frames] and adds that window to _ramp_total_emitted. After any overshoot — async_scheduling: true can deliver more than one talker row before the next emit — the next window is larger than leftover pending: the cumulative counter jumps ahead of real frames, a later pad-only or empty step can hit torch.stack([]), and finish flush math is wrong. When emit is true, ship the actual unsent pending count and advance _ramp_total_emitted by that count; do not treat the Qwen3 window length as a delete-buffer size.
| # Unlike Qwen3-TTS (which accumulates ALL frames in code_prompt_token_ids | ||
| # and slices windows), MOSS-TTS deletes emitted frames from pending_frames. | ||
| # So we track a cumulative total for ramp/adaptive threshold math. | ||
| if ramp is not None or adaptive_enabled: |
There was a problem hiding this comment.
[P1] Enabled ramp/adaptive path has no CI-aligned regression pin
The new ramp/adaptive branches and the MOSS tts_local_seed fallback have no focused tests. The CI guards that cover these paths are "Simple · Model Executor Test" (pytest -sv tests/model_executor -m 'core_model and cpu') and "Simple · Engine&Entrypoints Test" (pytest -sv tests/entrypoints tests/engine -m 'core_model and cpu'). Qwen3 already pins the same contracts in tests/model_executor/stage_input_processors/test_qwen3_tts_async_chunk.py and test_prepare_speech_generation_qwen3_default_seed_sets_tts_local_seed. The PR only reports vllm bench serve --backend openai-audio-speech ... on 910B plus an informal default-path [1, 15, 15] check — those CI selectors were not run and would not collect any new moss processor cases. Add a moss raw-async-chunk pin (default / ramp / adaptive / finished / abort cleanup, with adapter-style ramp_chunk_count increments) and a serving seed-fallback test.
| finished = bool(is_finished and len(pending_frames) == 0) | ||
|
|
||
| # Track cumulative total for ramp threshold math. | ||
| transfer_manager._ramp_total_emitted[req_id] += emit_frames |
There was a problem hiding this comment.
[P2] _ramp_total_emitted is never reclaimed on finish or abort
_ramp_total_emitted is a new per-request defaultdict. Adapter/mixin cleanup pops ramp_chunk_count and _adaptive_states only, so this map survives every finished request and every abort that never sets is_finished. The finished block also pops ramp_chunk_count and then immediately does += 1, recreating that key if adapter cleanup does not run. Empty-finished early returns likewise skip _ramp_total_emitted / _adaptive_states pops. Over a long-lived server these host maps grow without bound. Pop _ramp_total_emitted on every terminal path, do not increment after pop, and keep abort cleanup aligned with the adapter.
Omni ReviewBot: finding feedback[p1] Processor double-increments adapter-owned ramp_chunk_count —
This finding appears in the bot's COMMENT review, but GitHub could not place it as an inline diff comment. If you are the PR author and disagree, react 👎 to this comment. The disagreement will be shown to the maintainer; it does not approve or merge the PR. |
Omni ReviewBot: finding feedback[p1] Ramp emit window is treated as a delete-buffer slice length —
This finding appears in the bot's COMMENT review, but GitHub could not place it as an inline diff comment. If you are the PR author and disagree, react 👎 to this comment. The disagreement will be shown to the maintainer; it does not approve or merge the PR. |
Omni ReviewBot: finding feedback[p1] Enabled ramp/adaptive path has no CI-aligned regression pin — The new ramp/adaptive branches and the MOSS This finding appears in the bot's COMMENT review, but GitHub could not place it as an inline diff comment. If you are the PR author and disagree, react 👎 to this comment. The disagreement will be shown to the maintainer; it does not approve or merge the PR. |
4197479 to
5d6da7c
Compare
|
Thank you for the detailed review. I've addressed all 4 findings in the latest push (commit 5d6da7c): P1: Processor double-increments adapter-owned ramp_chunk_countFixed. Removed the processor-side Verified with P1: Ramp emit window treated as delete-buffer slice lengthFixed. The common emit path now clamps actual_emit = min(emit_frames, pending)
chunk_rows = pending_frames[:actual_emit]
del pending_frames[:actual_emit]
transfer_manager._ramp_total_emitted[req_id] += actual_emitThis ensures P2: _ramp_total_emitted never reclaimed on finish or abortFixed.
Verified with P1: No CI-aligned regression pinAdded.
All 13 tests pass: |
Omni ReviewBot triage noteResolved as of |
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
PR description
MOSS-TTS Local streaming reused Qwen3's shared ramp and adaptive chunk-size helpers so Stage 0 no longer jumps from a 1-frame first codec chunk to 15-frame steady chunks. The raw async processor now chooses emit size from codec_chunk_ramp or AdaptiveChunkController, tracks cumulative emitted frames separately because it deletes sent rows, and leaves ramp_chunk_count increment/cleanup to the chunk adapter. Serving also copies the deploy default seed into tts_local_seed when a MOSS request omits seed, so bench/serve paths stay reproducible; deploy YAML only documents the new keys and leaves the old 1→15 default on.
Change flow
flowchart TD
talker["[EXISTING] Stage-0 raw codec rows"]:::existing
utils["[EXISTING] chunk_size_utils ramp/adaptive API"]:::existing
proc["[CHANGED] talker2codec_raw_async_chunk"]:::changed
adapter["[CHANGED] chunk_transfer_adapter state cleanup"]:::changed
speech["[CHANGED] serving_speech tts_local_seed fallback"]:::changed
tests["[NEW] test_moss_tts_async_chunk.py"]:::new
codec["[EXISTING] Stage-1 streaming codec decode"]:::existing
talker --> proc
utils --> proc
proc --> adapter
adapter --> codec
speech --> talker
tests --> proc
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
See inline comments below.
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_moss_tts_seed_fallback_sets_tts_local_seed(): |
There was a problem hiding this comment.
[P2] Seed-fallback tests never call serving_speech
The two new seed tests rebuild copy.deepcopy + setdefault("tts_local_seed", …) in the test body and never import or invoke OmniSpeechServing / _prepare_speech_generation. They stay green if the live moss fallback at serving_speech.py:1993 is removed or if _tts_model_type misses moss_tts_local. They also live under tests/model_executor, so the Engine&Entrypoints guard that covers serving_speech.py would not collect a real pin. Replace them with an entrypoints test that constructs a MOSS speech server, leaves request.seed unset, and asserts extra_args["tts_local_seed"] equals the stage-0 default (including seed=0).
Omni ReviewBot: no human activity for 8 days@jingchengtian this pull request has had no human commit, comment or review since 2026-09-24. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state. To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline. |
…S-TTS Rebased onto latest main after vllm-project#8124 (static chunk ramp) and vllm-project#8213 (MOSS Local 1.5 MRV2 serving) were merged. The static ramp portion of the original PR is now redundant — vllm-project#8124 already landed it using parse_chunk_ramp / ramp_chunk_size. This rebased commit keeps only the two features not yet on main: 1. Adaptive chunk controller (codec_chunk_adaptive): - moss_tts.py: Add adaptive branch to talker2codec_raw_async_chunk. When codec_chunk_adaptive is set, AdaptiveChunkController + compute_adaptive_emit from chunk_size_utils.py (introduced by Qwen3-TTS vllm-project#6001) replace the static ramp/IC-steady threshold. Chunk 0 uses IC/steady (same as original); chunk 1+ uses the controller's buffer-feedback target_size. Adaptive takes precedence over static ramp when both are configured. The ramp threshold override is moved after the adaptive branch so it only applies when adaptive is off. _adaptive_states cleanup is owned by the adapter (chunk_transfer_adapter already pops it on segment-finish and abort on main). - moss_tts_local_v1.yaml: Document codec_chunk_adaptive (commented). 2. MOSS-TTS seed fallback (tts_local_seed): - tts_adapters/moss_tts.py: In apply_sampling_overrides, propagate the deploy config's default_sampling_params.seed to tts_local_seed (matching Qwen3-TTS pattern). Shallow-copies stage 0 params so shared defaults stay immutable. Tests: - test_moss_tts_async_chunk.py: 7 new adaptive tests (chunk 0 IC threshold, finish flush, empty-finish sentinel, adaptive-over-ramp precedence, independent requests, states lifecycle). Existing static-ramp tests unchanged. - test_serving_speech.py: MOSS seed-fallback test via _prepare_speech_generation (entrypoints-level, not model_executor). All 23 tests pass (16 existing + 7 new). Signed-off-by: jingchengtian <tjc1995@126.com>
5d6da7c to
3bdcf74
Compare
Rebase onto latest main — conflict resolvedThe branch has been force-pushed with a clean rebase onto current
What was redundant and removedThe original PR's static ramp path ( What this PR now contributes (only things not yet on main)1. Adaptive chunk controller (
2. MOSS-TTS seed fallback (
Files changed (5 files, +306/-4)
Test resultsAll 23 tests pass ( Backward compatibility
Review bot findings addressedAll 4 original P1/P2 findings from the review bot are resolved in this rebase:
|
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
Changes since the previous review
- 1 new inline finding(s); 0 finding(s) below.
- Fixed: Seed-fallback tests never call serving_speech —
tests/model_executor/stage_input_processors/test_moss_tts_async_chunk.py
CI at
3bdcf74024a7(2026-10-09T10:19:21.657708+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Full review analysis
PR description
The frozen diff does not add the static codec ramp described in the PR body. That ladder is already on main. This head adds an opt-in adaptive branch in MOSS talker2codec_raw_async_chunk: when connector extra codec_chunk_adaptive is enabled it takes precedence over the existing ramp, keeps chunk 0 on the initial/steady threshold, and uses AdaptiveChunkController for later chunks. The deploy file only comments the switch, so default streaming stays on the current initial/steady or static-ramp path. The MOSS adapter also copies stage sampling params and writes stage 0's seed into extra_args["tts_local_seed"] so a missing request seed can follow the YAML default.
Change flow
flowchart LR
cfg["[EXISTING] Connector extra config"]:::existing
proc["[CHANGED] talker2codec_raw_async_chunk"]:::changed
ctrl["[EXISTING] AdaptiveChunkController"]:::existing
ramp["[EXISTING] Static ramp or IC/steady"]:::existing
adapter["[CHANGED] Moss apply_sampling_overrides"]:::changed
seed["[CHANGED] Stage-0 tts_local_seed"]:::changed
cfg --> proc
proc --> ctrl
proc --> ramp
adapter --> seed
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
See inline comments below.
🤖 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!
| if default_seed is not None: | ||
| if stage0_params.extra_args is None: | ||
| stage0_params.extra_args = {} | ||
| stage0_params.extra_args.setdefault("tts_local_seed", int(default_seed)) |
There was a problem hiding this comment.
[P1] tts_local_seed keeps the YAML seed over an explicit request seed
Evidence and suggested fix
apply_sampling_overrides never reads request.seed. Whenever stage 0's seed is not None it setdefaults that value into extra_args["tts_local_seed"]. moss_tts_local_v1.yaml sets stage-0 seed: 42, so the new key is installed for ordinary Local requests. MOSSTTS-3a says Local consumes tts_local_seed (including seed 0) and that an explicit request seed must beat the stage default. build() still records request.seed on tts_params["seed"], but this later write can disagree with it. The comment that the generic serving layer restores precedence is not implemented in this function, and serving_speech.py is not in the snapshot, so a later overwrite of this key was not verified. test_prepare_speech_generation_moss_default_seed_sets_tts_local_seed mocks build(), leaves request.seed unset, and only asserts the default 42, so an explicit seed or seed 0 stays green. If extra_args is already a shared dict, copy.copy aliases it and setdefault mutates the stage default. Write request.seed when it is not None, including 0, and use the stage seed only otherwise; copy extra_args before updating it.
Omni ReviewBot: finding feedback[p1] tts_local_seed keeps the YAML seed over an explicit request seed — See the review for details. If you are the PR author and disagree, react 👎 here; the maintainer will see your disagreement. |
|
Reopened in trimmed form: only MOSS-TTS seed fallback (deploy seed → tts_local_seed). Adaptive chunk controller excluded; deploy YAML not re-added (already consolidated on main). See updated body/title for details. |
|
P1 note on |
Purpose
MOSS-TTS residual/talker_mtp sampling reads
tts_local_seedand otherwise falls back to the global RNG, so an unseeded request's audio can depend on batch composition. This PR mirrors the existingqwen3_ttsandaudio8_ttsadapters: whenrequest.seedisNone, propagate the deploy stage-0 seed intotts_local_seedvia the MOSS adapter'sapply_sampling_overrides. An explicitrequest.seedstill wins because the generic serving layer writestts_local_seedafter the adapter hook.Changes
vllm_omni/entrypoints/openai/tts_adapters/moss_tts.py: in_MossTTSAdapterBase.apply_sampling_overrides, callapply_max_new_tokens, then ifsampling_params_list[0].seedis notNone, deep-copy stage 0 andsetdefault("tts_local_seed", int(seed))(usinggetattrfor safety).tests/entrypoints/openai_api/test_serving_speech.py: addmoss_tts_serverfixture andtest_prepare_speech_generation_moss_default_seed_sets_tts_local_seedthat drives_prepare_speech_generationwith an unseeded request and asserts the deploy seed reaches stage-0extra_argswithout mutating the shared default sampling params.Validation
test_prepare_speech_generation_moss_default_seed_sets_tts_local_seed,test_prepare_speech_generation_qwen3_default_seed_sets_tts_local_seed, and related moss/tts adapter tests.ruff check/formatclean; SPDX/forbidden imports/torch CUDA/test marks/tts adapter checks pass for the changed files.Notes