[https://nvbugs/6160248][fix] AutoDeploy: fixed broken pattern matching of fuse_rope_into_trtllm_attention transform#14038
Conversation
…ugh rope trace
After fuse_gemms_mixed_children switched its post-fusion split path from
``split_with_sizes-closure → getitem`` to ``narrow → contiguous → view``,
the contiguous node is emitted via ``graph.call_method("contiguous", ...)``,
i.e. a ``call_method`` op rather than a ``call_function``. This broke
``fuse_rope_into_trtllm_attention``'s QKV-passthrough leg: ``_unwrap_contiguous``
only handled ``call_function`` forms, and ``_trace_narrow`` walked
``view → narrow`` directly without peeling intermediate contiguous nodes.
As a result the passthrough silently failed to fire and ``_trtllm_fused_qkv``
was never set, causing
``test_gemm_fusion_trtllm.py::test_fuse_qkv_passthrough_with_rope`` to fail.
Fix:
- Extend ``_unwrap_contiguous`` to also skip ``call_method`` nodes whose
target is the string ``"contiguous"``.
- In ``_trace_narrow`` and ``_trace_split`` apply ``_unwrap_contiguous`` to
the view's input before testing for narrow / getitem.
Signed-off-by: Eran Geva <19514940+MrGeva@users.noreply.github.com>
|
/bot run --extra-stage "DGX_B200-4_GPUs-AutoDeploy-1, DGX_H100-4_GPUs-AutoDeploy-1" --disable-fail-fast |
|
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 (1)
📝 WalkthroughWalkthroughThe PR enhances RoPE-attention graph tracing by expanding the ChangesContiguous unwrapping in fused RoPE-attention tracing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 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 |
|
PR_Github #47923 [ run ] triggered by Bot. Commit: |
|
PR_Github #47923 [ run ] completed with state
|
The recent commit 6cd23bc changed fuse_gemms_mixed_children to emit narrow → contiguous (call_method) → view instead of split_with_sizes (closure) → getitem. But _try_trace_to_fused_qkv._trace_narrow only handles view → narrow directly — it does not unwrap a call_method("contiguous", ...) node sitting between the view and the narrow, and _unwrap_contiguous only handles call_function contiguous, not the call_method form. Result: the passthrough leg silently fails and _trtllm_fused_qkv is never set.
Fix:
_unwrap_contiguousto also skipcall_methodnodes whose target is the string"contiguous"._trace_narrowand_trace_splitapply_unwrap_contiguousto the view's input before testing for narrow / getitem.Summary by CodeRabbit
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)
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.