[https://nvbugs/6517842][fix] Handle mutable tensor lists in remove c… - #16926
Conversation
…opy pass auto_functionalized_v2 represents mutable Tensor[] arguments with a list length and per-element base indices. The remove-copy pass assumed every mutable argument had a single base index, causing torch.compile to look up the nonexistent _tensors_base_index key for pipeline-parallel send and receive operations. Reconstruct mutable tensor lists from their flattened bases, map each functionalized output back to the corresponding input tensor, and retain the existing scalar and optional-tensor behavior. Add regression coverage for both pp_send_tensors and pp_recv_tensors. Unwaive the cases tracked by NVBUG 6517842, NVBUG 6445472, and NVBUG 6474888. Tested on B200: - 5 remove_copy_pass unit tests passed - DeepSeekV3-Lite TP2/PP2 torch.compile accuracy case passed - TensorRT-LLM build completed successfully Signed-off-by: Jin Li <59594262+liji-nv@users.noreply.github.com>
|
/bot run --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)
💤 Files with no reviewable changes (1)
WalkthroughThe remove-copy pass now reconstructs replacements for flattened mutated tensor-list outputs in the v2 auto-functionalization path. Unit coverage validates TRT-LLM tensor-list inplace operations, and selected DeepSeekV3Lite integration waivers are updated. ChangesTensor-list mutation handling
Integration waiver updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant auto_functionalized_v2
participant remove_copy_for_mutates_args
participant FXGraph
auto_functionalized_v2->>remove_copy_for_mutates_args: provide flattened mutated outputs
remove_copy_for_mutates_args->>remove_copy_for_mutates_args: build tensor_list_replacements
remove_copy_for_mutates_args->>FXGraph: replace getitem users and insert inplace call
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #62083 [ run ] triggered by Bot. Commit: |
BowenFu
left a comment
There was a problem hiding this comment.
Regression-safe as far as I can tell. The non-list path is byte-identical to base 89c6635, and the new branch only fires when the _<arg>_length kwarg is present, which only happens for a mutable Tensor[] — so every node shape that worked before still takes the old path. min(mutates_args) can't raise because every inplace_info() entry is non-empty. And _all_bases indices are absolute across scalar and list-element bases, so first_mutated_output + base_index is the right output slot.
One residual, non-blocking: a mutable scalar arg positioned after a mutable list would have its mutates_args key shifted by the list expansion and isn't reconstructed here. No registered op in inplace_info() has that shape today, and it never worked before either, so it's not a regression — just worth a comment if someone adds such an op later.
|
PR_Github #62083 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #62198 [ run ] triggered by Bot. Commit: |
|
PR_Github #62198 [ run ] completed with state |
Signed-off-by: Jin Li <59594262+liji-nv@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #62419 [ run ] triggered by Bot. Commit: |
|
PR_Github #62419 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #62666 [ run ] triggered by Bot. Commit: |
|
PR_Github #62666 [ run ] completed with state |
…opy pass
auto_functionalized_v2 represents mutable Tensor[] arguments with a list length and per-element base indices. The remove-copy pass assumed every mutable argument had a single base index, causing torch.compile to look up the nonexistent _tensors_base_index key for pipeline-parallel send and receive operations.
Reconstruct mutable tensor lists from their flattened bases, map each functionalized output back to the corresponding input tensor, and retain the existing scalar and optional-tensor behavior. Add regression coverage for both pp_send_tensors and pp_recv_tensors.
Unwaive the cases tracked by NVBUG 6517842, NVBUG 6445472, and NVBUG 6474888.
Tested on B200:
Dev Engineer Review
remove_copy_for_mutates_args(v2auto_functionalized_v2path) to correctly reconstruct mutable tensor lists from flattened mutated outputs by building per-getitemreplacement mappings from_all_basesand the inplace function schema write args._{arg.name}_lengthis present, includinglength is None→kwargs[arg.name] = None) and the single-base-index fallback.getitemreplacement logic so tensor-list consumers are mapped back to the correct input tensors (from the per-index replacement map when available), while preserving optional/Nonesemantics via existing assertions.getitemusers, inserts the inplacecall_functionwith reconstructedkwargs, and erases theauto_functionalized/auto_functionalized_v2node and now-unneededgetitems.pp_send_tensors.defaultpp_recv_tensors.defaulttests/integration/test_lists/waives.txtto remove/adjust DeepSeekV3Lite skip waivers tied to NVBUG 6517842, 6445472, and 6474888, aligning with the intended “waive unwaived” behavior.QA Engineer Review
Test list changes
tests/integration/test_lists/waives.txtaccuracy/test_llm_api_pytorch.py::TestDeepSeekV3Lite::test_bfloat16_4gpus:tp2pp2waiver formtp_nextn=2overlap_scheduler=False→overlap_scheduler=True.test_nvfp4_4gpus:tp2pp2withfp8kv=Trueandtorch_compile=True.test_cute_dsl_nvfp4_4gpus:tp4+fp8kv=False+torch_compile=TrueSKIPs forfull:B300,full:GB200,full:GB300.full:B200, replacedtorch_compile=Truewaiver withtorch_compile=False.Verdict: needs follow-up (CBTS coverage data not provided here).
Test code changes
tests/unittest/_torch/compilation/test_remove_copy_pass.pytest_remove_copy_for_mutates_tensor_list(inplace_func: Callable[..., object])(parametrized overtorch.ops.trtllm.pp_recv_tensors.defaultandtorch.ops.trtllm.pp_send_tensors.default).kwargs["tensors"], removal ofauto_functionalized_v2, and preservation of inplace node behavior.tests/integration/test_lists/(test-db/qa) entry for this new unit test was identifiable from the provided context.Verdict: needs follow-up (no CBTS/test-db/qa coverage linkage data available in the provided context).
torch.compileaccuracy case passedDescription
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.