[TRTLLM-12762][fix] Enable multi-node TP for MiniMax-M2 - #14314
Conversation
The QK-norm path in MiniMaxRMSNorm uses an IPC-based fused all-reduce kernel that is only available between GPUs with peer-to-peer access, preventing MiniMax-M2 from running with tensor parallelism that spans multiple nodes. Additionally, the k_norm weight loader does not replicate heads when num_kv_heads < tp_size, so checkpoints fail to load in typical cross-node configurations (e.g. 8 KV heads with TP=16). This change: - Detects cross-node TP via can_access_peer(mapping) at construction and caches the result on MiniMaxRMSNorm. - When peer access is unavailable, MiniMaxRMSNorm.forward falls back to an NCCL all-reduce of the partial sum-of-squares followed by a local RMS normalization. MiniMaxM2Attention.apply_qk_norm falls back to separate per-tensor q_norm(q) / k_norm(k) calls instead of the fused IPC kernel. - In MiniMaxRMSNorm.load_weights, when the checkpoint tensor is smaller than tp_size * hidden_size, replicate at the head level using repeat_interleave before passing to load_weight_shard. This mirrors duplicate_kv_weight behavior for k_proj/v_proj. Intra-node TP behavior is unchanged: the fast IPC-based fused kernel is still used when can_access_peer(mapping) is true. Signed-off-by: Pietro Cicotti <5833013+pcicotti@users.noreply.github.com>
|
/bot run |
📝 WalkthroughWalkthroughThis PR extends ChangesRMS Normalization P2P/IPC Support
🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tensorrt_llm/_torch/models/modeling_minimaxm2.py (1)
144-166:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffAdd validation for weight replication divisibility constraints.
The weight replication logic uses integer division without validation:
- Line 153:
num_total_heads = src.shape[0] // self.head_dimassumessrc.shape[0]is divisible byhead_dim- Line 154:
reps = self.mapping.tp_size // num_total_headsassumestp_sizeis divisible bynum_total_headsIf these constraints don't hold, the replication will silently produce incorrect weights, leading to model corruption. For example, with
tp_size=16andnum_total_heads=9,repswould be 1 instead of the required ~1.78, producing only 9 heads instead of 16.🛡️ Proposed validation
full_size = self.mapping.tp_size * self.hidden_size if src.shape[0] < full_size and self.head_dim is not None: num_total_heads = src.shape[0] // self.head_dim + if src.shape[0] % self.head_dim != 0: + raise ValueError( + f"Checkpoint weight size {src.shape[0]} is not divisible by head_dim {self.head_dim}" + ) reps = self.mapping.tp_size // num_total_heads + if self.mapping.tp_size % num_total_heads != 0: + raise ValueError( + f"TP size {self.mapping.tp_size} must be divisible by num_total_heads {num_total_heads} " + f"for weight replication. Consider using a different TP configuration." + ) src = (🤖 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/models/modeling_minimaxm2.py` around lines 144 - 166, In load_weights, validate divisibility before computing num_total_heads and reps: ensure src.shape[0] is divisible by self.head_dim (so num_total_heads = src.shape[0] // self.head_dim is exact) and ensure self.mapping.tp_size is divisible by num_total_heads (so reps = self.mapping.tp_size // num_total_heads is exact); if either check fails, raise a clear ValueError mentioning load_weights, src.shape[0], self.head_dim, num_total_heads and self.mapping.tp_size so the caller knows why replication cannot proceed safely.
🤖 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.
Outside diff comments:
In `@tensorrt_llm/_torch/models/modeling_minimaxm2.py`:
- Around line 144-166: In load_weights, validate divisibility before computing
num_total_heads and reps: ensure src.shape[0] is divisible by self.head_dim (so
num_total_heads = src.shape[0] // self.head_dim is exact) and ensure
self.mapping.tp_size is divisible by num_total_heads (so reps =
self.mapping.tp_size // num_total_heads is exact); if either check fails, raise
a clear ValueError mentioning load_weights, src.shape[0], self.head_dim,
num_total_heads and self.mapping.tp_size so the caller knows why replication
cannot proceed safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bda3c719-f8ea-4010-ae80-3d7af8de3a9c
📒 Files selected for processing (1)
tensorrt_llm/_torch/models/modeling_minimaxm2.py
|
PR_Github #49217 [ run ] triggered by Bot. Commit: |
|
PR_Github #49217 [ run ] completed with state
|
…eight replication Assert that the checkpoint weight size is divisible by head_dim and that tp_size is divisible by num_total_heads before head-level replication in `MiniMaxRMSNorm.load_weights`. Without these checks, integer-truncated `reps = tp_size // num_total_heads` could silently produce fewer heads than `tp_size`, leading to wrong shard sizes downstream. Signed-off-by: Pietro Cicotti <5833013+pcicotti@users.noreply.github.com>
|
/bot run --disable-fail-fast CI #38889 failures look unrelated to this PR (only touches
Both "Test terminated unexpectedly" (worker crash). Historically flaky — cf. #3921 / https://nvbugs/5247232.
Re-running with fail-fast disabled. |
|
PR_Github #49466 Bot args parsing error: Failed to parse bot args |
|
/bot run --disable-fail-fast |
|
PR_Github #49468 [ run ] triggered by Bot. Commit: |
|
PR_Github #49468 [ run ] completed with state |
Add a MiniMax-M2 (FP8 block-scales) case to test_multi_nodes_eval at TP16/PP1/EP16, covering the cross-node tensor-parallel fallback added in PR NVIDIA#14314: NCCL all-reduce RMS norm, per-tensor QK-norm, and head-level weight replication (num_kv_heads=8 < tp_size=16). The case is marked skip_pre_hopper and reuses the existing MMLU threshold. Registered the TP16 case in the QA multi-node functional test list. Signed-off-by: Jie Li <lijie@nvidia.com>
Add a MiniMax-M2 (FP8 block-scales) case to test_multi_nodes_eval at TP16/PP1/EP16, covering the cross-node tensor-parallel fallback added in PR NVIDIA#14314: NCCL all-reduce RMS norm, per-tensor QK-norm, and head-level weight replication (num_kv_heads=8 < tp_size=16). The case is marked skip_pre_hopper and reuses the existing MMLU threshold. Registered the TP16 case in the QA multi-node functional test list. Signed-off-by: Jie Li <lijie@nvidia.com>
Add a MiniMax-M2 (FP8 block-scales) case to test_multi_nodes_eval at TP16/PP1/EP16, covering the cross-node tensor-parallel fallback added in PR NVIDIA#14314: NCCL all-reduce RMS norm, per-tensor QK-norm, and head-level weight replication (num_kv_heads=8 < tp_size=16). The case is marked skip_pre_hopper and reuses the existing MMLU threshold. Registered the TP16 case in the QA multi-node functional test list. Signed-off-by: Jie Li <lijie@nvidia.com>
Add a MiniMax-M2 (FP8 block-scales) case to test_multi_nodes_eval at TP16/PP1/EP16, covering the cross-node tensor-parallel fallback added in PR NVIDIA#14314: NCCL all-reduce RMS norm, per-tensor QK-norm, and head-level weight replication (num_kv_heads=8 < tp_size=16). The case is marked skip_pre_hopper and reuses the existing MMLU threshold. Registered the TP16 case in the QA multi-node functional test list. Signed-off-by: Jie Li <lijie@nvidia.com>
Add a MiniMax-M2 (FP8 block-scales) case to test_multi_nodes_eval at TP16/PP1/EP16, covering the cross-node tensor-parallel fallback added in PR NVIDIA#14314: NCCL all-reduce RMS norm, per-tensor QK-norm, and head-level weight replication (num_kv_heads=8 < tp_size=16). The case is marked skip_pre_hopper and reuses the existing MMLU threshold. Registered the TP16 case in the QA multi-node functional test list. Signed-off-by: Jie Li <lijie@nvidia.com>
Background
MiniMaxM2's QK-norm path uses an IPC-based fused all-reduce kernel(
MiniMaxAllReduceRMS) for the variance sum-of-squares reduction. IPCis only available between GPUs with peer-to-peer access, so MiniMax-M2
cannot run with tensor parallelism that spans more than one node.
Additionally,
MiniMaxRMSNorm.load_weightsdoes not replicate headswhen
num_kv_heads < tp_size, so the k_norm weights fail to load intypical cross-node configurations (e.g. 8 KV heads with TP=16).
JIRA: TRTLLM-12762
Summary
Detect cross-node TP via
can_access_peer(mapping)at construction.When peer access is unavailable,
MiniMaxRMSNorm.forwardfalls back toan NCCL all-reduce of the partial sum-of-squares followed by a local
RMS normalization, and
MiniMaxM2Attention.apply_qk_normfalls back toseparate per-tensor
q_norm(q)/k_norm(k)calls instead of thefused IPC kernel. In
MiniMaxRMSNorm.load_weights, when the checkpointtensor is smaller than
tp_size * hidden_size, the weight is replicatedat the head level using
repeat_interleavebefore passing toload_weight_shard, mirroringduplicate_kv_weightfor k_proj/v_proj.Impact
is still used when
can_access_peer(mapping)is true.nodes) with
num_kv_heads < tp_size.Summary by CodeRabbit
Release Notes