[https://nvbugs/6418815][fix] Revert the per-batch cross-attention slicing (drop real_text_lens plumbing in… - #15986
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 (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughCosmos3CrossAttention and Cosmos3GenDecoderLayer no longer accept ChangesCosmos3 Cross-Attention Simplification and Test Updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
be138a5 to
1f80504
Compare
1f80504 to
57af1f8
Compare
BowenFu
left a comment
There was a problem hiding this comment.
LGTM — clean, complete revert of the per-batch cross-attention slicing regression; single concatenated-attention path restored with no dangling references.
…put feature The Cosmos3 audio-output feature (commit f50ca53) silently changed three pipeline behaviors that the T2I LPIPS golden was baked against: 1. Refactored Cosmos3CrossAttention to per-batch slice k_und/v_und by real_text_lens when batch_size > 1. Under CFG (batch_size=2), the shorter (negative-prompt) entry now attends over only its real text length rather than the padded max_real_len slice used by every other batch entry, producing a substantially different image. 2. Replaced the long descriptive COSMOS3_DEFAULT_NEGATIVE_PROMPT with "". 3. Bumped COSMOS3_720P_PARAMS["max_sequence_length"] from 1024 to 4096. The result was LPIPS = 0.608404 on test_cosmos3_nano_t2i_lpips_against_golden (12x the 0.05 threshold). This mirrors the T2V-sibling fix on branch repair-bot-bug6410093 (commit a8cf30c); the same underlying regression affects both variants but T2V happens to still land under the 0.05 threshold with the new pipeline defaults while T2I diverges. Fix: - Revert the per-batch cross-attention slicing: drop the real_text_lens parameter from Cosmos3CrossAttention.forward and Cosmos3GenDecoderLayer.forward, and stop computing/passing it in Cosmos3VFMTransformer.forward. All batch entries now share the same k_und[:, :max_real_len] slice as before. The per-batch path was only preparatory for future audio work and is not exercised by any existing audio test (audio tests use batch_size=1). - Pin the LPIPS-golden-specific negative_prompt and max_sequence_length in the test itself (matching the WAN21/22, LTX2, QwenImage pattern), so the LPIPS test stays decoupled from future public-default changes. - Remove the nvbugs/6418815 waiver. Verified: LPIPS score drops to 0.000142 (well below the 0.05 threshold). Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com> Signed-off-by: handongl <handongl@nvidia.com>
…only tests Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com> Signed-off-by: handongl <handongl@nvidia.com>
57af1f8 to
42f89cc
Compare
Summary
Test plan
Links
Summary by CodeRabbit
Bug Fixes
Tests