[https://nvbugs/6410093][fix] Revert the per-batch cross-attn slicing (drop real_text_lens plumbing through… - #15915
Closed
trtllm-agent wants to merge 1 commit into
Closed
Conversation
…n LPIPS golden defaults The Cosmos3 audio-output feature (commit f50ca53) silently changed three pipeline behaviors that the 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 video. 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.4458 on test_cosmos3_nano_t2v_lpips_against_golden (9x the 0.05 threshold). 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/6410082 waiver. Verified: LPIPS score drops to 0.009633 (well below the 0.05 threshold). Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
Contributor
|
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)
📝 WalkthroughWalkthroughRemoves ChangesCosmos3 cross-attention simplification and test re-enablement
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Transformer as Cosmos3VFMTransformer
participant Layer as Cosmos3GenDecoderLayer
participant Attn as Cosmos3CrossAttention
Transformer->>Layer: layer(k_und, v_und, ...)
Layer->>Attn: cross_attention(k, v, k_und, v_und)
Attn->>Attn: concatenate k_und/v_und with k/v
Attn->>Attn: run _attn_impl with FULL mask
Attn-->>Layer: attention output
Layer-->>Transformer: layer output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Links
Summary by CodeRabbit