[https://nvbugs/6501376][fix] Test-only fix — drop the if hidden_size % 2 != 0: with pytest.raises(...)… - #16844
Conversation
…mn linear PR NVIDIA#14875 added VANILLA-mode uneven-TP support in tensorrt_llm/_torch/modules/linear.py via _auto_tp_sharding, so test_row_linear[2-unbalanced] (hidden=15) and test_column_linear [2-unbalanced] (hidden=127) no longer hit the divisibility assert. The pre-existing tests still expected an AssertionError on the odd branch, producing "Failed: DID NOT RAISE" in pre-merge CI. Unconditionally assert all ranks return True. Signed-off-by: handongl <handongl@nvidia.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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
ChangesUneven linear sharding
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant LinearInit
participant AutoTPSharding
participant LinearForward
participant Allgather
LinearInit->>AutoTPSharding: Query per-rank shard widths
AutoTPSharding-->>LinearInit: Return shard boundaries
LinearForward->>Allgather: Pass gather_output_sizes
Allgather-->>LinearForward: Gather uneven column outputs
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: junq <22017000+QiJune@users.noreply.github.com>
|
/bot run |
|
PR_Github #61699 [ run ] triggered by Bot. Commit: |
|
PR_Github #61699 [ run ] completed with state
|
|
/bot run |
|
PR_Github #61711 [ run ] triggered by Bot. Commit: |
|
/bot help |
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. |
|
/bot kill |
…allgather Signed-off-by: Lori Ren <lorir@nvidia.com>
|
PR_Github #61714 [ kill ] triggered by Bot. Commit: |
|
PR_Github #61711 [ run ] completed with state |
|
PR_Github #61714 [ kill ] completed with state |
|
PR_Github #61716 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #61721 [ run ] triggered by Bot. Commit: |
|
PR_Github #61721 [ run ] completed with state
|
|
/bot run |
|
PR_Github #61732 [ run ] triggered by Bot. Commit: |
|
PR_Github #61732 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #61742 [ run ] triggered by Bot. Commit: |
|
/bot kill |
|
PR_Github #61761 [ kill ] triggered by Bot. Commit: |
|
PR_Github #61742 [ run ] completed with state |
|
PR_Github #61761 [ kill ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #61764 [ run ] triggered by Bot. Commit: |
|
PR_Github #61764 [ run ] completed with state
|
StanleySun639
left a comment
There was a problem hiding this comment.
Should we close #16840? That looks like an incomplete fix.
I plan to close it after this one is merged |
|
/bot run --disable-fail-fast |
|
PR_Github #61794 [ run ] triggered by Bot. Commit: |
|
PR_Github #61794 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #61816 [ run ] triggered by Bot. Commit: |
|
PR_Github #61816 [ run ] completed with state |
…nnahz/dep-1083-port-flashinfer-stable-va-lifecycle-for-native-all-reduce * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (54 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
…nnahz/dep-1082-shared-mnnvl-moe-lifecycle * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (142 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Summary
pytest.raises(AssertionError)on the odd-hidden_sizebranch expected a divisibility assert that PR [TRTLLM-13117][feat] Implement Uneven TP Linear for VisualGen models #14875's_auto_tp_shardingno longer fires for VANILLA row/column Linear.if hidden_size % 2 != 0: with pytest.raises(...)branch in bothtest_row_linearandtest_column_linear; unconditionallyfor r in results: assert r is True.Test plan
Links
Dev Engineer Review
tensorrt_llm/_torch/modules/linear.pyto correctly handlegather_output=Truefor VANILLATensorParallelMode.COLUMNwhen_auto_tp_shardingproduces uneven per-rank shard widths:_tp_sharding_is_autotracking to distinguish whethertp_shardingcame from_auto_tp_shardingvsoverride_tp_sharding.gather_output_sizes(defaultNone) to store per-rankallgathersizes when uneven widths are detected._auto_tp_sharding(..., rank=...)to accept an optionalrankargument so shard boundaries can be queried for other ranks.forwardfor COLUMN +gather_output=True, passedsizes=self.gather_output_sizesinto the COLUMN allgather to avoid relying on shape inference.gather_outputwithoverride_tp_sharding(via aValueError), aligning with the referenced bug about this unsupported mode combination.tests/unittest/_torch/multi_gpu/test_linear.py:pytest.raises(AssertionError)expectations for “unbalanced/odd”hidden_sizein bothtest_column_linearandtest_row_linear.True, reflecting the post-[TRTLLM-13117][feat] Implement Uneven TP Linear for VisualGen models #14875 behavior where_auto_tp_shardingno longer triggers that assertion for odd hidden sizes.QA Engineer Review
tests/unittest/_torch/multi_gpu/test_linear.py):test_column_linear(removed conditional failure expectation; now assertsTruefor all ranks)test_row_linear(removed conditional failure expectation; now assertsTruefor all ranks)tests/integration/test_lists/waives.txt):unittest/_torch/multi_gpu/test_linear.py::test_column_linear[2-unbalanced]unittest/_torch/multi_gpu/test_linear.py::test_row_linear[2-unbalanced]