[None][feat] enable GQA and cross-attention for attn2d - #14961
Conversation
📝 WalkthroughWalkthroughThis PR enhances 2D mesh-sharded attention to support Group Query Attention (different Q and KV head counts) and cross-attention (different Q and KV sequence lengths). The implementation adds separate shard sequence tracking, runtime validation, and comprehensive distributed tests validating correctness against full-sequence reference behavior. Changes2D Attention: Group Query Attention and Cross-Attention Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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)
tests/unittest/_torch/visual_gen/multi_gpu/test_attn2d_attention.py (1)
403-491: Coverage is sufficient for this PR scope.In
tests/unittest/_torch/visual_gen/multi_gpu/test_attn2d_attention.py, coverage is sufficient for the new GQA and cross-attention Attention2D paths (distributed correctness vs SDPA reference).
Optional follow-up outside this PR: add one negative test for invalidnum_heads/num_kv_headsratio handling in_LSEVanillaAttention.Also applies to: 531-540
🤖 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 `@tests/unittest/_torch/visual_gen/multi_gpu/test_attn2d_attention.py` around lines 403 - 491, Add a negative unit test to validate that _LSEVanillaAttention rejects invalid num_heads / num_kv_heads ratios: call _LSEVanillaAttention with mismatched num_kv_heads (e.g., num_heads=8, num_kv_heads=3 or 0) and assert it raises the expected error (ValueError or a specific exception) rather than silently proceeding; place the test alongside the existing multi-GPU tests (the file test_attn2d_attention.py) and reference the constructor _LSEVanillaAttention to locate the code path that should perform the validation.
🤖 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 `@tests/unittest/_torch/visual_gen/multi_gpu/test_attn2d_attention.py`:
- Around line 403-491: Add a negative unit test to validate that
_LSEVanillaAttention rejects invalid num_heads / num_kv_heads ratios: call
_LSEVanillaAttention with mismatched num_kv_heads (e.g., num_heads=8,
num_kv_heads=3 or 0) and assert it raises the expected error (ValueError or a
specific exception) rather than silently proceeding; place the test alongside
the existing multi-GPU tests (the file test_attn2d_attention.py) and reference
the constructor _LSEVanillaAttention to locate the code path that should perform
the validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 662e9eda-102f-4236-988b-271899d66b06
📒 Files selected for processing (3)
tensorrt_llm/_torch/visual_gen/attention_backend/parallel.pytensorrt_llm/_torch/visual_gen/modules/attention.pytests/unittest/_torch/visual_gen/multi_gpu/test_attn2d_attention.py
878508c to
da6c8ab
Compare
|
/bot run |
|
PR_Github #52412 [ run ] triggered by Bot. Commit: |
|
PR_Github #52412 [ run ] completed with state
|
|
/bot run |
|
PR_Github #52521 [ run ] triggered by Bot. Commit: |
|
PR_Github #52521 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #52592 [ run ] triggered by Bot. Commit: |
|
PR_Github #52592 [ run ] completed with state
|
d27f4ca to
c64d151
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #52788 [ run ] triggered by Bot. Commit: |
|
PR_Github #52788 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #52864 [ run ] triggered by Bot. Commit: |
|
PR_Github #52864 [ run ] completed with state |
Signed-off-by: Shreyas Misra <shreyasm@nvidia.com>
Signed-off-by: Shreyas Misra <shreyasm@nvidia.com>
Signed-off-by: Shreyas Misra <shreyasm@nvidia.com>
Signed-off-by: Shreyas Misra <shreyasm@nvidia.com>
Signed-off-by: Shreyas Misra <shreyasm@nvidia.com>
88a2253 to
4b73508
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #53077 [ run ] triggered by Bot. Commit: |
|
PR_Github #53077 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #53320 [ run ] triggered by Bot. Commit: |
|
PR_Github #53320 [ run ] completed with state
|
|
/bot kill |
|
PR_Github #53432 [ kill ] triggered by Bot. Commit: |
|
PR_Github #53432 [ kill ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #53444 [ run ] triggered by Bot. Commit: |
|
PR_Github #53444 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #53519 [ run ] triggered by Bot. Commit: |
|
PR_Github #53519 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #53581 [ run ] triggered by Bot. Commit: |
|
PR_Github #53581 [ run ] completed with state |
Summary by CodeRabbit
Bug Fixes
Tests
Description
Test Coverage
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.