[None][fix] LTX-2: re-enable Ulysses for v2a cross-attention - #15303
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR updates Ulysses sequence parallelism wiring across visual generation attention modules. The base Attention module extends its Ulysses gating logic to account for context parallelism size, while LTX2Attention broadens its dual-attention pair creation for cross-attention and enables Ulysses for audio-visual cross-attention paths. ChangesUlysses Attention Configuration
Possibly related PRs
Suggested reviewers
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
|
/bot run |
|
PR_Github #53857 [ run ] triggered by Bot. Commit: |
NVShreyas
left a comment
There was a problem hiding this comment.
LGTM after requested change
|
PR_Github #53857 [ run ] completed with state
|
…es + log) Address NVShreyas's review on PR NVIDIA#15303: - wrap_parallel_attention now takes a use_ulysses argument and gates the UlyssesAttention wrap on it, so the outer wrap stays consistent with the inner backend's head-count decision (avoids a full-head backend being wrapped in Ulysses). The single use_ulysses flag now drives both the inner head count and the wrap. - Add a debug log when Ulysses is disabled despite ulysses_size>1 and sequence-parallel on (a SEPARATE_QKV cross-attn that is neither async nor pure-Ulysses, cp_size>1), so the all-gather fallback is observable. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #54227 [ run ] triggered by Bot. Commit: |
…es + log) Address NVShreyas's review on PR NVIDIA#15303: - wrap_parallel_attention now takes a use_ulysses argument and gates the UlyssesAttention wrap on it, so the outer wrap stays consistent with the inner backend's head-count decision (avoids a full-head backend being wrapped in Ulysses). The single use_ulysses flag now drives both the inner head count and the wrap. - Add a debug log when Ulysses is disabled despite ulysses_size>1 and sequence-parallel on (a SEPARATE_QKV cross-attn that is neither async nor pure-Ulysses, cp_size>1), so the all-gather fallback is observable. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
42cc7f1 to
a6166db
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54284 [ run ] triggered by Bot. Commit: |
|
PR_Github #54227 [ run ] completed with state |
|
PR_Github #54284 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #54436 [ run ] triggered by Bot. Commit: |
…es + log) Address NVShreyas's review on PR NVIDIA#15303: - wrap_parallel_attention now takes a use_ulysses argument and gates the UlyssesAttention wrap on it, so the outer wrap stays consistent with the inner backend's head-count decision (avoids a full-head backend being wrapped in Ulysses). The single use_ulysses flag now drives both the inner head count and the wrap. - Add a debug log when Ulysses is disabled despite ulysses_size>1 and sequence-parallel on (a SEPARATE_QKV cross-attn that is neither async nor pure-Ulysses, cp_size>1), so the all-gather fallback is observable. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
a6166db to
9d701e3
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54471 [ run ] triggered by Bot. Commit: |
|
PR_Github #54436 [ run ] completed with state |
|
PR_Github #54471 [ 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>
…es + log) Address NVShreyas's review on PR NVIDIA#15303: - wrap_parallel_attention now takes a use_ulysses argument and gates the UlyssesAttention wrap on it, so the outer wrap stays consistent with the inner backend's head-count decision (avoids a full-head backend being wrapped in Ulysses). The single use_ulysses flag now drives both the inner head count and the wrap. - Add a debug log when Ulysses is disabled despite ulysses_size>1 and sequence-parallel on (a SEPARATE_QKV cross-attn that is neither async nor pure-Ulysses, cp_size>1), so the all-gather fallback is observable. Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
9d701e3 to
811b044
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #55441 [ run ] triggered by Bot. Commit: |
|
PR_Github #55441 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #55534 [ run ] triggered by Bot. Commit: |
|
PR_Github #55534 [ run ] completed with state |
…15303) Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
Description
#14044 added Ulysses sequence-parallelism for the v2a (video→audio) cross-attention; #13978 (async-Ulysses refactor) silently disabled it — cross-attn was excluded from the Ulysses path at both the base-class
use_ulyssesgate and the LTX-2 dual-attn gate, anduse_ulysses=Truewas dropped from the v2a construction. As a result v2a fell back to all-gathering the full video K/V every block, so its per-rank communication grows withulysses_sizeinstead of scaling.This restores the v2a Ulysses path under pure Ulysses (
cp_size == 1), while preserving #13978's async-Ulysses (self-attn only) and keeping the combined ring/attn2d + Ulysses (cp_size > 1) all-gather fallback unchanged:modules/attention.py: head-shard theSEPARATE_QKVcross-attn inner backend whencp_size == 1.transformer_ltx2.py: build the dual-attn pair for cross-attn under pure Ulysses, and passuse_ulysses=Truetovideo_to_audio_attn.Output is numerically unchanged; this is a communication/perf fix (v2a uses the Ulysses 4-collective a2a instead of the full-video all-gather).
Test Coverage
Existing multi-GPU parity tests (
multi_gpu/test_ltx2_ulysses.py,multi_gpu/test_ltx2_async_ulysses.py) cover correctness/parity for the v2a + async paths.GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.Summary by CodeRabbit
Release Notes