Skip to content

perf(attention): shard-major Ulysses output all-to-all (wan_ulysses_out_a2a="chunked"/"shard_major") - #511

Open
Perseus14 wants to merge 1 commit into
feat/wan-token-patch-embedfrom
feat/ulysses-shard-major-a2a
Open

Perseus14 wants to merge 1 commit into
feat/wan-token-patch-embedfrom
feat/ulysses-shard-major-a2a

Conversation

@Perseus14

@Perseus14 Perseus14 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on feat/wan-token-patch-embed. Third of four pieces split out of #491. Adds wan_ulysses_out_a2a: "chunked" | "shard_major" (default "flat"): _ulysses_seq_to_heads performs the inverse Ulysses exchange as a single layout-preserving all_to_all over a reshaped (num_shards, chunk) leading axis, so XLA stops inserting the four relayout copies per layer that the flat split_axis=seq form costs at the 720p shard shape (~1.5 ms on v6e, ~0.7 ms on tpu7x). Same data movement, same values. With the custom splash kernel (S_local % block_q == 0, D == 128, heads_per_tile == 1) the forward Q exchange also stays shard-major (4-D q) and the kernel writes the shard-major output directly (out_num_shards), on both the 1D Ulysses path (v6e) and the 2D Ulysses+Ring path (tpu7x): custom_splash_attention.py gains a 4-D q BlockSpec / 4-D output, ring_attention_kernel.py threads out_num_shards through the ring accumulators (save_residuals=True with out_num_shards > 1 raises NotImplementedError), and _compute_fixed_m_metadata accepts the 5-D shard-major query.

Reachability note: The Pallas shard-major Q/out path requires S_local % block_q == 0. At 720p/81f with CP=4 the per-device sequence is 18,900 tokens, which no launcher block_q divides (BQ=9472 on v6e, BQ=6400 on v7 at this PR), so on this PR alone the launcher's chunked setting only changes the inverse all-to-all; the 4-D q / out_num_shards Pallas kernel path engages once #491 (feat/wan-fast-inference-optimizations) pads the sequence to 18,944 (wan_seq_pad=lane) and sets BQ=4736 (4736 × 4 = 18,944).

E2E latency (TPU v6e-8, Wan 2.2 T2V-A14B, 720p / 81 frames / 40 steps, CP=4, DP=2, wan_ulysses_out_a2a="chunked", DVFS unpinned): 128.9 s generate / 126.7 s denoise (29.6 s compile), down from 130.6 s generate / 128.5 s denoise on #510 (−1.7 s generate / −1.8 s denoise end-to-end from the layout-preserving inverse Ulysses all-to-all alone).

Tests: ulysses_out_a2a_test.py (6, new; flat vs chunked vs shard_major parity, mode guard, save_residuals + shard-major guard, bit-exact 4-D ring kernel output, and bit-exact wrapper output on the 1D and 2D paths asserting via spy that out_num_shards == U), wan_runtime_options_test.py (+enum validation).

Stack: #477 → #507 → #478 → #508 → #479 → #488 → #509 → #510 → #511 → #491

@github-actions

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the wan_ulysses_out_a2a configuration option (supporting "flat", "chunked", and "shard_major" modes) to optimize the inverse Ulysses exchange and avoid expensive post-kernel relayout copies on TPUs. This is achieved by updating the custom splash and ring attention kernels to support 4D shard-major query inputs and output sharding, alongside adding a new test suite to verify correctness. The review feedback highlights a critical bug in _ulysses_ring_custom_attention where use_shard_major fails to check for transpose_out, potentially causing layout mismatches. Additionally, the reviewer suggests using the more idiomatic jnp.square instead of ** 2 in JAX operations and wrapping a long line in ring_attention_kernel.py to comply with PEP 8 line length guidelines.

Comment thread src/maxdiffusion/models/attention_flax.py
Comment thread src/maxdiffusion/models/attention_flax.py Outdated
Comment thread src/maxdiffusion/models/attention_flax.py Outdated
Comment thread src/maxdiffusion/kernels/splash_attention/ring_attention_kernel.py
@Perseus14
Perseus14 added this pull request to stack #512 October 15, 2026 16:00
@Perseus14
Perseus14 force-pushed the feat/ulysses-shard-major-a2a branch from 2cabbb8 to f12639a Compare October 15, 2026 16:05
@Perseus14
Perseus14 force-pushed the feat/ulysses-shard-major-a2a branch from f12639a to 86f71b7 Compare October 15, 2026 17:53
…ut_a2a="chunked"/"shard_major")

