[TRTLLM-15403][fix] VisualGen: warn when a requested attention backend silently falls back to VANILLA - #17698
Conversation
…d silently falls back to VANILLA VisualGen attention resolution silently swaps a requested TRTLLM (or CUTEDSL VSA) backend to VANILLA for SEPARATE_QKV modules, and Cosmos3 cross-attention does the same before construction. Add logger.warning_once at both fallback sites, keyed on (module class, requested, resolved), so the fallback is visible without per-instance log spam. No behavior change. Evidence: internal harness (B200, 1.3.0rc24 baseline) — Qwen-Image 60/60 joint-attention modules and cosmos3 128/128 attention modules constructed VANILLA under a TRTLLM request with zero log output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Chang Liu <9713593+chang-l@users.noreply.github.com>
|
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)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. WalkthroughThe changes add deduplicated warnings for unsupported attention backend configurations. The warnings identify the requested backend, resolved ChangesAttention backend diagnostics
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This change only makes existing attention-backend fallbacks visible through deduplicated warnings without changing backend resolution or runtime behavior; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run --disable-fail-fast |
|
PR_Github #67147 [ run ] triggered by Bot. Commit: |
|
PR_Github #67147 [ run ] completed with state
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/bot run --disable-fail-fast |
|
PR_Github #67191 [ run ] triggered by Bot. Commit: |
|
PR_Github #67191 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #67214 [ run ] triggered by Bot. Commit: |
|
PR_Github #67214 [ run ] completed with state
|
|
/bot run |
|
PR_Github #67281 [ run ] triggered by Bot. Commit: |
|
PR_Github #67281 [ run ] completed with state
|
|
/bot run |
|
PR_Github #67494 [ run ] triggered by Bot. Commit: |
|
PR_Github #67494 [ run ] completed with state |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand. Details
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
Transfer landed on main as a squash (cbc3784), so the conflict set is the same action-vs-transfer overlap staged earlier on cosmos3-action-staging; the resolution is taken from there (1e4c1bb286) with the two hunks transfer gained after that staging base folded in via a file-level three-way merge (the Cosmos3CrossAttention backend-fallback warning_once from NVIDIA#17698 and the guardrail early-return fix from NVIDIA#17510). Resolution summary, argued in full in the staging commit: - Token layout control|video|action(|audio): transfer prepends control tokens, action appends, so the velocity decode reads video at T_control..T_control+T_vid_tokens and the extra span starts after it. Control latents are mutually exclusive with both audio (transfer's guard) and action (added here). - Kept the per-request cached_freqs_gen_combined mRoPE table for the audio branch rather than transfer's per-step rebuild. - Dropped action's probe_video_dimensions for transfer's video_stream_info (a strict superset: same header read also returns the frame rate); action_reference_size raises on an unreadable container instead of propagating None. - Action skips the source-following parameter resolution: its frame rate is a trained property of the embodiment, not of the reference footage. - Action's scheduler stream joins transfer's `transfer_config is None` rebuild guard; the two never arrive together. Signed-off-by: Igor Shovkun <igshov@gmail.com>
[TRTLLM-XXXXX][fix] VisualGen: warn when a requested attention backend silently falls back to VANILLA
Description
Problem
VisualGen attention backend resolution can silently ignore the backend the user requested:
tensorrt_llm/_torch/visual_gen/modules/attention.py— anyAttentionmodule constructedwith
qkv_mode=SEPARATE_QKV(cross-attention, and separate-projection joint attention suchas Qwen-Image's
QwenJointAttentionorCosmos3CausalAttention) swaps a requestedTRTLLMbackend — or
CUTEDSLwith a VSA sparse config — toVANILLAwith zero log output.tensorrt_llm/_torch/visual_gen/models/cosmos3/transformer_cosmos3.py—Cosmos3CrossAttentionswaps a requestedTRTLLMbackend toVANILLAbefore calling thebase constructor, also silently.
A user who sets
attention.backend=TRTLLMcan end up with every attention module in themodel running VANILLA (torch SDPA) and nothing in the logs saying so. Observed with an internal
study harness (B200, TRT-LLM 1.3.0rc24 container; numbers are from that baseline):
by_resolved_backend {VANILLA: 60})The Cosmos case was initially recorded in a perf study as if TRTLLM had run; the silent
fallback was only caught later by a post-construction module census and invalidated two study
conclusions. The same silent path also blocks CUTEDSL VSA sparse attention on SEPARATE_QKV
models: the sparse config is accepted but every module quietly resolves to dense VANILLA.
The change
Add a
logger.warning_onceat both fallback sites naming the module class, the requestedbackend, and the resolved backend. The warning is keyed on the
(module class, requested, resolved)triple, so each distinct fallback is reported exactlyonce per process instead of once per module instance (one line instead of 60 on Qwen-Image).
No behavior change — backend resolution is unchanged; this only makes the existing
fallback visible. Dedup reuses the existing
Logger.log_oncemachinery(
tensorrt_llm/logger.py); no new mechanism or configuration knob is introduced.Example output (standard
[TRT-LLM] [W]warning line):Test Coverage
python3 -m py_compileon both touched files.added on paths already exercised by existing unit tests
(
tests/unittest/_torch/visual_gen/test_attention_integration.py,tests/unittest/_torch/visual_gen/test_qwen_image_pipeline.py,tests/unittest/_torch/visual_gen/test_cosmos3_transformer.py).Logger.log_onceimplementation.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.
Dev Engineer Review
SEPARATE_QKVmodules and Cosmos3 cross-attention.QA Engineer Review
No test changes.