Repository navigation
Conversation
d01635f to
b55078c
Compare
Both MiniMax-Music3 stages run on Model Runner V2 on CUDA. Other platforms stay on V1. - minimax_music3/model_state.py: an MRv2 model state that feeds the talker's existing row binding, frame feedback and guided frame draw from the MRv2 batch, and registers the draw as an OmniSampler. - pipeline.py: both stages declare supports_native_mrv2_data_plane. - Deploy files: model_runner v1 with platforms.cuda.model_runner v2. - Stage input processor: a terminal span published on the step that samples <|audio_end|> is not reported as the talker running ahead of the engine. - Docs and unit tests. Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
…-Music3 On V2 each window reaches stage 1 as its codes, 200 frames x 8 codebooks = 1600 prompt tokens, where V1 uses a one-token placeholder. With max_num_batched_tokens at 4096 only two windows fit per step. A window is never split and the scheduler stops at the first one that does not fit, so the requests at the front of the queue kept every step: with four streams at once, one stream waited 80 s between two pieces of audio and another got its first audio only after ~100 s. Size the budget for every request's window: 16384 for max_num_seqs 8, 32768 for the two-GPU file's 16. V1 never comes near either value. The XPU override (V1, two rows) keeps 4096, which also stays within max_num_seqs x max_model_len. Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
…buffer The MRv2 intermediate buffer already keeps each request's sampling params and drops them when the request is removed, so the talker's model state no longer keeps its own copy per slot. Also: - docs: on MRv2, stage 1's max_num_batched_tokens has to hold max_num_seqs x 1600 tokens; the recipe now points to that section; - the recipe's tables and nested lists follow markdownlint now that the file is in the diff; - both Music3 deploy files join the native-runner platform defaults test. Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
…puts The talker sets omni_pooler_payload_include_hidden = False, which the V1 runner honors. On V2 the switch is the sampler output's include_hidden_states, which defaults to True, so every stage-0 step also copied the backbone hidden state out and published it under the stage's "latent" key, the key the talker's own payload uses. The sampler now returns include_hidden_states=False. The talker's payload and the audio are unchanged. Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
With async_chunk: false, MiniMax-Music3 returned HTTP 200 with no audio on both model runners, and on main. In that mode the engine collects stage 0's outputs and ar2acoustic hands stage 1 the whole song, so: - The talker publishes the whole song once, when it ends, instead of its overlapping windows. - On Model Runner V2 the sampler sets finalize_multimodal: without hidden states the runner builds no per-request payload otherwise. - ar2acoustic accepts the MultimodalPayload the engine hands it, which is a Mapping rather than a dict, and reads the seed V1 delivers as a 0-d tensor. Without the seed, stage 1 draws from seed 0 and returns a different song. - Rows with nothing to publish get empty placeholders in the hidden states' dtype (int64 for codes) rather than float32, so the engine does not turn a bf16 song into float32 when it joins a request's outputs. With async_chunk: true the audio does not change. The SPDX hook updated the headers of two touched files. Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
|
Self-review before asking for review:
|
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 @twu3202, 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. |
amy-why-3459
left a comment
There was a problem hiding this comment.
Thanks for the careful MRv2 port and the byte-identical single-request evidence. Focused on model_state.py, the talker hooks, the stage-1 budget change, and the async_chunk: false payload path.
The overall shape looks right: one talker implementation for both runners, prepare_inputs → prepare_runner_inputs before FULL decode replay, custom sampler matching the Higgs registration route, and the V2 terminal-span / MultimodalPayload Mapping fixes are well motivated.
One concrete budget gap below (last spans vs the new max_num_batched_tokens). The rest is small.
| # is never split, and the scheduler stops at the first window that does | ||
| # not fit, so a smaller budget lets the same few requests take every step. | ||
| max_num_batched_tokens: 16384 | ||
| devices: "0" | ||
| input_connectors: |
There was a problem hiding this comment.
The comment already notes that a last span can be 300 frames → 2400 tokens, but 8 × 2400 = 19200 still exceeds 16384. With several songs ending in the same step, the same “scheduler stops at the first window that does not fit” serialization you fixed for the steady 1600-token windows can come back on the terminal spans.
Suggest sizing stage 1 for the last-span case: at least max_num_seqs × 2400 (19200 here; 38400 in the 2-GPU file, where 16 × 2400 also exceeds 32768). That still sits under max_num_seqs × max_model_len (32768 / 65536).
There was a problem hiding this comment.
Thanks for checking this. The 300-frame figure in my deploy comment was wrong. Windows are released one frame late, so a song's last span is at most 200 frames when the model ends the song, and 201 (1608 tokens) when max_new_tokens ends it on the frame that would have released the next window, because that step sends the last span instead. test_terminal_span_is_bounded now checks both endings. That gives 8 × 1608 = 12,864 against 16,384 and 16 × 1608 = 25,728 against 32,768, so I kept both budgets. When I logged stage 1's scheduler in c=8 runs at the shipped 16,384 budget, no request got more than 1600 tokens in a step and no step more than 12,800, so no request had two windows in one step. The deploy comments and the stage-input docstring, which had the same number, are fixed in ea96092.
| On MRV2 each window reaches stage 1 as its codes, 200 frames x 8 codebooks = | ||
| 1600 prompt tokens, where V1 sends one placeholder token. A window is never | ||
| split, so in your own config keep stage 1's `max_num_batched_tokens` at least | ||
| `max_num_seqs` x 1600 on MRV2. With less, a few requests take every step and | ||
| the other streams wait. |
There was a problem hiding this comment.
Same point as on the deploy file: max_num_seqs × 1600 covers the steady 200-frame window, but the talker’s last span can reach 300 frames (2400 tokens). Worth stating the rule as max_num_seqs × 2400 (or “at least the largest published span × codebook count”) so custom configs do not rediscover the serialization under concurrent song endings.
There was a problem hiding this comment.
Agreed it should cover the last span, but that is 201 frames rather than 300 (see the deploy-file thread), so the rule now reads max_num_seqs x 1608.
| - The acoustic stage runs in float32. bfloat16 is accepted but measurably | ||
| degrades the solver. | ||
| - The external API is non-streaming; `stream: true` is rejected. | ||
| - `max_model_len` is 10,240, so a maximum-length prompt (5,000 tokens) and a |
There was a problem hiding this comment.
Nit while this file is already in the diff: the PR results say both stream_format: "audio" and SSE streaming worked on V1 and V2, but this still claims stream: true is rejected. Worth aligning the known-limitations list with what you measured (or clarifying which streaming form is still refused).
There was a problem hiding this comment.
The rejected-parameters row and the known-limitations line were both stale: the adapter never rejected stream. In my runs stream: true (SSE) and stream_format: "audio" both gave the same audio as on V1, and it started arriving after the first window. I replaced the two lines with a short note on streaming.
The deploy comments said a song's last span can reach 300 frames, and the stage-input docstring said a window plus a hop. Windows are released one frame late, so the last span is at most 200 frames when the model ends the song, and 201 (1608 tokens) when the frame budget ends it on the frame that would have released the next window. The shipped stage-1 budgets already fit that, the docs rule now reads max_num_seqs x 1608, and the frame-state test checks the tighter bound for both endings. The recipe said `stream: true` is rejected. The adapter does not reject it, and the recipe now describes SSE and raw audio streaming. The SPDX hook updated the test file's header. Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
amy-why-3459
left a comment
There was a problem hiding this comment.
Re-reviewed tip ea96092de (delta from 35c697791).
Covered: the corrected last-window bound in the deploy comments / stage-input docstring / stage_configs.md rule, the recipe streaming rewrite, and test_terminal_span_is_bounded for both model-end and budget-end endings.
The one-frame-late release story hangs together with AR_CHUNK_FRAMES=200: last span ≤201 frames → 1608 tokens, so 8×1608=12864 fits under 16384 and 16×1608=25728 under 32768. Keeping the shipped budgets is fine once the 300-frame figure is dropped. The recipe now matches the measured SSE / stream_format: "audio" behavior instead of claiming stream: true is rejected.
No new blockers from this tip-delta.
| spans, _ = self._run(total, budget_ends_song=budget_ends_song) | ||
| assert len(spans[-1]) <= AR_CHUNK_FRAMES + 1 | ||
| spans, _ = self._run(301, budget_ends_song=True) | ||
| assert len(spans[-1]) == AR_CHUNK_FRAMES + 1 |
There was a problem hiding this comment.
Nice tightening: AR_CHUNK_FRAMES + 1 for both endings, plus the explicit budget_ends_song=True case that hits exactly 201, is the right regression for the deploy-budget arithmetic.
Contributor intent (required human answers)
1. What outcome do you want to achieve?
MiniMax-Music3 runs on Model Runner V2.
2. What is the highest-leverage variable for that outcome?
Whether MiniMax-Music3's performance metrics are stable on V2, and how they differ from V1.
3. How will this work move that variable?
vLLM main may soon drop Model Runner V1 (vllm-project/vllm#63373 removes it), so MiniMax-Music3 needs its V2 port now. This PR does the port and compares V2 with V1 on the same GPU.
4. Whose customer or user needs does this address, and what are those needs?
People who want to serve MiniMax-Music3 with the V2 runner. They need it to run on V2 the way it does on V1.
Purpose
Upstream vLLM plans to remove Model Runner V1 (vllm-project/vllm#63373, an open draft). Both MiniMax-Music3 stages run on V1 today, and the music models are not in the #9387 table. This PR moves both stages to MRV2 on CUDA and makes that the default. Other platforms stay on V1.
minimax_music3.yamlandminimax_music3_2gpu.yamlnow setmodel_runner: v1withplatforms.cuda.model_runner: v2, the same layout asmoss_tts_local.yaml. To run V1 on CUDA, setmodel_runner: v1underplatforms.cudain a copy. If you would rather start with V2 as an opt-in profile, asvoxcpm2_mrv2.yamldoes, I can move it to its own deploy file and leave the defaults on V1.The talker keeps one implementation of row binding, frame feedback, the guided frame draw and window emission for both runners. A new MRV2 model state feeds those hooks from MRV2's batch, and the draw is registered as an
OmniSamplerthroughcustom_sampler, the same route as Higgs Audio v3. The talker's numerics do not change. The async-chunk rules indocs/design/feature/async_chunk.md(a window is never split, and the scheduler stops at the first window that does not fit) are unchanged; they are why stage 1 needs a larger budget on V2.Changes:
minimax_music3/model_state.py(new): the MRV2 model state and sampler.prepare_runner_inputsruns fromprepare_inputs, so it still runs before a FULL decode graph replay. Each row's CFG role, scale and seed come from the sampling params the intermediate buffer keeps per request.include_hidden_states=False, matching the talker'somni_pooler_payload_include_hidden = False, which V1 already honors. Without it, every V2 stage-0 step also published the backbone hidden state under thelatentkey the talker's payload uses. The audio does not change.talker.py: thecreate_mrv2_model_statehook.pipeline.py: both stages declaresupports_native_mrv2_data_plane. The acoustic stage needed no code change.Deploy files, stage 1:
max_num_batched_tokensgoes from 4096 to 16384 (32768 in the 2-GPU file, whose stage 1 has 16 rows). On V1 each window reaches stage 1 as a one-token placeholder prompt; on V2 its codes are the prompt, 200 frames x 8 codebooks = 1600 tokens. At 4096 two requests took every step while the others waited (results below). The XPU override, which stays on V1, keeps 4096.stage_input_processors/minimax_music3.py: on the MRV2 data plane the talker's terminal span is published with the step that samples<|audio_end|>, before the scheduler acknowledges the finish, so a warning fired once per request. The check now also looks at the step's sampled ids. Only the warning changes. The hook also updated this file's SPDX header.async_chunk: false(fifth commit). With it, Music3 returned HTTP 200 with no audio on both runners and on main. In that mode the engine collects stage 0's outputs andar2acoustichands stage 1 the whole song, so:finalize_multimodal, because withinclude_hidden_states=Falsethe runner otherwise builds no per-request payload in this mode.ar2acousticaccepts theMultimodalPayloadthe engine hands it (aMapping, not adict) and reads the seed V1 delivers as a 0-d tensor. Without the seed, stage 1 draws from seed 0 and the song changes.With
async_chunk: truethe audio does not change.Docs: a section in
docs/configuration/stage_configs.md, including the stage-1 budget rule for your own configs (max_num_seqsx 1608 on MRV2: a window is 1600 tokens, and a song's last window can be one frame longer), and a pointer from the recipe. The recipe now describes streaming instead of sayingstream: trueis rejected (sixth commit). markdownlint reformatted the recipe's tables and one nested list once the file was in the diff.Tests: unit tests for the model state, the V2 terminal-span check, the whole-song payload and its handoff in
ar2acoustic; both Music3 deploy files jointest_recommended_native_runner_platform_defaults; the frame-state test now checks the 201-frame bound when the model ends the song and when the frame budget does (sixth commit).Test Plan
MiniMaxAI/MiniMax-Music3at1b525ccb6.minimax_music3.yaml; 2x RTX 4090 with non-standard 48 GB boards withminimax_music3_2gpu.yaml.vllm serve <weights> --omni --deploy-config <file> --trust-remote-code --disable-log-stats. Special runs pass an overlay that changes one value (stage-0max_num_batched_tokens: 64, stage-0gpu_memory_utilization, stage-1 budget, orasync_chunk: false). The/metricsruns use--log-stats./v1/audio/speech, using the recipe's request body withseed,max_new_tokens, lyrics and caption varied, plusstream_format: "audio"orstream: true. Audio is compared by the sha256 of the WAV or PCM bytes. Every server starts fresh; its first request is the 250-frame smoke request, which is counted, and there is no other warmup. The script covers single requests and repeats (250 to 4500 frames, 1 to 201 frames for short songs), streamed PCM, c=1/2/4/8/16 rounds, four streamed songs at once, staggered arrivals, identical requests sent together, aborts and cancels (after the first audio and before the first window), and a 40-song soak.async_chunk: falseruns of the fifth commit and the head, where V1 ran the same commit. The four-server comparison, chunked prefill and the e2e test ran the first commit alone on main 150bcf5, with stage 1 at 4096. The stage-1 budget, KV-pressure and two-GPU runs ran the first two commits on main 8ded596 (the budget server had an earlier revision of the second commit with the same CUDA values). On 8ded596 the third commit ran single requests, streaming, short songs, cancels before the first audio, c=1, 2, 4 and 16,async_chunk: false, the invalid-parameter test and the unit tests. The fourth commit (2f64fbc before the rebase) ran single requests, streaming, the 2250-frame song, short songs, both kinds of cancel and the changed unit tests. The fifth commit (c9ccec8 before the rebase) ranasync_chunk: falseon V2 (single requests, streaming, the 2250- and 4500-frame songs, short songs, c=2) and on V1 (the same without c=2), the shipped file on V2 (single requests, streaming, short songs, aborts and cancels after the first audio), and the changed unit tests. 35c6977 is the five commits rebased onto main e6a7e63 with no patch change (git range-diff). On the head, V2 ran single requests, streaming, the 2250- and 4500-frame songs, short songs, three rounds of four streams, c=2, aborts and both kinds of cancel, the e2ecore_modelcases and the invalid-parameter test, with V1 on main e6a7e63. Both runners ran the head withasync_chunk: false. The Music3 unit tests and the CI CPU groups ran on the head and on main. Every request sent on its own gave the same hash as before the rebase, withasync_chunktrue and false. The sixth commit, after review, corrects the size of a song's last window in the comments, docs and stage-input docstring (at most 201 frames, not 300), updates the recipe's streaming lines and tightens the frame-state test; it changes no runtime code, and the Music3 unit tests and pre-commit pass on it.Test Result
Against the #9387 parity bar:
async_chunk: false(fifth commit and head), V2 and V1 also match for every request sent on its own. Byte-identical audio is a stricter check than WER here; music has no speaker to compare.async_chunk: true, or withasync_chunk: falseon the fifth commit and the head; there were no tracebacks or out-of-memory errors, and peak memory is within 1% of V1.Throughput, one GPU (audio seconds per wall second). c=1 to c=8 and the soak: range over the rounds on two servers per runner (first commit, stage 1 at 4096). c=16: two rounds on one server per runner (third commit).
Stage-1 budget. At 4096 on V2 the scheduler stops at the first window that does not fit, so two streams take every step: with four streams, the first stream's second window waited at least 53 s and the fourth stream about 80 s for its first window. On 8ded596, four rounds of four streams per server:
The lower mean latency at 4096 came from serving two songs at a time while the rest waited. Throughput (c=4 0.81–0.83, c=8 0.87–0.88 audio seconds per wall second) and peak memory (42,929–43,083 MiB) were about the same for V2 at 4096, V2 at 16384 and V1. The stream that started first still waits longer than on V1 (longest gap 45 s against 28 s): its second window shares a 24 s step with the other streams' first windows, where V1 runs it in a 7 s step of its own.
async_chunk: false. Before the fifth commit this returned HTTP 200 with no audio on V1, V2 and main (8ded596). With it, V2 and V1 return the same audio for every request sent on its own: smoke and repeats, the 750-, 2250- and 4500-frame songs, streamed PCM and SSE, and short songs of 1 to 201 frames. Againstasync_chunk: true, songs of up to 201 frames reach stage 1 as one span in both modes and are byte-identical. Longer songs, which here arrive as one span instead of window by window, differ by at most 1 in a 16-bit sample, in about 0.02% of sample frames.Two limits of this mode. A request that reaches
max_model_len(10,240) before its song ends returns no audio; with async chunks it loses only its last window. With the full 9,000-frame budget, that takes a prompt over about 1,240 tokens. I found this by reading the code and did not run it. And stage 0 keeps the songs on the GPU, outside the KV cache, until they end: about 560 MiB per 9,000-frame song, so up to about 4.4 GiB with the 8 songs the one-GPU file runs at once.Concurrent requests: not reproducible run to run on either runner
A song that shares the batch with others depends on the steps at which the others join and leave, so two servers of the same runner also differ. Number of requests whose audio differs in the four-server runs:
The same variants show up on both runners. In the second c=2 round, three servers (one V2, both V1) gave
a2509686andcdd86564, and the other V2 server gavef7e665f0and8069aaf1. Four identical seed-1 requests sent together gave9aa29770twice anddf445917twice on each of the four servers.Single-request hashes
0619ca97afe8e71888a9ba74stream_format: audioand SSE3a675da1bf87fff81a5a37a5b7ee929c1b438636,dc9ad7a5,e27c917fThe four comparison servers and the three stage-1 budget servers returned all of these. With the shipped file, the third to fifth commits and the head returned the same hash for every case they ran. The fourth ran all of them except the 4500-frame song, c=1 and the checks after the soak and the four-stream drop; the fifth also skipped the 2250-frame song; the head ran all but c=1 and the checks after the soak and the four-stream drop. Short songs give the same audio on V2 (third to fifth commits and the head) and V1 (main): 1 frame
eea797eb, 50e97debd9, 199e4209d24, 200269a7a8a, 20162882b63; streamed 50504a5c51and 1991be034d7. Withasync_chunk: false(fifth commit and head, V2 and V1; the head skipped the 4500-frame song): seed 1, 250 and 750 framesc1eeadad,5b2cd624; seed 7, lyrics 2, 750 framesfd5e312b; 2250-frame capb5444348; 4500-frame cap75f5245b; streamed PCM, seed 1 (stream_format: audioand SSE)aac4bb2b, seed 7 lyrics 22b917735; short songs as above. Chunked prefill (stage 0 atmax_num_batched_tokens: 64) gives other hashes, as expected when the prefill is split, but the same ones on V1 and V2 for all 11 single requests.Two GPUs (2x RTX 4090 48 GB)
enforce_eager: true); I did not check which of the two matters.acoustic done ... windows=2). V1 runs a stream's windows a step apart and usually frees it first. In the run that drops two of four streams, the other two songs finished at 73.8 s on V2 and 63.4–66.4 s on V1 (one round); on one GPU, at 98.0 s and 93.5–97.2 s.KV pressure, warnings, e2e and unit tests
gpu_memory_utilization0.45, 0.43 and 0.42, and the solo request gave0619ca97each time. At 0.42 (about 11.1k KV tokens), stage 0 preempted 2 times on V2 and 9 times on V1 (vllm:num_preemptions_total); stage 1 never did. A preemption split a CFG pair once on V2 and 3 times on V1 ("CFG pair ... split (one member preempted)"), after which the cond row continues unguided. That path is the same on both runners and is not changed here.release_request_resources ... TimeoutErrorappears in everyasync_chunk: truerun on both runners (about 100 times in the 112-request runs) and is not changed here. On V2, each request cancelled before its first window logs "MiniMax Music 3 talker produced no audio" at ERROR level, a message main already has; V1 does not log it. The next request still returns the reference audio.tests/e2e/online_serving/test_minimax_music3.py, run directly withMODEL_PREFIX(its marker asks for an H100):core_modelpasses on both runners.advanced_modelfails the same way on both, since whisper-small transcribes the songs as "da da da"; that is [Bug]: weekly CI, tests/e2e/online_serving/test_minimax_music3.py::test_text_to_music_001[minimax_music3], AssertionError: Transcript doesn't match input: similarity=0.00, transcript=' Lalalala' #6842, and [Bugfix] Fix MiniMax Music 3 output validation #6879 removes the check.tests/dfx/reliability/invalid_param_test/test_invalid_audio_speech.py -k minimax_music3: 7 passed on both runners.tests/config/test_config_factory.py(542 passed) andtests/config/test_omni_config.py(326 passed, 1 skipped) pass on the third commit, and the changed Music3 test files pass on the head. The CI CPU groups (core_model and cpu) fail or error on the same test ids on the head and on main e6a7e63 in my environment (25, 10 and 182, none in Music3 code), apart from one diffusion video test that failed only on main.pre-commitpasses.CI: the weekly TTS H100 job and "Reliability Test · Invalid parameters · H100 · Single-GPU" start
minimax_music3.yaml, so after this PR they run V2.Not run: XPU, an H100, and V1 on the 2-GPU file at 32768.
async_chunk: falseon two GPUs, with chunked prefill, with aborts or cancels, or above c=2.🤖 Generated with Claude Code