Repository navigation
Conversation
|
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/diffusion/index.md. Module owners: @david6666666 @wtomin @Isotr0py Routing: @david6666666 via module of the changed files, module named in the PR description, CODEOWNERS; @wtomin via module of the changed files, module named in the PR description, CODEOWNERS; @Isotr0py via module of the changed files, module named in the PR description @Windyskr, 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 routing recordAssigned Strict on cursor (cursor-grok-4.6-high) under experiment |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
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). |
|
Self-review. I read every changed line. Things I checked in particular:
AI assistance: this comment was drafted with Claude Code. |
Omni ReviewBot attempt recordReview attempt ended as failed (Cursor exited with 1: ActionRequiredError: You've hit your usage limit You've saved $3501 on API model usage this month with Ultra. Switch to a different model or set a Spend Limit to continue with Auto. Your usage limits will reset when your monthly cycle ends on 10/20/2026.; retrying direct/cursor). |
Omni ReviewBot attempt recordReview attempt ended as failed (Cursor exited with 1: ActionRequiredError: You've hit your usage limit You've saved $3501 on API model usage this month with Ultra. Switch to a different model or set a Spend Limit to continue with Auto. Your usage limits will reset when your monthly cycle ends on 10/20/2026.; retrying direct/cursor). |
Omni ReviewBot attempt recordReview attempt ended as failed (Cursor exited with 1: ActionRequiredError: You've hit your usage limit You've saved $3501 on API model usage this month with Ultra. Switch to a different model or set a Spend Limit to continue with Auto. Your usage limits will reset when your monthly cycle ends on 10/20/2026.; no fallback: the fallb). |
|
@Windyskr, one merge-readiness TODO for head
Acceptance: GitHub can merge the updated branch cleanly and the relevant regressions pass on that new head. Your existing passing checks and test evidence are retained for the present head; they do not validate a future conflict-resolution commit. Initial readiness triage only; substantive review remains pending. |
|
@Windyskr gentle ping — this high-priority PR has seen no updates for 10 days and review feedback is waiting. Could you share a status or ETA when you get a chance? Thanks! |
|
Thanks for the ping, and sorry for the silence — that one is on me. Status: your 10-16 merge-readiness TODO is what I am working on, and nothing had been pushed since. I have just gone through the conflicts against current
Plan and ETA:
One thing that may be useful on your side: every ReviewBot attempt on 10-11 ended in Happy to split the LingBot paging fix into its own PR if that makes this easier to land; it is self-contained and |
The direct (request-local) path paged the visible K/V with the whole chunk
as one FlashAttention block. At 832x480 a three-frame chunk is 4680 tokens,
which FA2 on SM80 rejects ("Paged KV cache block size must be divisible by
16"). Page at paging_block_size() like the AR runner and pad the last page;
the padding past seq_lens is never read. Chunks that were already legal
pages keep their paging.
Signed-off-by: windyskr <54269955+windyskr@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Self-Forcing distills Wan2.1-T2V-1.3B into a block-causal generator that produces each three-latent-frame block in four DMD steps. The transformer reuses LingBot World's causal attention and K/V paths. The pipeline runs in request mode and in step execution on the AR-Diffusion engine, streaming one block per chunk with stateful VAE decode and mid-stream prompt updates. Adds the stepwise deploy config, registry and metadata entries, model_extras defaults, a checkpoint preparation tool, tests and an A800 recipe. Signed-off-by: windyskr <54269955+windyskr@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4b1f1be to
56b39eb
Compare
|
Done, and the branch now merges cleanly. New head The rebase. 213 commits, six files conflicting. Five were registry and doc entries added on both sides ( The sixth is worth a note: One environment note for anyone reproducing this: Results on # Unit, registry and LingBot regression (CPU + 1 GPU)
pytest tests/diffusion/models/self_forcing tests/model_tests/diffusion/test_alignment.py \
tests/entrypoints/openai_api/test_video_pipeline_capability.py tests/diffusion/models/lingbot_world \
tests/diffusion/ar_diffusion/test_streaming_decode.py tests/config/test_lingbot_world_stepwise_deploy.py \
-m "not parallel"
# -> 393 passed, 3 deselected in 48.68s
# Checkpoint E2E: streamed chunks vs request mode
python tools/prepare_self_forcing_checkpoint.py --output-dir ./self-forcing-dmd
VLLM_OMNI_SELF_FORCING_MODEL_PATH=./self-forcing-dmd pytest tests/e2e/offline_inference/test_self_forcing.py
# -> 1 passed in 61.91s
# LingBot multi-GPU direct/paged after the paging fix
pytest tests/diffusion/models/lingbot_world/test_lingbot_world_transformer.py -m parallel
# -> 1 passed, 2 skipped, 54 deselected in 17.66sThe request/stepwise equivalence test is the one in the E2E run, and it passes on the new head. The two skips in the parallel run are the SP4 and TP2+SP2 cases, which need four free GPUs; at their 256-token frames the paging fix leaves the page size unchanged, as before. The previous figure in the description was 347 passed; the increase to 393 is tests For transparency: the E2E first failed with #8757 stacks on this PR and I will rebase it as soon as this lands. |
Bounty-hunter
left a comment
There was a problem hiding this comment.
Review
Must fix
tools/prepare_self_forcing_checkpoint.py:L75— Please reject an output directory that aliases the local base-model directory before deleting any targets. With--base-model /models/wan --output-dir /models/wan, eachsourceresolves to/models/wan/<component>andtargetnames the same directory; lines 80–83 then unlink/rmtree the source itself.--copysubsequently fails because its source is gone, while the default mode creates a dangling symlink to the deleted path. This can irreversibly destroy the user's tokenizer, text encoder, and VAE. Resolve both roots up front, reject overlap (at minimum equality), and add a regression test that verifies the source tree remains intact.
Verdict: Blocking issue found in the checkpoint assembly path; no formal approval or request-changes action was taken.
|
@Bounty-hunter thank you — this is real, and worse than "fails to assemble". Fixed in I traced it to confirm rather than take it on faith. With The fix compares the resolved roots and refuses before anything is removed: if output_dir.resolve() == base_dir.resolve():
raise ValueError(
f"--output-dir must not be the base model directory ({base_dir}). Assembling in place "
f"would delete the base components this script reads from."
)Resolving both ends also covers an output path that reaches the base directory through a symlink, which has the same effect and would otherwise slip past a plain equality check. Regression tests are in I left nesting alone — |
Omni ReviewBot: supersededThe CI failure noted on |
Bounty-hunter
left a comment
There was a problem hiding this comment.
Incremental review of cc177a113ff1
The previously reported in-place assembly bug is fixed: the resolved-root guard runs before component deletion and the new cases cover literal and symlink aliases.
Must fix
tests/tools/test_prepare_self_forcing_checkpoint.py:L47— Keep these regression tests hermetic instead of invoking the default checkpoint download. Every case callsmain()without--checkpoint;main()therefore reacheshf_hub_download()attools/prepare_self_forcing_checkpoint.py:L69-L72before the new alias guard at line 79. The referencedcheckpoints/self_forcing_dmd.ptis 5,676,252,553 bytes according to the Hugging Face model metadata, so a clean test worker must access the network and download roughly 5.7 GB before testing a local path check. Pass a dummy local--checkpoint(the guard raises beforetorch.load), or stubhf_apiand assert that no download is attempted; preferably move the path guard ahead of checkpoint resolution as well. The current pre-commit run stops earlier because this test file is also missing the required level and hardware marks, so CI has not validated this path.
Verdict: The original data-loss issue is resolved, but the new regression test is not CI-safe yet. No formal approval or request-changes action was taken.
cc177a1 to
812018b
Compare
|
Right on all three, and the first one would have bitten CI hard. Fixed in The ordering was the real bug in my fix. The guard sat after checkpoint resolution, so The tests are offline now. assert no_downloads == [], f"the guard ran after a Hub call: {no_downloads}"That assertion is what keeps the ordering honest, rather than relying on anyone noticing the line order later. Moving the guard back after checkpoint resolution fails all three tests; with it in front, Marks added: I kept the guard to the equality and alias cases. Nesting in either direction gives distinct source and target paths and deletes nothing, so refusing it would reject layouts that work today. |
Bounty-hunter
left a comment
There was a problem hiding this comment.
Incremental review of 812018b1a731
The substantive follow-ups are fixed: the alias guard now runs before checkpoint resolution, the regression tests fail any Hub access and assert no download was attempted, and the required pytest marks are present. I found no new correctness or architecture issue in this increment.
Must fix
tests/tools/test_prepare_self_forcing_checkpoint.py:L65— Please commit the ruff-generated changes so the requiredpre-commitcheck passes. The current head fails because bothruff-checkandruff-formatmodify this file: CI moves therepo_utilsimport below the tool import and reformats the longtest_output_dir_equal_to_base_model_is_refusedsignature. Runningruff format --checkon the frozen file also reproduces1 file would be reformatted. This is mechanical, but the head remains blocked until the generated diff is committed and CI is rerun.
Verdict: The previous data-loss, download-before-guard, offline-isolation, and mark issues are resolved; only the required formatting check remains red. No formal approval or request-changes action was taken.
… model With `--output-dir` naming the same directory as `--base-model`, every target in the component loop resolves to its own source. The loop removes whatever sits at the target before linking or copying, so it deletes the user's tokenizer, text encoder and VAE, and then either fails with the source gone (`--copy`) or leaves a symlink pointing at the deleted path. The base model is not re-downloaded in that case, since the directory still exists, so the loss is unrecoverable. Compare the resolved roots and refuse before anything is removed, which also catches an output path that reaches the base directory through a symlink. The check runs before the checkpoint is resolved, so a bad pair of paths costs nothing rather than a multi-gigabyte download first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: windyskr <54269955+windyskr@users.noreply.github.com>
812018b to
e212639
Compare
|
Committed in The diff was the two things you named: the My mistake was not running the repo's own |
Purpose
Adds Self-Forcing (weights, Apache-2.0): Wan2.1-T2V-1.3B distilled into a block-causal generator that produces each three-latent-frame block in four DMD steps and caches the clean block's K/V for the next one. RFC #6227 lists it as a reference realtime system; at 1.3B on one GPU it is a cheap workload for the stepwise AR-Diffusion path that LingBot World 2.0 uses (#6844).
Closes #8755.
What is added
SelfForcingTransformer3DModel: the LingBot causal Wan block without the camera path. Self- and cross-attention, and both K/V paths (request-local and runner-paged), are imported fromlingbot_world.transformerrather than re-implemented.from_configvalidates the released checkpoint geometry.SelfForcingPipeline:Omni,POST /v1/videos) rolls the clip out against request-local K/V;self_forcing_stepwise.yaml,ARDiffusionEngine) streams one AR block per chunk from the runner's paged session, keeps the text K/V in the session's cross-attention pool, and decodes each chunk with the statefulWanStreamingDecoder;PromptInteractionHandler.Both modes run the same probe/transition/commit code and draw noise in the same order, so streamed chunks are bitwise equal to request mode.
_NO_CACHE_ACCELERATION), post-process, interaction registry, video output metadata, pipeline registry,model_extrasdefaults, stepwise deploy YAML.tools/prepare_self_forcing_checkpoint.py: the release ships onlycheckpoints/self_forcing_dmd.pt, so this assembles a model directory from it plus the Wan2.1-T2V-1.3B-Diffusers text encoder, tokenizer and VAE.recipes/gdhe17/Self-Forcing-A800.md,supported_models.md,diffusion_features.md.Fix in LingBot's direct attention path (needed here)
LingBotSelfAttention's direct (request-local) path paged the visible K/V with the whole chunk as one block (block_size = key.shape[1]). At 832x480 a three-frame chunk is 4680 tokens, and FA2 on SM80 rejects it:Paged KV cache block size must be divisible by 16. It now pages atpaging_block_size(...), the unit the AR runner already uses for 1560-token frames, and pads the last page (never read pastseq_lens). Chunks that were already legal pages keep their old paging. Self-Forcing's request mode at its default geometry depends on this. The tiny-geometry GPU tests intests/diffusion/models/self_forcingfail without it, and LingBot's suites still pass with it (below). I can split this into its own PR if preferred.Design notes
supports_chunk_step_grouping: it saved no time here (81 frames over WS, 10.57 s without vs 10.58 s with) and applies a mid-chunk prompt update one block late ([Bug]: Chunk step grouping applies a mid-chunk interaction one chunk late #8756, fix in [Bugfix] Fix grouped chunk interaction latency #8810).dummy_run_num_frames = 0): its 2-step 512x512 probe is not a valid DMD rollout.guidance_scale > 1, negative prompts,num_inference_stepsother than 4, image inputs, TP/SP/CFG/PP/HSDP/VAE patch parallel. TeaCache and Cache-DiT are not applicable (all four DMD steps are distinct).Not duplicating existing work
Searched open and closed PRs and issues for "Self-Forcing", "self_forcing", "SelfForcing" and "causal Wan2.1 DMD": no Self-Forcing integration exists. Related: #4441 (CausVid draft, predates the AR-Diffusion runtime), #8647 (Matrix-Game 2.0 issue on the same causal Wan 1.3B backbone, no PR yet; this transformer could serve as its base), RFC #6227, RFC #4480.
Test Plan
vLLM Version: 0.31.0 (PyTorch 2.13.0+cu132)
vLLM-Omni Commit: this branch on
b1fc5fcf. Measurements were taken on194b9366; the one commit in between only changes ROCm CI. The full test set was rerun after the rebase.Hardware: 1x NVIDIA A800-SXM4-80GB (SM80), driver 590.48.01, on a shared host. Timings are bounded local measurements.
Reference: guandeh17/Self-Forcing at
33593df, PyTorch 2.9.1+cu128, flash-attn 2.8.3,generator_emaweights, bf16, same GPU. Comparison scripts are in the collapsed section below.Test Result
Tests:
347 passed, 3 deselected(the deselected are LingBot's multi-GPU tests); LingBot SP2 directpassed(relative L2 2.5e-7 vs SP1); SP4 direct and TP2+SP2 paged not run (four free GPUs were not available on the shared host); at their 256-token frames the paging fix leaves the page size unchanged.Accuracy (480x832, 81 frames, bf16)
Free-running rollouts are chaotic at bf16. The reference drifts to 24% latent rel-L2 against itself when FlashAttention is swapped for SDPA, so exact pixel agreement with the reference is not a meaningful bar. The comparison therefore uses per-call parity (tolerance: no worse than the reference's own bf16 error against fp32) plus CLIP score.
Performance, request mode. Each side: 1 warmup + 5 measured runs, same prompt. The reference is timed around
CausalInferencePipeline.inference(GPU tensor out); vLLM-Omni aroundOmni.generate(uint8 frames in the client process).The rollout runs at parity with the reference. Most of the end-to-end gap is the VAE decode (diffusers
AutoencoderKLWanvs the reference's Wan VAE, both bf16, frame-by-frame causal decode).Serving
POST /v1/videos/sync(eager): 81-frame 832x480 MP4 at 16 fps; 10.9 s first request, 10.2 s warm (engine time). Without--enforce-eagerthe first request compiles (~60 s) and warm requests take 8.6 s; compiled output quality was not evaluated.WS /v1/realtime/video, 81 frames: first chunk (9 frames) at 1.80 s; later chunks 1.16-1.74 s each for 12 frames (0.75 s of playout); all 81 frames at 10.57 s, about 0.48x realtime. Eager A800 does not sustain 16 fps playout.AsyncOmni, streamed latents and decoded frames are bitwise equal to request mode for the same seed.video.start: chunk 0 byte-identical to the base run, update active from chunk 1. Visually the scene stays (prompt inertia, see above).Not run: H100/FA3, TP/SP (rejected by the pipeline), quality under torch.compile, multi-session streaming.
The scripts behind these numbers (evidence, not part of the diff) are in this gist.
AI assistance: Claude Code (Claude Opus) wrote the implementation, tests, benchmark scripts, docs and this description under my direction, and ran the tests and measurements above on my A800. I reviewed every changed line.
🤖 Generated with Claude Code