Repository navigation
Qwen3-Omni: one grouping for the prefill schedule and the KV layout - #1165
Merged
Merged
Conversation
The schedule and the KV layout each grouped the plan into steps on their own; now both read _thinker_steps, so the layout's opens_step can't drift from the steps that run. The schedule's skips go: process_prompt already indexes every part's tensors at intake, after check_attachments and check_plan, so nothing reaching the conductor has a part to skip. The layout/schedule test now covers every prompt the parity and prefix cache tests use.
next_step ends a step at a change of walk, so such a layout would only show up later as a dropped lookup. Fail it where it's built instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1110, the two points from its review.
Change
The schedule and the layout read one grouping.
_thinker_stepsnow returns the Thinker's prefill steps asThinkerStep(walk, parts): each attachment with the text before it, the last one also with the text after it._build_thinker_prefill_schedulebuilds one entry per step from it, and_kv_layoutsetsopens_stepon the first part of each step. Before, each worked out the grouping on its own, which is how the two-image layout drop in #1110 happened. Now the layout's steps are the steps that run.The schedule's skips go: a part with no tensors,
if entry:, and a text index past the list.process_promptrunscheck_attachmentsandcheck_plan, then_kv_layoutindexes every part's tensors with no skip, and the data worker is the only place a request reaches the conductor from (data_worker._prepare_inputalways handsprocess_prompta tensors dict). So a request with a part to skip fails at intake and never gets here. The schedule can't come out empty either, sinceprefill_planalways ends with a text part, so the empty-schedule fallbacks in_get_thinker_initial_argsgo too._get_thinker_prefill_inputsalready indexedschedule[0]without checking.A layout
next_stepcan't walk fails where it's built.next_steponly extends a step over spans of its own walk, so a span withopens_step=Falseafter a span of another walk would end the step early, and the lookup would drop the layout with only a warning.LabelLayout.row()now raisesValueErrorfor that, next to the span-kind check, naming the span index and both walks (or saying no span comes before it, for span 0). Thenext_step,Spanandadding_models.rstdocs say so. Only Qwen3-Omni sets the mark today, and its layouts all pass.Tests
ruff check .: clean.test_the_layout_says_where_each_step_the_schedule_runs_endsnow covers 15 prompts: every layout the parity and prefix-cache tests use (text, image, image+text, audio, video, text around each, text then image, two images, image text image, image text image text, three images, image video text, image audio text). For each one it checks the walks and span count of every step against a hand-written table, and each span's slots against what the schedule entry's row writes. I checked it against four mutations (TEXT_BEFORE forced to 0, every span its own step, text order reversed, text after an attachment joined to it); each fails 2 to 14 of the 15 cases.test_a_step_that_changes_walk_fails_where_its_layout_is_built(new): text then an image markedopens_step=False, and a first span marked so, both raise; the same layout with one walk passes. It fails on main.test_qwen3_omni_prefix_cache.py,test_kv_prefix_layout_lease.py,test_multimodal_ordering.py,test_qwen3_omni_prompt_parity.py): 155 passed, 2 skipped.select_cpu_tests.pypaths,OMP_NUM_THREADS=2): 11342 passed, 1034 skipped, 10 failed; main at the same base: 11331 passed, 1034 skipped, 10 failed, the same 10 by id (test_send_registration_failure5,test_eval_compare_multigpu2,test_client_error_text,test_qwen3_omni_encoders,test_serving_metrics). The 11 more are the new cases.GPU check
1x B300 (sm_103), torch 2.12.1+cu132,
qwen3omni_colocatedon GPU 0 with-O2and prefix caching on, over/v1/chat/completionsso the parts keep their order. Each prompt sent four times, greedy, on two image sets, after a warm pass on other files. TTFT in ms:Repeats come in 25-30 ms under the first send, as in #1110, and the video prompt stays level, since only its leading text is keyed. No "its chain ends here" line in the whole server log. All four sends gave the same reply in all 8 sets.
I2T at c=16 (Food101, 256 tokens,
ignore_eos, 200 requests after 5 warmup waves, prefix caching on): 1863 tok/s, TTFT p50 202 ms, ITL p50 7.6 ms, 0 failed. That's one server on this branch, not an A/B against main. The CPU check above that the schedule and layouts match main is what says nothing moved.