[https://nvbugs/6506918][fix] Reject the incompatible configuration up front in Linear.__init__ with an… - #16870
Conversation
Uneven-TP column-parallel Linear (TRTLLM-13117) produces different local output sizes per rank, which the NCCL AllGather collective cannot handle without per-rank sizes. Combined with gather_output=True, this deadlocks the collective on odd out_features (e.g. hidden_size=127 with tp_size=2), manifesting as "Test terminated unexpectedly" in tests/unittest/_torch/multi_gpu/test_linear.py::test_column_linear[2-unbalanced]. Reject the incompatible config up front in Linear.__init__ with a clear assertion instead of hanging mid-collective. Uneven TP still works for COLUMN Linears that keep the shard local (gather_output=False), which is what the VisualGen feature was designed for. Also remove the now-passing test from waives.txt. Signed-off-by: handongl <handongl@nvidia.com>
Walkthrough
ChangesLinear column-sharding validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_torch/modules/linear.py`:
- Around line 3309-3317: Replace the assert in the gather_output column-parallel
validation with an explicit ValueError or RuntimeError, preserving the existing
condition and descriptive message. Ensure the unsupported uneven-TP
configuration is rejected unconditionally before reaching the allgather path,
including when Python runs with optimizations.
- Around line 3309-3317: Update the COLUMN gather_output validation near the
linear module initialization to account for override_tp_sharding before
accepting the configuration. Resolve the per-rank shard sizes after applying the
override and reject any uneven sizes, even when out_features is divisible by
tp_size, since the allgather path cannot provide sizes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ad46ddb9-3697-4f7d-bc13-be768550bb4e
📒 Files selected for processing (2)
tensorrt_llm/_torch/modules/linear.pytests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/test_lists/waives.txt
| # gather_output requires a uniform NCCL AllGather; uneven column shards | ||
| # would produce mismatched local buffers and hang the collective. Reject | ||
| # this configuration up front rather than fail mid-collective. | ||
| if (gather_output and self.tp_size > 1 | ||
| and self.tp_mode == TensorParallelMode.COLUMN): | ||
| assert out_features % self.tp_size == 0, ( | ||
| f"gather_output=True is not supported with uneven TP. " | ||
| f"out_features={out_features} must be divisible by tp_size={self.tp_size}." | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python -O - <<'PY'
def guard():
assert False, "must fail"
guard()
print("assertions are disabled under -O")
PYRepository: NVIDIA/TensorRT-LLM
Length of output: 190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section with line numbers.
sed -n '3280,3345p' tensorrt_llm/_torch/modules/linear.py
printf '\n--- search for related shard calculations ---\n'
rg -n "override_tp_sharding|calculate_local_out_features|gather_output|tp_mode == TensorParallelMode.COLUMN|out_features % self.tp_size" tensorrt_llm/_torch/modules/linear.pyRepository: NVIDIA/TensorRT-LLM
Length of output: 5656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- calculate_local_out_features ---'
sed -n '3452,3495p' tensorrt_llm/_torch/modules/linear.py
printf '\n%s\n' '--- gather_output runtime path ---'
sed -n '3690,3735p' tensorrt_llm/_torch/modules/linear.py
printf '\n%s\n' '--- override_tp_sharding references ---'
rg -n "override_tp_sharding|calculate_local_out_features|calculate_local_in_features" tensorrt_llm/_torch/modules/linear.pyRepository: NVIDIA/TensorRT-LLM
Length of output: 5487
Use an explicit exception here instead of assert. assert is stripped under python -O, so this unsupported gather_output/uneven-TP path can still reach allgather and hang. Raise ValueError or RuntimeError so the guard always runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tensorrt_llm/_torch/modules/linear.py` around lines 3309 - 3317, Replace the
assert in the gather_output column-parallel validation with an explicit
ValueError or RuntimeError, preserving the existing condition and descriptive
message. Ensure the unsupported uneven-TP configuration is rejected
unconditionally before reaching the allgather path, including when Python runs
with optimizations.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 4 'override_tp_sharding|gather_output|calculate_local_out_features' \
tensorrt_llm testsRepository: NVIDIA/TensorRT-LLM
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '3320,3475p' tensorrt_llm/_torch/modules/linear.py
printf '\n---\n'
sed -n '3688,3722p' tensorrt_llm/_torch/modules/linear.py
printf '\n---\n'
sed -n '1,220p' tensorrt_llm/_torch/modules/gated_mlp.pyRepository: NVIDIA/TensorRT-LLM
Length of output: 19337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'def allgather|allgather\(' tensorrt_llm/_torch tensorrt_llm/distributed.py tensorrt_llm/distributed -g '!**/__pycache__/**'Repository: NVIDIA/TensorRT-LLM
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'override_tp_sharding=' tensorrt_llm/_torch
printf '\n---\n'
sed -n '120,220p' tensorrt_llm/_torch/custom_ops/cpp_custom_ops.py
printf '\n---\n'
sed -n '220,320p' tensorrt_llm/_torch/custom_ops/cpp_custom_ops.pyRepository: NVIDIA/TensorRT-LLM
Length of output: 19188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 'gather_output=True[^\n]*\n.*override_tp_sharding=|override_tp_sharding=.*\n.*gather_output=True' tensorrt_llm/_torch -U
printf '\n---\n'
rg -n -C 3 'tensor_parallel_mode=TensorParallelMode\.COLUMN.*gather_output=True|gather_output=True.*tensor_parallel_mode=TensorParallelMode\.COLUMN' tensorrt_llm/_torch -URepository: NVIDIA/TensorRT-LLM
Length of output: 157
Check override_tp_sharding before COLUMN gather_output (tensorrt_llm/_torch/modules/linear.py:3309-3317). An explicit per-rank override can still make local output widths differ even when out_features % tp_size == 0, and this allgather(output, self.mapping) path has no sizes argument. Validate the resolved shard sizes after applying the override, or reject uneven overrides here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tensorrt_llm/_torch/modules/linear.py` around lines 3309 - 3317, Update the
COLUMN gather_output validation near the linear module initialization to account
for override_tp_sharding before accepting the configuration. Resolve the
per-rank shard sizes after applying the override and reject any uneven sizes,
even when out_features is divisible by tp_size, since the allgather path cannot
provide sizes.
|
Closing as bug https://nvbugs/6506918 is not reproducible |
Summary
sizes=argument is passed, and the underlying C++ AllGather with sizes is also broken for this case.Linear.__init__with anassert out_features % self.tp_size == 0guard that fires whengather_output=True and tp_mode==COLUMN and tp_size>1. This restores the pre-TRTLLM-13117 behavior for exactly the code path that gather_output relies on (uniform NCCL AllGather), matches the test'spytest.raises(AssertionError)expectation, and leaves uneven TP working forgather_output=FalseVisualGen use cases. Also remove the correspondingwaives.txtentry.Test plan
Links
Dev Engineer Review
Linear.__init__to reject gathered outputs for uneven column-parallel sharding, preventing NCCL AllGather buffer-size mismatches and hangs.gather_output=False.QA Engineer Review
test_column_linear[2-unbalanced]waiver fromtests/integration/test_lists/waives.txt.