Repository navigation
[TRTLLM-15864][chore] Consolidate the VisualGen attention domain under _torch/visual_gen/attention/ - #20214
Merged
Conversation
Merged
1 task done
Merged
1 task done
YihuiLu512
force-pushed
the
feat-dev-J15864
branch
from
October 13, 2026 09:22
b71fd4a to
e9c6265
Compare
YihuiLu512
marked this pull request as ready for review
October 13, 2026 09:22
YihuiLu512
requested review from
PerkzZheng,
allisonlim-nv,
arysef,
chang-l,
hanjingtian,
karljang,
weiminwang-nv,
yibinl-nvidia,
yuxianq and
zhaoyangwang-nvidia
October 13, 2026 09:23
Collaborator
Author
|
/bot run --disable-fail-fast |
kris1025
approved these changes
Oct 13, 2026
Collaborator
|
PR_Github #77489 [ run ] triggered by Bot. Commit: |
zhenhuaw-me
approved these changes
Oct 13, 2026
YihuiLu512
enabled auto-merge (squash)
October 13, 2026 09:34
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughVisual Gen attention modules and references now use the ChangesVisual Gen attention package reorganization
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 50 files. (15 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the chore and the primary change: consolidating the VisualGen attention domain under the new package path. Description check The description explains the move, lists updated references, documents deferred compatibility shims, identifies affected dependent PRs, and provides relevant test coverage. The required Description, T… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 50 files. (15 skipped: 4 unsupported, 11 over the file limit.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #77555 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #77555 [ run ] completed with state
|
…r _torch/visual_gen/attention/ Move the VisualGen attention backends and the DiT attention module into one domain package: _torch/visual_gen/attention_backend/ -> _torch/visual_gen/attention/backends/ _torch/visual_gen/modules/attention.py -> _torch/visual_gen/attention/attention.py Relative imports that leave the moved subtree gain one dot for the new depth; modules/__init__.py resolves Attention/QKVMode lazily to avoid an import cycle; CODEOWNERS, the groovy related-file list, docs and comments follow the new paths. No old path is left behind, so git records every move as a rename; forwarding shims follow in a separate change. Signed-off-by: Yihui Lu <269394165+YihuiLu512@users.noreply.github.com>
- Drop the Attention/QKVMode re-export from visual_gen/modules/__init__.py; nothing in the repo imports them from there, and the forwarding for out-of-tree callers belongs with the shims in the follow-up PR. - Restore the Lightricks copyright line in transformer_ltx2.py. - Update the remaining stale references to the old attention_backend path (cute_dsl/fmha.py docstring, blog25 links). Signed-off-by: Yihui Lu <269394165+YihuiLu512@users.noreply.github.com>
YihuiLu512
force-pushed
the
feat-dev-J15864
branch
from
October 13, 2026 19:46
bed96f3 to
debb28a
Compare
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #77587 [ run ] triggered by Bot. Commit: |
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #77623 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #77587 [ run ] completed with state |
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #77686 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #77623 [ run ] completed with state |
Collaborator
|
PR_Github #77686 [ run ] completed with state
|
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #77711 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #77711 [ run ] completed with state |
This was referenced Oct 14, 2026
Merged
chang-l
added a commit
to chang-l/TensorRT-LLM
that referenced
this pull request
Oct 14, 2026
Resolve conflicts with upstream NVIDIA#20214, which consolidated the VisualGen attention domain under _torch/visual_gen/attention/: - modules/attention.py moved to attention/attention.py; the PR's changes carry over. The L0_MergeRequest.groovy path filter now points at the new location, and parallel/ stays. - Point the two PR-added imports of the moved module at the new path (parallel/token_sharded_modules.py, test_token_sharded_tp_layout.py). - Keep both new multi-GPU unit tests in l0_cpu.yml. - Feature matrix: keep the token-sharded TP footnote and the SOL-Attn column. Signed-off-by: Chang Liu <9713593+chang-l@users.noreply.github.com>
Collaborator
Author
|
shim PR：#20215 |
1 task
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.
Description
Pure move, mirroring #17968:
tensorrt_llm/_torch/visual_gen/attention_backend/tensorrt_llm/_torch/visual_gen/attention/backends/tensorrt_llm/_torch/visual_gen/modules/attention.pytensorrt_llm/_torch/visual_gen/attention/attention.pyAll references, CODEOWNERS, and the groovy file list are updated.
visual_gen.modulesno longer re-exportsAttention/QKVMode(no in-repo users); #20215 restores it lazily.Forwarding shims for the old paths come in a follow-up PR, because a shim left at the old path in this squash commit would break rename detection and
git blame.Please hold #19751 and #19358 until the shim PR merges. They add files under the old directory.
Test Coverage
Existing VisualGen tests. Locally, against base: every moved file is detected as a rename, the collected test set is identical, and the
l0_cpuVisualGen results are identical.PR Checklist
Dev Engineer Review
The change consolidates VisualGen attention code under
tensorrt_llm/_torch/visual_gen/attention/. It moves the backend package andmodules/attention.py, then updates source imports, documentation, ownership rules, and the Groovy multi-GPU file list to use the new paths.visual_gen.modulesno longer re-exportsAttentionorQKVMode. Callers that import these symbols from that package must update; a lazy compatibility re-export is planned for a follow-up. No forwarding shims are included, so consumers of the old module paths must also update. Review finding counts are unavailable.QA Engineer Review
The change updates existing VisualGen unit-test imports but does not report changes to test functions or test IDs. The PR description reports that the collected test set is unchanged. It also reports identical local
l0_cpuVisualGen results. CI comments report failures in two L0 pipelines but do not identify the failing tests. Coverage verdict: needs follow-up until those failures are understood.Per-File QA Perspective
tensorrt_llm/_torch/visual_gen/attention/attention.py: Verify attention construction and calls after updating backend imports.tensorrt_llm/_torch/visual_gen/attention/backends/**: Verify backend imports and behavior after the package move, especially relative imports in sparse and parallel backends.tensorrt_llm/_torch/visual_gen/modules/__init__.py: Verify callers no longer rely onmodulesre-exportingAttentionorQKVMode.tensorrt_llm/_torch/visual_gen/models/: Verify model imports resolve from the relocated attention modules.tests/unittest/_torch/visual_gen/**: Updated test imports retain existing coverage, including multi-GPU, sparse attention, backend, and integration behavior. Test-list membership is not established by the supplied evidence..github/CODEOWNERS,jenkins/L0_MergeRequest.groovy, and documentation files: These updates align ownership, multi-GPU file selection, and documented paths with the new layout; verify the intended review ownership and CI selection.