[TRTLLM-12653][feat] LTX-2 Ulysses cross-attention for v2a with audio padding - #14044
Conversation
📝 WalkthroughWalkthroughAdds UlyssesCrossAttention backend wrapper for sequence-parallel cross-attention (S_q ≠ S_kv), integrates key padding mask support into VanillaAttention, introduces audio padding configuration and pipeline in LTXModel to satisfy Ulysses divisibility constraints, wires masking through transformer blocks, and validates correctness with multi-GPU tests for collectives and RoPE equivariance. ChangesUlysses Cross-Attention & Audio Padding
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/_torch/visual_gen/attention_backend/vanilla.py (1)
107-118: ⚡ Quick winConsider using ValueError instead of assertions for validation.
Lines 108 and 109-115 use
assertstatements for input validation. Per coding guidelines (Pydantic section), validation should raiseValueErrorinstead of using assertions, as assertions can be disabled withpython -Oand are intended for debugging rather than input validation in production code.🔒 Suggested change
if key_padding_mask is not None: - assert not is_causal, "key_padding_mask is not supported with causal attention" - assert key_padding_mask.dim() == 2 and key_padding_mask.shape == ( - q.shape[0], - k.shape[2], - ), ( - f"Invalid key_padding_mask shape: expected [B={q.shape[0]}, " - f"S_kv={k.shape[2]}], got {tuple(key_padding_mask.shape)}" - ) + if is_causal: + raise ValueError("key_padding_mask is not supported with causal attention") + if key_padding_mask.dim() != 2 or key_padding_mask.shape != (q.shape[0], k.shape[2]): + raise ValueError( + f"Invalid key_padding_mask shape: expected [B={q.shape[0]}, " + f"S_kv={k.shape[2]}], got {tuple(key_padding_mask.shape)}" + ) # [B, S_kv] -> [B, 1, 1, S_kv] so SDPA broadcasts over H and S_q. attn_mask = key_padding_mask[:, None, None, :] return F.scaled_dot_product_attention(q, k, v, attn_mask=attn_mask, scale=self.scale)As per coding guidelines: "Use
@field_validatorand@model_validatorin Pydantic instead of manualvalidate()methods; raiseValueErrorinstead of assertions"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/_torch/visual_gen/attention_backend/vanilla.py` around lines 107 - 118, Replace the two assert checks used for input validation with explicit ValueError raises so validation cannot be disabled: instead of "assert not is_causal, 'key_padding_mask is not supported with causal attention'" raise ValueError("key_padding_mask is not supported with causal attention") and replace the shape/assertion block that checks key_padding_mask.dim() and key_padding_mask.shape with a single ValueError that includes the same detailed message (use q.shape[0] and k.shape[2] in the message to show expected [B, S_kv] and tuple(key_padding_mask.shape) for actual). Keep the subsequent construction of attn_mask and the call to F.scaled_dot_product_attention unchanged; update only the assertions to ValueError in the code paths referencing key_padding_mask and is_causal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tensorrt_llm/_torch/visual_gen/attention_backend/vanilla.py`:
- Around line 107-118: Replace the two assert checks used for input validation
with explicit ValueError raises so validation cannot be disabled: instead of
"assert not is_causal, 'key_padding_mask is not supported with causal
attention'" raise ValueError("key_padding_mask is not supported with causal
attention") and replace the shape/assertion block that checks
key_padding_mask.dim() and key_padding_mask.shape with a single ValueError that
includes the same detailed message (use q.shape[0] and k.shape[2] in the message
to show expected [B, S_kv] and tuple(key_padding_mask.shape) for actual). Keep
the subsequent construction of attn_mask and the call to
F.scaled_dot_product_attention unchanged; update only the assertions to
ValueError in the code paths referencing key_padding_mask and is_causal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 28647f03-f28c-4881-9bdb-b26f6ff56b35
📒 Files selected for processing (7)
tensorrt_llm/_torch/visual_gen/attention_backend/parallel.pytensorrt_llm/_torch/visual_gen/attention_backend/vanilla.pytensorrt_llm/_torch/visual_gen/config.pytensorrt_llm/_torch/visual_gen/models/ltx2/ltx2_core/transformer_args.pytensorrt_llm/_torch/visual_gen/models/ltx2/transformer_ltx2.pytests/unittest/_torch/visual_gen/multi_gpu/test_ulysses_cross_attention.pytests/unittest/_torch/visual_gen/multi_gpu/test_ulysses_cross_rope_equivariance.py
…dio-sharded The v2a forward used ``_audio_is_sharded`` as a proxy for "Ulysses wrapper is active", which is wrong under Attention2D: ``configure_audio_ulysses`` sets ``_audio_is_sharded=True`` whenever ``use_seq_parallel + seq_parallel_size>1``, including the Attention2D case where ``ulysses_size==1`` and the v2a wrapper was never built. The branch then skipped the all-gather and passed seq-sharded K/V to the plain attention backend, which expects full K/V — silently producing wrong outputs for v2a under Attention2D + audio padding. Add ``LTX2Attention.is_ulysses_active()`` as the getter symmetric with ``set_ulysses_active``, and gate the v2a K/V path on it. Returns False whenever no Ulysses wrapper was built or the module is currently routed to the plain attn, so the AG fallback fires in every case where the plain backend will run. Reported by @yibinl-nvidia on NVIDIA#14044. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
d056233 to
6432850
Compare
…dio-sharded The v2a forward used ``_audio_is_sharded`` as a proxy for "Ulysses wrapper is active", which is wrong under Attention2D: ``configure_audio_ulysses`` sets ``_audio_is_sharded=True`` whenever ``use_seq_parallel + seq_parallel_size>1``, including the Attention2D case where ``ulysses_size==1`` and the v2a wrapper was never built. The branch then skipped the all-gather and passed seq-sharded K/V to the plain attention backend, which expects full K/V — silently producing wrong outputs for v2a under Attention2D + audio padding. Add ``LTX2Attention.is_ulysses_active()`` as the getter symmetric with ``set_ulysses_active``, and gate the v2a K/V path on it. Returns False whenever no Ulysses wrapper was built or the module is currently routed to the plain attn, so the AG fallback fires in every case where the plain backend will run. Reported by @yibinl-nvidia on NVIDIA#14044. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #49918 [ run ] triggered by Bot. Commit: |
|
PR_Github #49918 [ run ] completed with state
|
…dio-sharded The v2a forward used ``_audio_is_sharded`` as a proxy for "Ulysses wrapper is active", which is wrong under Attention2D: ``configure_audio_ulysses`` sets ``_audio_is_sharded=True`` whenever ``use_seq_parallel + seq_parallel_size>1``, including the Attention2D case where ``ulysses_size==1`` and the v2a wrapper was never built. The branch then skipped the all-gather and passed seq-sharded K/V to the plain attention backend, which expects full K/V — silently producing wrong outputs for v2a under Attention2D + audio padding. Add ``LTX2Attention.is_ulysses_active()`` as the getter symmetric with ``set_ulysses_active``, and gate the v2a K/V path on it. Returns False whenever no Ulysses wrapper was built or the module is currently routed to the plain attn, so the AG fallback fires in every case where the plain backend will run. Reported by @yibinl-nvidia on NVIDIA#14044. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
3709672 to
d60dc84
Compare
@NVShreyas Do you mean e2e test testing the output quality or testing the padding correctness for fa4 and vanilla backend? I think for the former one, there is no test yet and for the latter one, I've added two tests. |
3844859 to
d60dc84
Compare
|
/bot run --disable-fail-fast |
|
Added |
yes I meant e2e test with a small input size. It is fine as long as there is some test ensuring output of transformer is consistent with ulysses enabled, for both self and cross attention. |
b82436a to
0530756
Compare
|
added |
|
/bot run --disable-fail-fast |
|
PR_Github #50765 [ run ] triggered by Bot. Commit: |
|
PR_Github #50765 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #50808 [ run ] triggered by Bot. Commit: |
|
PR_Github #50808 [ run ] completed with state
|
Adds a Ulysses sequence-parallel wrapper for the video-to-audio (v2a)
cross-attention in LTX-2. v2a is the asymmetric direction where the
wrapper saves communication: Q is audio (small, T_a~126) and K/V are
video (large, T_v=15360 for 768x1280). Replacing the per-block video
K/V all-gather with a Q+(K|V fused)+output three-collective pattern
cuts per-rank communication volume by ~4x.
The opposite a2v direction (Q=video, K/V=audio) is left on the existing
all-gather path because the wrapper would move two video-sized tensors
(Q a2a + output a2a) in exchange for replacing a cheap audio-K/V
all-gather — net 30x MORE bytes per rank at U=4.
For LTX-2 at 768x1280 / 121f / U=4: T_v = 15360, T_a = 126.
AllGather path (current main for both cross-attns):
2 * (U-1)/U * B * T_kv * D_a * 2
a2v (K/V = audio, T_kv = T_a = 126): ~0.78 MB per rank/block
v2a (K/V = video, T_kv = T_v = 15360): 94 MB per rank/block
UlyssesCrossAttention path (3 a2a):
(U-1)/U^2 * B * (T_q + 2*T_kv + T_q) * D_a * 2
a2v (T_q = T_v, T_kv = T_a): 24 MB per rank/block (30x MORE)
v2a (T_q = T_a, T_kv = T_v): 24 MB per rank/block (~4x LESS)
Break-even: Uly < AG iff (T_q + T_kv) < U * T_kv, equivalently
T_q/T_kv < U - 1. With T_v/T_a ~ 122, the wrapper is only profitable
when the small modality is on the Q side. Hence v2a only.
- `UlyssesCrossAttention` (parallel.py, +107 lines): three-collective
wrapper for S_q != S_kv. Q all_to_all_4d, fused K|V 5D all_to_all,
output all_to_all_4d. Stacks K|V on a new dim of size 2 so both
tensors travel in a single collective. world_size==1 fast path
bypasses all three.
- `audio_pad_for_ulysses` flag (ParallelConfig, default True): when set,
LTXModel pads the audio modality at forward entry so T_a is divisible
by ulysses_size, attaches a [B, T_a_padded] bool validity mask to
TransformerArgs, and strips the padded tail on exit. Required because
T_a is derived from num_frames/fps and is usually NOT divisible
(e.g. 121 video frames at 24fps gives T_a=126; 126%4=2). Pad scheme:
zero-pad latent, repeat-last for positions / per-token timesteps,
repeat-last for the (cos,sin) PE tuple via a seq-dim auto-detect helper.
- `LTX2Attention.use_ulysses_cross`: builds the inner backend at
(H/U, H_kv/U) heads and wraps with UlyssesCrossAttention. Gated on
ulysses_size > 1 (skipped under Attention2D mode which is mutually
exclusive with Ulysses per VisualGenMapping). set_ulysses_active
toggles between the wrapper and the base plain backend at runtime
(used by configure_audio_ulysses and Stage 2 fall-back).
- VanillaAttention gains an optional `key_padding_mask` kwarg (expanded
to [B,1,1,S_kv] as SDPA attn_mask, non-causal only). The
`audio_to_video_attn` call site passes the mask so attention zeros
out audio pad slots. `video_to_audio_attn` does not need a mask
(video K/V is unpadded; pad audio Q is stripped on output).
- `_shard_transformer_args` passes the full-seq audio_padding_mask
through unchanged; the mask is identical across Ulysses ranks.
- test_ulysses_cross_attention.py: wrapper init, forward shape at
world_size in {2, 4}, world_size==1 bit-exact fast path, and parity
vs full SDPA on shared seed inputs.
- test_ulysses_cross_rope_equivariance.py: verifies the
rope-then-a2a == a2a-then-full-rope identity that the wrapper relies
on (since rope is applied locally on sharded inputs in
LTX2Attention.forward and then a2a'd, the per-position rotation must
be preserved by the redistribution).
768x1280, 121 frames, num_inference_steps=40,
3 timed runs after warmup)
Wall clock e2e:
Baseline (no wrapper): 9.497s
v2a wrapper: 9.091s (-0.406s, -4.3%)
NCCL kernel time, per rank, per step (1 denoise step capture via
TLLM_PROFILE_VISUAL_GEN_START_STOP=5-5 + nsys --capture-range=
cudaProfilerApi --cuda-graph-trace=node):
Baseline: ~37 ms NCCL (1640 SendRecv + 1653 AllGather summed
across 8 ranks; per rank ~205 + ~207 kernels)
v2a Uly: ~27 ms NCCL average (528 SendRecv + 196 AllGather per
rank; rank-0 outlier 37.7 ms, rest 24-29 ms)
Per-rank per-step savings: ~10 ms.
40 steps * 10 ms = 400 ms ~ measured 406 ms e2e.
Audio padding cost is negligible at typical T_a (126%4=2 padded to 128;
1.6% padded tokens, masked out in attention).
Disabling the wrapper at runtime is supported via
`parallel.audio_pad_for_ulysses: false` — `_audio_is_sharded` then only
becomes True when T_a%U==0, and the wrapper falls back to the existing
all-gather path otherwise.
Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…change) Addresses post-review feedback on the v2a Ulysses wrapper. Pure refactoring; no functional changes. 1. PE pad moved into prepare_text_cache (done once per generate() instead of per forward step). 2. _pad_pe takes seq_dim explicitly (was fragile auto-detect by shape match); raises on out-of-range seq_dim. 3. Forward audio pad guard reduced from 4 conjuncts to 2. 4. Removed defensive ``if model_config is not None else True`` fallback. 5. PE pad pre-assign + overwrite block removed from forward (subsumed by NVIDIA#1). 6. configure_audio_ulysses two branches flattened to a single formula. 7. v2a fallback restructured to flat if/elif/else (was nested). Plus a stale comment fix: ``Ulysses wrap (a2v in LTX-2)`` → ``(v2a in LTX-2)``. Diff: ~44 insertions, ~57 deletions in transformer_ltx2.py only. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…added K/V Adds a single-rank CPU test verifying the math guarantee that audio K/V padding relies on: VanillaAttention.forward with key_padding_mask masking pad columns produces output bit-identical to running SDPA on the unpadded K/V (max|diff|=0 in the realistic O(1) pad-magnitude regime). Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…attn Drops the bespoke UlyssesCrossAttention class. Its unfused path (Q + K + V independent 4D a2a + output a2a) is functionally equivalent to the existing UlyssesAttention._forward_unfused path, which already handles asymmetric S_q != S_kv via the inner backend's seq_len / seq_len_kv kwargs. Since VanillaAttention.support_fused_qkv() is False, UlyssesAttention auto-selects the unfused path, so v2a now flows through the standard wrapper. The dropped K|V 5D fusion (1 collective vs 2) is a perf-only optimization whose isolated contribution was not measured; if a future profile shows it is worthwhile, add a fused_kv flag to UlyssesAttention as a separate PR. Deletes: - tests/.../multi_gpu/test_ulysses_cross_attention.py - tests/.../multi_gpu/test_ulysses_cross_rope_equivariance.py Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…actor staleness - Drop a copy-paste duplicate of the "Audio padding for Ulysses" comment in LTXModel.forward. - a2v block: drop the "wrapper's three full-video collectives" count, stale after dropping K|V 5D fusion (now 4 collectives), and restore a blank line removed in the original change. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…dio-sharded The v2a forward used ``_audio_is_sharded`` as a proxy for "Ulysses wrapper is active", which is wrong under Attention2D: ``configure_audio_ulysses`` sets ``_audio_is_sharded=True`` whenever ``use_seq_parallel + seq_parallel_size>1``, including the Attention2D case where ``ulysses_size==1`` and the v2a wrapper was never built. The branch then skipped the all-gather and passed seq-sharded K/V to the plain attention backend, which expects full K/V — silently producing wrong outputs for v2a under Attention2D + audio padding. Add ``LTX2Attention.is_ulysses_active()`` as the getter symmetric with ``set_ulysses_active``, and gate the v2a K/V path on it. Returns False whenever no Ulysses wrapper was built or the module is currently routed to the plain attn, so the AG fallback fires in every case where the plain backend will run. Reported by @yibinl-nvidia on NVIDIA#14044. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
… audio_attn1 fallback @yibinl-nvidia flagged that audio_attn1 with audio_pad_for_ulysses=True silently drops key_padding_mask under TRTLLM/FA4 backends, letting valid audio Q attend to padded audio K and corrupting the real audio stream. Two surgical fixes, mirroring the existing cross-attn TRTLLM->VANILLA fallback pattern in modules/attention.py: (1) FA4: route key_padding_mask through FlashAttn4Attention.forward as seqused_k = mask.sum(dim=1).int() (FA4 cute interface accepts per-batch valid lengths on padded [B, S, H, D] input - no varlen repack). LTX-2 audio padding is always a True-prefix; assert and lean on that. (2) TRTLLM audio_attn1: downgrade to VANILLA at construction when audio_pad_for_ulysses=True. Audio self-attn is small (T_a ~ 126), so the backend downgrade is negligible, while it keeps cross-attn (already VANILLA via the existing cross-attn fallback) free to use the padding. v2a cross-attn never receives the mask (caller strips pad Q at LTXModel.forward exit), so it is unaffected. Tested: - tests/.../test_fa4_key_padding_mask.py - 3 cases on a B200, all pass. Self-attn parity, cross-attn parity, pad-fill invariance. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…s mode @NVShreyas flagged that this branch's old guard ("Attention2D keeps ulysses_size == 1 — mutual exclusion") is stale: main now enables ring + ulysses, and attn2d + ulysses is coming soon. Under those combined modes seq_size = cp_size * ulysses_size, so the seq axis is sharded by more than ulysses_size and this wrapper's seq-shape assumptions no longer hold. Add an explicit cp_size == 1 gate so v2a only builds the Ulysses dual-attn pair under pure Ulysses, and falls back to plain + AG everywhere else. Updates the now-stale comment accordingly. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
Comment-only pass across the PR's touched files: - Drop dev-process narrative (verbose "why-we-did-this" repeated in code). - Generic attention backends (vanilla, flash_attn4) no longer name LTX-2 in their docstrings. - Tighten field-storage and configure_audio_ulysses sections that were documenting both the YAML knob and the runtime read paths in prose. - Preserve the non-obvious bits: cp_size gate rationale, TRTLLM→VANILLA fallback motivation, is_ulysses_active() vs _audio_is_sharded gate reasoning, seq_dim disambiguation for SPLIT vs INTERLEAVED rope. No behavior change. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…+ extend mask docstring to FA4 Address @NVShreyas review: - Move the inline `from ...parallel import UlyssesAttention` to the module's top-level imports. - Update the `key_padding_mask` docstring on `LTX2Attention.forward` to mention that FA4 also honors the mask (previously the doc only listed VANILLA), and to point to the audio_attn1 TRTLLM->VANILLA construction-time fallback. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…n + key_padding_mask Address @NVShreyas review (LGTM-with-suggestion): "Might be good to have tests validating output with ulysses enabled for audio." Single-card backend tests (test_vanilla_key_padding_mask.py, test_fa4_key_padding_mask.py) cover the math VanillaAttention / FlashAttn4Attention apply when masking padded K/V columns. They do not, however, exercise the UlyssesAttention wrapper's a2a + sharded SDPA + reverse-a2a pipeline with a key_padding_mask in kwargs - the wrapper mutates kwargs (seq_len, seq_len_kv) and the post-a2a K shape must align with the [B, S] mask. This adds a multi-rank parity test that covers that composition. Test pattern mirrors LTX-2's audio_attn1 path under audio_pad_for_ulysses=True: - Audio Q=K=V padded so S_padded is divisible by ulysses_size. - key_padding_mask = True on [0, S_real), False on the pad tail. - UlyssesAttention(VanillaAttention) wrapper; each rank holds [B, S_padded/U, H, D] of the padded sequence. - Reference: plain SDPA on the unpadded [0, S_real) prefix. - Assert: valid Q rows in this rank's shard match the reference within bf16-like tolerance. ws=2 (one sharding boundary), CPU + gloo - runs in pre-merge CI without GPUs. Forces device="cpu" to avoid cuda:rank ordinal errors when fewer GPUs are visible than spawned ranks. Tested locally in the dev container: 1 passed in 44.01s. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…s=2)
End-to-end multi-GPU test that runs ``LTXModel.forward`` on a 2-layer
AudioVideo mini-config with ``ulysses_size=2`` and compares to the
single-GPU reference (same weights, same inputs). One test logic
function covers self-attn (audio_attn1), a2v cross-attn, and the v2a
Ulysses wrapper simultaneously.
Cases (parametrized over VANILLA + FA4 backends, ws=2):
- test_av_ulysses_no_audio_pad: T_a divisible by U, no padding.
- test_av_ulysses_audio_pad: T_a NOT divisible by U; exercises
audio_pad_for_ulysses + the
key_padding_mask thread through
audio_attn1 + a2v.
head_dim=32 keeps the test runnable in dev images that don't have the
LTX-2 fused norm+rope C++ extension built (``fused_dit_split_norm``);
the v2a Ulysses + key_padding_mask code paths under VANILLA / FA4 are
unchanged by that choice. cross_attention_dim is set to inner_dim
(=num_heads * head_dim) because the LTX-2 caption_projection projects
caption_channels → inner_dim and feeds straight into
``block.attn2.to_k`` whose input dim is ``cross_attention_dim``.
Tolerance is rtol=atol=5e-2: measured BF16 drift through 2 transformer
layers + Ulysses collectives is ~1.5% on the failing rank, well within
the threshold, and tight enough to catch real numerical regressions.
Verified locally: 4/4 PASSED in 73.7s (2 × B200).
Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
Rebase-conflict-resolution introduced two over-wrapped expressions in transformer_ltx2.py that the project's ruff-format pre-commit hook wants on one line. No behavior change. - LTX2Attention.forward fused-fallback call to _forward_unfused. - BasicAVTransformerBlock.forward v2a AG-fallback gate condition. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
…ysses Per reviewer feedback, remove the ``parallel.audio_pad_for_ulysses`` ParallelConfig knob and derive audio padding behavior internally based on the runtime context. With Ulysses active, audio is always padded to make ``T_a`` divisible by ``ulysses_size`` and a ``[B, T_a_padded]`` validity mask is attached so attention zeros pad slots; with Ulysses inactive nothing changes. The opt-out mode the knob exposed was a silent performance downgrade (disengaged the v2a Ulysses wrapper, fell back to plain attention on non-divisible T_a) and not something users should be deciding. Source changes (all behavior-equivalent to the previous True default): * Drop ``ParallelConfig.audio_pad_for_ulysses``. * ``LTX2Attention._init_audio_modules``: TRTLLM→VANILLA downgrade for ``audio_attn1`` now keys on ``vgm.ulysses_size > 1`` (the condition that actually drives the mask requirement) instead of the dropped flag. * ``LTXModel.__init__``: drop the cached ``self._audio_pad_for_ulysses``. * ``LTXModel.configure_audio_ulysses``: always pad when the sharder is active. Removes the dead "no-pad mode" branch and the CP-without- Ulysses ``ValueError`` (now unreachable because padding always makes audio shardable). * Docstrings + the ``audio_padding_mask`` field comment updated. Tests adjusted: drop the parameter from the three call sites in test_ltx2_ulysses.py / test_ulysses_attention.py / test_fa4_key_padding_mask.py. Verified locally with the multi-GPU e2e LTXModel parity test (VANILLA backend, ws=2): both ``test_av_ulysses_no_audio_pad`` and ``test_av_ulysses_audio_pad`` PASS. The FA4 backend cases failed in this container due to an unrelated env regression already fixed on main (PR NVIDIA#13788, ``nvvm.fmax`` API change in ``nvidia_cutlass_dsl``); CI on a fresh container picks up the fix automatically. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
693348b to
7831a01
Compare
…class Reviewer suggested using ``enable_ulysses`` (PR NVIDIA#14012) to simplify our PR's hand-rolled v2a Ulysses wrapper. Two surgical changes that together delete the manual wrapper construction + the parallel ``use_ulysses_cross`` ctor flag. 1. Base ``Attention.__init__`` wrap dispatch (modules/attention.py): The combined gate ``(ring_size > 1 or ulysses_size > 1) and qkv_mode != SEPARATE_QKV`` excluded cross-attn from both Ring and Ulysses wrapping. Split it into two single-purpose gates: * Ring stays gated on ``qkv_mode != SEPARATE_QKV`` — Ring shards the seq dim and requires ``S_q == S_kv``. * Ulysses now keys on ``use_ulysses`` only — Ulysses shards the head dim and is value-preserving for ``S_q != S_kv``, so cross-attn is valid. Also fixes a latent bug where ``enable_ulysses=False`` would still build the wrapper while making the inner backend use the unsharded head count. 2. LTX2Attention (models/ltx2/transformer_ltx2.py): * Drop the ``use_ulysses_cross`` ctor flag. Caller passes a single ``use_ulysses=True`` for both self-attn and cross-attn modules. Internally, cross-attn carries an additional ``cp_size == 1`` gate (Ring/Attention2D + cross-attn Ulysses is unvalidated). * Delete the 30-line ``_ulysses_cross_attn = UlyssesAttention(...)`` hand wrap that bypassed the base class. With Step 1 the base class now builds the wrap for cross-attn too. * Collapse the cross-attn dual-attn pair into the existing self-attn dual-attn path (single ``_ulysses_attn`` / ``_plain_attn`` pair, single ``_has_dual_attn`` flag, single branch in ``set_ulysses_active`` / ``is_ulysses_active``). * Rename the locally derived flag from ``wants_ulysses`` to ``enable_ulysses`` so it matches the kwarg passed to the base class. Net: 94→58 lines in transformer_ltx2.py ctor + 30 fewer lines in toggle methods. ``use_ulysses_cross`` call site at ``video_to_audio_attn = LTX2Attention(..., use_ulysses=True)``. Verified: all 4 PR-touched test files PASS (26/26 in 6m57s) — e2e LTXModel parity (VANILLA + FA4, no_pad + audio_pad), multi-rank UlyssesAttention parity, FA4/Vanilla key_padding_mask single-rank. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #50968 [ run ] triggered by Bot. Commit: |
|
PR_Github #50968 [ run ] completed with state |
NVIDIA#14044 added Ulysses sequence-parallelism for the v2a (video->audio) cross-attention; NVIDIA#13978 (async-Ulysses refactor) silently disabled it, so v2a fell back to all-gathering the full video K/V every block (per-rank comm grows with ulysses_size instead of scaling). Restore the v2a Ulysses path under pure Ulysses (cp_size == 1), preserving NVIDIA#13978's async-Ulysses (self-attn only) and the cp_size > 1 all-gather fallback: - modules/attention.py: head-shard the SEPARATE_QKV cross-attn inner backend when cp_size == 1. - transformer_ltx2.py: build the dual-attn pair for cross-attn under pure Ulysses and pass use_ulysses=True to video_to_audio_attn. Output is numerically unchanged; this is a communication/perf fix. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
NVIDIA#14044 added Ulysses sequence-parallelism for the v2a (video->audio) cross-attention; NVIDIA#13978 (async-Ulysses refactor) silently disabled it, so v2a fell back to all-gathering the full video K/V every block (per-rank comm grows with ulysses_size instead of scaling). Restore the v2a Ulysses path under pure Ulysses (cp_size == 1), preserving NVIDIA#13978's async-Ulysses (self-attn only) and the cp_size > 1 all-gather fallback: - modules/attention.py: head-shard the SEPARATE_QKV cross-attn inner backend when cp_size == 1. - transformer_ltx2.py: build the dual-attn pair for cross-attn under pure Ulysses and pass use_ulysses=True to video_to_audio_attn. Output is numerically unchanged; this is a communication/perf fix. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
NVIDIA#14044 added Ulysses sequence-parallelism for the v2a (video->audio) cross-attention; NVIDIA#13978 (async-Ulysses refactor) silently disabled it, so v2a fell back to all-gathering the full video K/V every block (per-rank comm grows with ulysses_size instead of scaling). Restore the v2a Ulysses path under pure Ulysses (cp_size == 1), preserving NVIDIA#13978's async-Ulysses (self-attn only) and the cp_size > 1 all-gather fallback: - modules/attention.py: head-shard the SEPARATE_QKV cross-attn inner backend when cp_size == 1. - transformer_ltx2.py: build the dual-attn pair for cross-attn under pure Ulysses and pass use_ulysses=True to video_to_audio_attn. Output is numerically unchanged; this is a communication/perf fix. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
Description
This PR adds Ulysses sequence-parallelism support for the v2a (video-to-audio) cross-attention in LTX-2 by reusing the existing
UlyssesAttentionwrapper. It replaces the all-gather (AG) path on the heavy video K/V side with the Ulysses 4-collective (Q + K + V + output) all-to-all, which is significantly cheaper whenT_q << T_kv. The audio padding mode, FA4key_padding_masksupport, audio self-attn backend fallback, and the AG-fallback gating fix are all in service of making the Ulysses v2a path safe across all supported LTX-2 configurations.Why v2a, not a2v?
LTX-2 has two AV cross-attentions per block:
audio_to_video_attn(a2v)T_v ≈ 6300at 768×1280 / 121 frames)T_a ≈ 126, padded to 128 underaudio_pad_for_ulysses=True)video_to_audio_attn(v2a)T_a)T_v)Per-rank communication volume (cross-attn K/V path),
U = ulysses_size2 × (U-1)/U × T_kv × H × D × 2(U-1)/U² × 2 × (T_q + T_kv) × H × D × 2Setting
Ulysses < AGand simplifying gives the break-even:Plug in LTX-2 with
U = 4:T_q / T_kvU − 1 = 3Concrete byte numbers (LTX-2 v2a:
H × D = 32 × 128,U = 4, bf16, per v2a layer per step, per rank):T_v = 6300)2 × 0.75 × 6300 × 32 × 128 × 2 B0.75 × (128 + 6300) × 32 × 128 × 2 B2 × 0.75 × 128 × 32 × 128 × 2 BOPT ships 27% of baseline for the v2a/a2v cross-attn comm budget — a theoretical −73% byte volume per layer per step. This is the upper bound on wall-time savings; per-kernel launch overhead (OPT has roughly 2× the kernel count) is what eats into the realized win.
Hence only v2a gets the wrapper; a2v stays on AG. Hardcoded in
BasicAVTransformerBlock._init_av_cross_modulesviause_ulysses_cross=Trueonvideo_to_audio_attnonly.Performance
NCCL kernel time, per rank, single denoise step (
step 5capture, 4× B200, CFG=1 × Ulysses=4, 768×1280, 121 frames)Captured with nsys
--capture-range=cudaProfilerApi --cuda-graph-trace=node,TLLM_PROFILE_VISUAL_GEN_START_STOP=5-5:audio_pad_for_ulysses=false, AG fallback)audio_pad_for_ulysses=true, v2a wrapper)AG time drops 23.4 msbecause the dominant cost item (gathering full video K/V) is gone — the remaining AGs ship only the small audio K/V (a2v) and avg per-op cost falls 8× (138 µs → 17 µs).SR time rises 14 msbecause the 4 a2a collectives in the v2a wrapper are realized asncclDevKernel_SendRecvpairs (U-1 SR ops per a2a per rank). The net 9.3 ms / step / rank saved is well below the 73% theoretical byte savings — launch overhead from going from 385 to 770 kernel calls eats the rest.End-to-end wall time (8× B200, CFG=2 × Ulysses=4, 768×1280, 121 frames, 40 steps)
audio_pad_for_ulysses=false, AG fallback)audio_pad_for_ulysses=true, v2a wrapper engaged)Same prompt / seed / steps; output MP4s are visually identical (sample artifacts:
tmp/ulysses-a2v/ltx2_{baseline,opt}_refactor.mp4).Implementation
LTX2Attention.__init__builds a dual-attn pair onvideo_to_audio_attnonly — both aUlyssesAttention(inner_backend=...)wrapper and the base-class plain attention — and togglesself.attnbetween them at runtime viaset_ulysses_active(bool). Symmetric getteris_ulysses_active()lets the block forward gate the AG fallback correctly under Attention2D / ring (where_audio_is_shardedcan be true without a Ulysses wrapper being built).use_ulysses_cross=True ∧ ulysses_size > 1 ∧ cp_size == 1(pure Ulysses). Combined modes (ring+Ulysses, attn2d+Ulysses) shard the seq axis byseq_size = cp_size × ulysses_size, which this wrapper has not been validated against — they fall through to the existing AG path.configure_audio_ulysses(audio_seq_len)runs once per request: whenparallel.audio_pad_for_ulysses=True(default) it pads audio to a multiple ofulysses_sizeso the wrapper can always engage; otherwise it falls back to the natural-divisibility check.LTXModel.forwardpads audio (zero-pad latent, repeat-last positions/timesteps) on entry and strips the padded tail on exit.prepare_text_cachepre-pads audio PE so the denoise hot path sees consistent shapes. A[B, T_a_padded]validity mask travels throughTransformerArgs.audio_padding_maskand is consumed byaudio_attn1andaudio_to_video_attn.Backend safety for
key_padding_maskPadded audio means valid Q tokens would attend to padded K positions unless the backend honors
key_padding_mask:key_padding_mask→ SDPAattn_mask)key_padding_mask→ cuteseqused_k)**kwargsaudio_attn1overrides backend to VANILLA whenaudio_pad_for_ulysses=True(mirrors the cross-attn TRTLLM→VANILLA fallback inmodules/attention.py; audio self-attn is small atT_a ≈ 126so the backend downgrade is negligible)v2a itself never receives the mask — padded audio Q rows are stripped on
LTXModel.forwardexit, so their attention output is discarded regardless.Test Coverage
tests/unittest/_torch/visual_gen/test_vanilla_key_padding_mask.py— single-rank parity forVanillaAttention.forward(key_padding_mask=...)vs unpadded SDPA.tests/unittest/_torch/visual_gen/test_fa4_key_padding_mask.py—FlashAttn4Attention.forward(key_padding_mask=...)on a B200: self-attn parity, cross-attn parity, and pad-fill invariance.PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.