Repository navigation
[Feature][SenseNova-U1.5] Resumable prepare phase for step execution - #7730
MrlixiangWE wants to merge 16 commits into
Conversation
|
This PR appears to belong to: docs/design/module/diffusion/offloader.md, docs/design/module/diffusion/index.md. Module owners: @david6666666 @Isotr0py @princepride Routing: @david6666666 via module of the changed files, module named in the PR description; @Isotr0py via module of the changed files, module named in the PR description; @princepride via module of the changed files, module named in the PR description @MrlixiangWE, 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@MrlixiangWE 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. |
8e016ed to
ae93726
Compare
MrlixiangWE
changed the title
6acc0b0 to
0541926
Compare
|
Three things that could break silently here, and what each is pinned by. A request whose decode loop is already over when it arrives. The two stop rules, which are not interchangeable: A second sequence reaching the model-local paged cache, which would read another request's prefix instead of failing. Removing the runner's three-line prepare hook turns exactly three tests red and leaves the rest green. |
MrlixiangWE
left a comment
MrlixiangWE
left a comment
There was a problem hiding this comment.
Self-review pass over this head. The first item is a reproduced crash that fails every request in the tick.
0541926 to
e78e29a
Compare
|
All fixed in 8c971ea. The crash is gone: the decode that finishes a request inside the prepare phase now runs whichever call ended the phase, so a zero-token text request no longer reaches The image path now runs through the runner — |
Step execution calls `prepare_encode()` once per request, so SenseNova-U1.5's think decode runs entirely inside that one call and every other scheduled request waits on it. `SupportsResumablePrepare` is an optional companion to `SupportsStepExecution`: `prepare_step(state)` advances the prepare phase by one step and `prepare_steps_remaining(state)` reports what is left, with `None` or `0` both meaning done. `DiffusionModelRunner` runs one prepare step per scheduler tick for each request still preparing and leaves it out of that tick's denoise batch; the step that ends the phase puts it back into the same tick. A request whose prepare phase produces the whole output is decoded and finished there, whichever call ended the phase — including `prepare_encode()` itself, which is how a zero-token text request behaves. Every scheduled request reaches the failure all-reduce the same number of times, so the collective count does not depend on pipeline-private state. SenseNovaU1Pipeline implements both. The two decode loops keep their bodies, including the stop rules that differ between them, and are split into single-token steps that request mode calls in a loop. Text sampling draws from the request's own generator, so an unseeded request keeps drawing from the global RNG. Step execution requires max_num_seqs=1, because `paged_decode.py` holds one sequence. Signed-off-by: MrlixiangWE <mrdanaer@gmail.com> Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com>
e78e29a to
8c971ea
Compare
Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Omni ReviewBot: no human activity for 7 days@MrlixiangWE this pull request has had no human commit, comment or review since 2026-09-26. 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. |
Preserve the component quantization and MoE loading paths while keeping the resumable prepare protocol and its profiler entry points. Execute preparation inside the runner forward context so model calls receive the current configuration before denoising begins. Cover native context propagation and restoration through all three prepare stages, and profile both request and step denoising after normal pipeline construction. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Retire request-local runner, batch and LoRA state at wave boundaries so cancelling the last request does not depend on another request arriving. Clean normal terminals and the complete Worker failure boundary, preserve startup profiling and keep native KV draining under its existing owner. Align stage-payload/config types and the SenseNova graph/seed recipe. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Omni ReviewBot triage noteResolved as of |
|
@hsliuustc0106 Ready for review. |
Omni ReviewBot routing recordAssigned Strict on cursor (cursor-grok-4.6-high) under experiment |
Review follow-up: the continuous-batching design doc still said the pipeline rejects max_num_seqs>1 at startup; describe the reviewed behavior instead — batched denoise waves with serialized think/text decode, and CFG on/off requests separated at admission. Qualify the recipe's step-execution measurements: the saved images and timing/ abort numbers predate the restack onto vllm-project#7730, note which contracts the test suite re-pins on the current stack, and state the waves-group-by- CFG-switch behavior in the serving section.
Review follow-up: the continuous-batching design doc still said the pipeline rejects max_num_seqs>1 at startup; describe the reviewed behavior instead — batched denoise waves with serialized think/text decode, and CFG on/off requests separated at admission. Qualify the recipe's step-execution measurements: the saved images and timing/ abort numbers predate the restack onto vllm-project#7730, note which contracts the test suite re-pins on the current stack, and state the waves-group-by- CFG-switch behavior in the serving section. Signed-off-by: shihongzhi <shi65881583@gmail.com>
Omni ReviewBot attempt recordReview attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 120s (try 2 of 3)). |
Omni ReviewBot attempt recordReview attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 600s (try 3 of 3)). |
Omni ReviewBot attempt recordReview attempt ended as failed (failed; falling back to direct/cursor/auto). |
vllm-omni-review-bot
left a comment
vllm-omni-review-bot
left a comment
There was a problem hiding this comment.
Omni ReviewBot review
Changes since the previous review
- 0 new inline finding(s); 0 finding(s) below.
CI at
e6e9f6f9b47e(2026-10-10T13:37:54.430801+00:00): required check(s) blocking:buildkite/vllm-omni(missing).
Note: The assigned review arm
strict/cursor/cursor-grok-4.6-highcould not complete this review, so it was produced by the fallback armdirect/cursor/auto. It is excluded from the routing experiment.
Full review analysis
PR description
SenseNova think and text decode no longer run to completion inside one prepare_encode() call. The runner can now advance an optional resumable prepare phase by one token per scheduler tick, then either admit the request to that tick's denoise batch or, for a text request that never sets timesteps, finish it with post_decode(). Request mode and step mode share the same stop rules and seed-aware sampling; step mode still allows only one sequence because the model-local paged cache is unchanged. Finished, failed, and cancelled step requests drop runner state, the cached batch, and Worker LoRA identities at a wave boundary, while native KV drain stays on its existing path.
Change flow
flowchart LR
A["[EXISTING] Scheduler tick"]:::existing --> B["[CHANGED] Runner prepare and denoise"]:::changed
B --> C["[NEW] SupportsResumablePrepare"]:::new
C --> D["[CHANGED] SenseNova one-token decode"]:::changed
D --> E["[CHANGED] Denoise batch or text finish"]:::changed
E --> F["[CHANGED] Worker state and LoRA release"]:::changed
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
🤖 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!
Omni ReviewBot: no human activity for 13 days@MrlixiangWE this pull request has had no human commit, comment or review since 2026-10-04. 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. |
Take main's step-state retirement (vllm-project#8719): the runner, worker and engine release_step_requests come from main, and this branch's executor method, duplicate definitions and profile-skip path are dropped, with the tests that exercised them. vllm-project#8254 made _denoise take use_cfg and moved the CFG step mask, vision grid and noise-scale embedding out of the step body. _denoise_one now reads them from a per-request plan built once by _denoise_plan: before the request-mode loop, and in _finish_prepare for step execution. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Main now releases a completed step request and its batch inside _update_states_after. A one-step profile request therefore leaves input_batch empty, and the step-mode profile_run, which checked that attribute to confirm a denoise step ran, raised at startup for every step-execution pipeline under the paged scheduler. Record that a denoise batch was built in a latch reset per profile run, and read the latch instead. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
…etirement hook Main's runner now calls pipeline.release_step_request(state) for every retired step state (vllm-project#8859). SenseNova implements it: drop the request's step context, including a prepare-phase decode cursor, and clear the flash KV buffers of its denoise caches when an abort or failure retires the request before post_decode. Requests that retire inside the resumable prepare phase, after a prepare failure or with a prepare-only output, now go through release_step_requests as well, so the pipeline sees the same callback as for a denoised request. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Main's check-mark hook rejects a bare platform mark and a CPU mark shared with an accelerator test. The module-level CPU mark becomes a per-class mark, and the one CUDA test becomes a module function marked only through hardware_test. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Run request mode and step mode over the same schedule with the real embeddings and the real _denoise, recording only the transformer branches. Every t2i/it2i CFG configuration takes the same branches at the same steps in both modes and produces the same image; step mode builds the noise-scale embedding once and does no host synchronization or grid rebuild inside a step. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
…se failures through the hook Startup memory profiling only runs under the paged scheduler, whose profile request needs Diffusion KV requests that no resumable pipeline produces, so the prepare-wave loop, its build latch and its verdict could not be reached by SenseNova and only changed main's behavior. profile_run is main's again. A per-request failure in the denoise phase now retires through release_step_requests, so the pipeline's release_step_request runs for it as it does for completed and aborted requests. The resumable-prepare tests stub the platform's peak-memory queries so they do not depend on the host's GPUs. A prepare-loop comment no longer claims rank lockstep. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Edit requests run CFG when cfg_scale or img_cfg_scale differs from 1, so cfg_scale > 1 misreported them; set the flag from the plan's CFG steps when prepare finishes. The real-_denoise test covers an edit with cfg_scale 1 and img_cfg_scale 2. The profiler entry-point test moves next to the other step-execution tests, and a runner test comment no longer describes an assertion it does not make. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Step execution was run on SenseNova-U1.5 only, so SenseNova-U1 and A3B are marked not verified and the prose names U1.5. The U1 recipe's profiler stage names follow the current targets and its note reference follows the renumbered features note; the U1.5 maintainer line is left as on main. Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
|
@hsliuustc0106 i have resove all conflect. could you help in review and merge? |
Co-authored-by: zijianc2 <157244773+zijianc2@users.noreply.github.com> Signed-off-by: MrlixiangWE <mrdanaer@gmail.com>
Purpose
Part of #7679. SenseNova's think decode currently runs entirely inside
prepare_encode(), so step execution cannot return to the scheduler between tokens.Propose
SupportsResumablePrepare:prepare_step(state)advances preparation once, andprepare_steps_remaining(state)reports its remaining work.max_num_seqs=1because the current identity block table holds one sequence.generator_device="cpu". With no seed, sampling remains stochastic.release_step_requests([Bugfix] Finalize step requests before engine idle #8719). SenseNova implementsrelease_step_request(state)([Core] Introduce a generic cross-attention abstraction with LingBot integration #8859): it drops the step context, including a prepare-phase decode cursor, and clears the flash KV of the denoise caches. Requests that retire inside the prepare phase, and denoise-phase per-request failures, also go throughrelease_step_requests._denoise(..., use_cfg)runs per step with no host synchronization.do_true_cfgfollows the same CFG mask, so edit requests withcfg_scale=1andimg_cfg_scale!=1report CFG.Change summary
Test Plan
63803damerged with main15bce34; vLLM 0.32.0, PyTorch 2.13.0+cu130, 2x NVIDIA A800 80GB.pytest tests/entrypoints tests/engine tests/diffusion --ignore=tests/diffusion/models -m 'core_model and cpu', this branch and main.pytest tests/diffusion/models/sensenova_u1 -m 'core_model and cuda' --run-level core_model.max_tokens64 (48 for sampled text), one request at a time. Three repetitions per process, two processes per arm in opposite order on 1x A800; TP=2 and CFG-parallel 2 on both GPUs.Test Result
15bce34sensenova_u1Output digests (MD5 of PNG bytes, SHA-256 prefix of generated text), six repetitions per arm at
a0e18c3; at63803darequest and step on 1x A800 give the same digests (three repetitions):7e4ec437e4ec437e4ec43e598938/e5989387e4ec43/7e4ec4343531b243531b243531b2df3aa27/df3aa2743531b2/43531b2ea46341ea46341ea4634148cb4a7/48cb4a7ea46341/ea463415c9b35b5c9b35b5c9b35b2db6c3e/2db6c3e5c9b35b/5c9b35b05b95cc05b95cc05b95cc8591ff8/8591ff805b95cc/05b95cc0d0cf230d0cf230d0cf230d0cf23/0d0cf230d0cf23/0d0cf2310ca5da10ca5da10ca5dae8267c9/e8267c910ca5da/10ca5da8fdbc968fdbc968fdbc96/8fdbc968fdbc96/8fdbc968fdbc968fdbc968fdbc96/8fdbc968fdbc96/8fdbc96Median latency of six repetitions at
a0e18c3: