Conversation
🤖 ds-review-bot Code Reviewv6未发现需要修复的问题。新的权重来源选择与发送端按 GPU 转发的布局一致,其他模式保持原有行为。Python 语法检查和 diff 检查通过;当前环境缺少 PyTorch,未运行多节点 CUDA 回归测试。 v5The MR fixes a real correctness bug in the hybrid combine reduce epilogue: for v4This MR fixes the hybrid combine path used for top-k weight gradients when do_expand=False and allow_multiple_reduction=False. In that mode the sender forwards a separate top-k weight row per destination GPU, but the reduce epilogue selected the source slot's master lane using the scale-out rank, so multiple GPUs inside the same scale-out domain could read the same (wrong) row. The fix computes the weight-source rank as the global GPU rank (stored_dst_expert_idx / kNumExpertsPerRank) for this mode, keeps the previous selection for all other modes, and adds a compile-time assertion that the per-GPU layout is the non-rank layout. A standalone multi-node regression (tests/elastic/test_combine_topk_weights.py) covers same-GPU, same-scale-out/different-GPU, cross-scale-out, and invalid top-k slots, with --allow-multiple-reduction=1 as a control. Verification notes: (1) the source selection is correct because Files reviewed: 2 |
With hybrid combine,
do_expand=Falseandallow_multiple_reduction=False, the sender forwards a separate top-k weight row for each destination GPU. The reduce epilogue currently selects the master slot by scale-out rank, so routes on different GPUs in the same scale-out domain can read the wrong row.For example, receiver-local score-gradient rows
[11, 0]and[0, 22]should produce[11, 22], but selecting one domain-wide row produces[0, 22]. This affectscombine(..., topk_weights=...), including its use for dispatch backward. A plain dispatch/combine weight roundtrip can hide the bug because the dispatched weights are duplicated.This change selects the score source master slot by global GPU rank for this mode, matching the forwarded row layout. The existing selection is preserved for other modes, with a compile-time assertion for slot layout. The original header formatting is preserved; the production change is limited to this one block (+9/-1 lines).
A standalone distributed regression uses synthetic receiver-local gradients and covers same-GPU, same-domain/different-GPU, and cross-domain routes, including invalid top-k slots. It supports
--allow-multiple-reduction=1as a control.Validation: Python syntax, Ruff 0.6.5, YAPF 0.40.2, and
git diff --checkpass. The header already differs from clang-format on the base branch; whole-file formatting is intentionally excluded to keep this fix focused. Real multi-node GPU validation completed for commit655d77e6bfcdcf46b065a4596104f6f2d4338503: two nodes with 8 NVIDIA H200 GPUs each (16 ranks), with fresh processes and separate JIT caches for--allow-multiple-reduction=0and=1. All three cases passed for all ranks in both modes, and both node launchers exited with code 0 for each run. No test or production-code changes were needed. This run used PyTorch 2.9.0 (CUDA 12.4) and nvcc 12.8.61; it does not validate the README's PyTorch >=2.10 baseline or add distributed CI coverage. See the GPU validation results.Run on each node (replace the rendezvous placeholders):
torchrun --nnodes=2 --nproc-per-node=2 --node-rank=NODE_RANK \ --master-addr=MASTER_ADDR --master-port=29500 \ tests/elastic/test_combine_topk_weights.pyThe test asserts at least two logical scale-out domains and at least two GPUs per domain. Repeat with
--allow-multiple-reduction=1to exercise the control path.