Third of four pieces split out of the Wan fast-inference optimizations PR.
The switch defaults off ("flat") in the Wan YAMLs and in wan_runtime_options;
it is a plain config key plumbed to FlaxWanAttention / NNXAttentionOp /
AttentionOp and down to the Ulysses kernels through the dispatch context.

- `_ulysses_seq_to_heads` is the inverse Ulysses exchange (attention output,
  head-sharded -> sequence-sharded). "flat" is the previous
  `all_to_all(split_axis=seq, concat_axis=1)`; XLA reorders the whole output
  so the sequence axis is major before the collective and back again after,
  four relayout copies per layer at the 720p shard shape (~1.5 ms on v6e,
  ~0.7 ms on tpu7x). "chunked" / "shard_major" is a single layout-preserving
  all_to_all over a reshaped (num_shards, chunk) leading axis (not chunked
  compute/comm overlap), so no relayout copies are inserted. Same data
  movement, same values.
- With the custom splash kernel (and S_local % block_q == 0, D == 128,
  heads_per_tile == 1) the forward Q exchange also stays shard-major (4-D q,
  `[B, U, H/U, S_local, D]`) and the kernel writes the shard-major output
  directly (`out_num_shards`), on both the 1D Ulysses path (v6e) and the 2D
  Ulysses+Ring path (tpu7x): custom_splash_attention.py gains a 4-D q
  BlockSpec and a 4-D shard-major output for `_splash_attention_forward` and
  `_splash_attention_forward_ring`; ring_attention_kernel.py threads
  `out_num_shards` through `_custom_ring_attention_forward` /
  `make_custom_ring_attention` and sizes the o/l/m accumulators accordingly
  (`save_residuals` with `out_num_shards > 1` raises NotImplementedError:
  the lse contract stays `(num_q_heads, orig_q_seq_len)`).
  `_compute_fixed_m_metadata` accepts the 5-D shard-major query.
- Reachability: the Pallas shard-major Q/out path requires
  S_local % block_q == 0. At 720p/81f with CP=4 the per-device sequence is
  18,900 tokens, which no launcher block_q divides (v6e BQ=9472, v7 BQ=6400
  at this PR), so on this PR alone the launcher's `chunked` only changes the
  inverse all-to-all; the 4-D q / `out_num_shards` kernel path engages once
  feat/wan-fast-inference-optimizations pads the sequence to 18,944
  (`wan_seq_pad=lane`) and sets BQ=4736 (4736 x 4 = 18,944). The rough
  1-1.5 s per 40-step 720p run measured for the kernel path on a pre-review
  revision was therefore taken with that padding active (DVFS state
  unrecorded; not re-measured since).
- run_wan_fast_inference.sh: the v6e and v7 profiles set
  WAN_ULYSSES_OUT_A2A=chunked (overridable); the generic profile keeps flat.

Tests
- ulysses_out_a2a_test.py (6): flat vs chunked vs shard_major parity of the
  inverse exchange, the mode guard, the save_residuals + shard-major guard,
  the 4-D q ring kernel bit-exact vs 3-D (interpret mode off TPU, compiled on
  TPU), and bit-exact wrapper output for the shard-major 1D Ulysses and 2D
  Ulysses+Ring paths vs the flat layout -- both wrapper tests spy on
  make_splash_mha / make_custom_ring_attention and assert out_num_shards == U,
  i.e. that the Pallas shard-major path was actually taken.
- wan_runtime_options_test.py (12): the new switch's YAML default and enum
  validation.
- CI: the three kernel bit-exact tests skip in GitHub Actions.
- end_to_end/tpu/run_wan_stack_tests.sh: adds ulysses_out_a2a_test.py.
Verified (this head): TPU v6e-8 (jax 0.11.2), every file of
end_to_end/tpu/run_wan_stack_tests.sh at this head run directly with unittest:
338 tests, all OK, 1 skipped (cross_attention_pallas_test: off-TPU fallback only);
ulysses_out_a2a_test.py 6 (all 4 multi-device tests ran), wan_runtime_options_test.py 12.
@Perseus14
Perseus14 force-pushed the feat/ulysses-shard-major-a2a branch from 86f71b7 to df7da19 Compare October 15, 2026 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant