Repository navigation
[Hardware][Intel] Wan VAE: one distributed-encode piece per rank instead of overlapping tile - #8966
Conversation
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/diffusion/index.md, docs/design/module/diffusion/parallelism.md. Module owners: @princepride @david6666666 @Isotr0py Routing: @princepride via module of the changed files, module named in the PR description, CODEOWNERS; @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 @xuechendi, 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. |
|
I have provided the details in PR description. Changes is identified when comparing to lightX2V on vae encode time |
|
Author self-review
Margin 0 is faster but less accurate than tile, which is why the default is 2.
|
|
|
||
| logger = init_logger(__name__) | ||
|
|
||
| ENCODE_SPLIT_ENV = "VLLM_OMNI_WAN_VAE_ENCODE_SPLIT" |
There was a problem hiding this comment.
[P2] Document the new encode controls and quality tradeoff
Both environment variables introduce user-facing behavior, but this PR updates no user documentation. The module docstring also omits the default margin and the reported accuracy degradation at margin zero. Please update the VAE parallelism guide with an invocation, activation requirements, defaults, and approximation limits.
There was a problem hiding this comment.
Thanks — addressed in dc9cc80.
- Added a "Distributed Encode Split (Wan)" section to
docs/user_guide/diffusion/parallelism/vae_parallelism.md. It covers:- invocation: both env vars, plus a serving example;
- when the split applies: a Wan VAE in a pipeline that encodes,
vae_patch_parallel_size > 1intileorspatial_shard_*mode, and an input larger than one tile; - defaults:
tile, margin 2; - approximation limits: neither split is exact, and margin 0 is less accurate than
tile; - the measured speed/accuracy table for 2–4 ranks.
- The module docstring now states the default margin and the margin-0 tradeoff, and links the guide.
572e50f to
dc9cc80
Compare
Omni ReviewBot: three questions on the performance claim@xuechendi 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. |
|
Integration note for head Please add #8968 deliberately stops auto-enabling tiling for This is a cross-PR integration correction, not a defect in the split/crop/merge math. Source review only; the reported B70 encode gains and full I2V output quality were not independently reproduced. |
|
@hsliuustc0106 Thanks, good catch — agreed, with #8968 applied the example would take the local untiled encode and the split would be a no-op. Fixed in 4d21ae4 (
This holds on current |
|
@vllm-omni-review-bot Answers from the B70 run logs:
Evidence
|
Omni ReviewBot routing recordAssigned Strict on claude-code (mimo-v2.6-pro) under experiment |
Omni ReviewBot review statusReview for commit |
Omni ReviewBot: supersededThe CI failure noted on |
4d21ae4 to
6b7b15c
Compare
…ead of overlapping tiles The tile-parallel encode uses diffusers' 256/192 tiles, blended at the seams: 28 tiles at 1280x720, so 1.65x the frame's pixels are encoded, in 3-4 small tiles per rank on 8 ranks. VLLM_OMNI_WAN_VAE_ENCODE_SPLIT=rank instead gives every rank one latent-aligned piece of a grid that matches the rank count (the squarest factorization, e.g. 2x4 for 8 ranks at 720p), widened by VLLM_OMNI_WAN_VAE_ENCODE_MARGIN (default 2) latent px of context on its inner edges that is cropped off again, so the pieces are concatenated, not blended. The default stays "tile". The split lives in its own module, wan_encode_split.py; DistributedAutoencoderKLWan only picks the operator in tiled_encode. test_wan_encode_split.py covers the grid choice, the bounds, the env switches, and an exact split -> merge round trip. The VAE parallelism guide documents both variables: when the split applies, how to set them for serving, the defaults, and the approximation limits. Margin 0 is less accurate than "tile". The guide includes the measured speed/accuracy table for 2-4 ranks. Signed-off-by: Chendi Xue <chendi.xue@intel.com>
…tial_shard_* Spatial-shard modes may not auto-enable VAE tiling (see vllm-project#8968). Without tiling the encode runs locally and VLLM_OMNI_WAN_VAE_ENCODE_SPLIT has no effect, so the serving example and the requirements list now pass --vae-use-tiling explicitly. Signed-off-by: Chendi Xue <chendi.xue@intel.com>
Add VLLM_OMNI_WAN_VAE_ENCODE_SPLIT / VLLM_OMNI_WAN_VAE_ENCODE_MARGIN as model-specific (promote) entries and update the reviewed snapshot counts. Signed-off-by: Chendi Xue <chendi.xue@intel.com>
6b7b15c to
81a1a27
Compare
|
Rebased onto current
|
|
Looks good overall: opt-in via Please rebase onto current Happy to approve after the rebase lands and CI stays green. |
…lit and spatial shard benchmarks/diffusion/bench_wan_vae_spatial_shard.py reproduces the VAE-stage table of this PR: one torchrun launch times tile, tile + rank split (vllm-project#8966), spatial_shard P2P halos and spatial_shard all-gather halos for encode (I2V condition) and decode on the same seeded inputs, records every timed sample (warm-up excluded, each bracketed by a device synchronize and a barrier) and rel_l2 / max_abs against a 1-card untiled encode/decode on rank 0, and writes config, versions and the code revision to --json. Signed-off-by: Chendi Xue <chendi.xue@intel.com>
1. What outcome do you want to achieve?
Lower latency for the multi-card Wan VAE encode in I2V/TI2V serving, without making the encoded latents less accurate than today's tile-parallel encode. It should stay opt-in, so default behavior doesn't change.
2. What is the highest-leverage variable for that outcome?
How much redundant work each rank does in the distributed encode. Today's tile split reuses diffusers' overlapping 256/192 tiles: at 1280×720 that's 28 tiles, so ~1.65× the frame's pixels are encoded. Those tiles are spread unevenly (3–4 per rank on 8 ranks), so the slowest rank sets the latency.
3. How will this work move that variable?
VLLM_OMNI_WAN_VAE_ENCODE_SPLIT=rank gives each rank exactly one latent-aligned piece on the squarest grid for the rank count, with a small context margin (default 2 latent px) that's cropped off afterwards.
4. Whose customer or user needs does this address, and what are those needs?
Users serving Wan2.2 I2V/TI2V on multi-card Intel XPU nodes [e.g. name the team, customer or deployment, if you can share it publicly]. They need lower per-request latency at 720p, where the VAE encode is a fixed cost on every image-to-video request. They also need it without lower output quality, and without changing behavior for anyone who doesn't opt in. [Add the concrete need you have in mind: a latency target, a deployment, or the XPU roadmap item.]
Purpose
Faster and more accurate multi-card Wan VAE encode. It applies to
--vae-patch-parallel-size > 1with pipelines that encode video (I2V, TI2V,video-to-video); T2V does not encode and is unaffected.tile, still the default)VLLM_OMNI_WAN_VAE_ENCODE_SPLIT=rankVLLM_OMNI_WAN_VAE_ENCODE_MARGIN(default 2) latent px of context on inner edgesCode layout.
module,
vllm_omni/diffusion/distributed/autoencoders/wan_encode_split.py.DistributedAutoencoderKLWan.tiled_encodeonly picks the operator.xuechendi:xpu/wan-vae-perf, which edits other parts ofautoencoder_kl_wan.py. A3-way merge of the two over
mainis conflict-free in either order.Accuracy. Neither split is exact against a single-card un-tiled encode: a finite
margin does not cover the encoder's full receptive field, and
tileblends. Theranksplit with margin ≥ 1 is closer to the un-tiled encode than today's default (see
results).
Docs. A new "Distributed Encode Split (Wan)" section in
docs/user_guide/diffusion/parallelism/vae_parallelism.mddocuments both variables:tile);The module docstring states the default margin and the margin-0 tradeoff, and links the
guide.
Test Plan
vLLM Version: v0.31.0 (
vllm/vllm-openai-xpu:v0.31.0base ofdocker/Dockerfile.xpu)vLLM-Omni Commit:
dc9cc80d8onorigin/mainebe2c1827Hardware: Intel Arc Pro B70 (31.9 GiB), torch 2.14.0+xpu, triton-xpu 3.7.2,
diffusers 0.40.0.
even_bounds(contiguous, covering, sizes within one);encoder that is local per 8x8 block.
DistributedAutoencoderKLWan:--vae-parallel-mode spatial_shard_height(the I2V server's setting) on 2, 3 and 4ranks;
tilevsrankwith margins 0/1/2/4;xcclgather.Test Result
1. Unit tests (commit
dc9cc80d8): 21 passed.2. Distributed encode, 720p/81f, against a single-card un-tiled encode:
2 ranks:
tiletile(default)rankmargin 0rankmargin 1rankmargin 2 (default)rankmargin 43 ranks:
tiletile(default)rankmargin 0rankmargin 1rankmargin 2 (default)rankmargin 44 ranks:
tiletile(default)rankmargin 0rankmargin 1rankmargin 2 (default)rankmargin 4than
tile.tileand 1.41–1.46x faster on2–4 ranks.
572e50fe7.dc9cc80d8adds only documentationand a docstring on top of it, rebased onto the latest
main.Pre-commit (local, files in this PR): all hooks pass, including markdownlint.
Not run / not measured:
test node; the encoder measured here is the same Wan2.1-style VAE I2V-A14B uses.