Fix megabatch_orthogonalize_async hang on empty FSDP2 shards#74
Merged
Conversation
When a 2D parameter's sharded global dim is smaller than world_size or doesn't divide evenly to fill all ranks under FSDP2's contiguous chunking, some ranks receive empty (numel=0) padding-only shards. The optimizer step then hangs at NCCL alltoall. Concrete repro: a weight of shape (num_attention_heads=18, hidden=2304) under dp_shard_size=8. _chunk_with_empty(t, 8, dim=0) returns 6 real chunks of (3, 2304) plus 2 empty chunks. Ranks 0-5 hold (3, 2304) shards, ranks 6-7 hold (0, 2304). Inside megabatch_orthogonalize_async, torch.stack(U_work[r*per_rank:(r+1)*per_rank]) produces input_chunks of (per_rank, 3, 2304) on ranks 0-5 and (per_rank, 0, 2304) on ranks 6-7. dist.all_to_all requires per-pair sizes to match across the wire; they don't, and NCCL hangs. Watchdog signature for 32 same-shape params under per_rank=4: Ranks 0-5: WorkNCCL(SeqNum=N, ALLTOALL, NumelIn=8x4x3x2304 = 221184) Ranks 6-7: WorkNCCL(SeqNum=N+1, ALLTOALL, NumelIn=8x7x96x2304 = 12386304) The 6/2 split exactly matches torch.chunk(18, 8, dim=0). Padding-only ranks' zero-sized alltoall trivially completes locally so they advance to the next pending collective. Independent of optimizer (NorMuon and Dion2 both hit it) and of the upstream loss (fused or otherwise). Fix: pad each rank's local shard along comm_dim to a rank-consistent padded_local_size = max(local_sizes) before the alltoall (one allreduce MAX), then narrow the post-alltoall result back to the rank's original local size. Newton-Schulz preserves zero rows, so padding doesn't change the orthogonalization of the real rows. Mirrors what FSDP2 itself does for its own all-gather/reduce-scatter (padded_sharded_param_size in torch/distributed/fsdp/_fully_shard/_fsdp_param.py). Adds a regression test using torch.multiprocessing.spawn with NCCL on 4 GPUs covering both the empty-shard cases and a non-divisible-but- no-empty sanity case.
Contributor
Author
|
@alint77 can you check this one? |
Contributor
|
LGTM. |
Contributor
|
this has introduced a sync point in the pipeline, im working on a fix atm, will submit PR soon |
Contributor
Author
|
Ah? Yes please! |
JohnLangford
pushed a commit
that referenced
this pull request
May 6, 2026
…#75) * megabatch: precompute padded_local_size, drop per-step allreduce sync The upstream empty-FSDP2-shard fix (#74) introduced an allreduce-MAX + .item() inside megabatch_orthogonalize_async to discover the rank- consistent padded local size, adding a CPU/GPU sync point on every sharded ortho task. Compute the value locally instead: ceil(global_dim / world_size) from the still-DTensor X[0].shape in each *_update_megabatch_async caller, threaded through as a required arg when comm_dim is not None. Same correctness under FSDP2 contiguous chunking, no per-step collective. Test: extend test_megabatch_empty_shard.py with world_size=2 cases ((1, D) empty-shard and (3, D) non-divisible) so the fast path is exercised on 2-GPU dev hosts; 4-GPU cases skip dynamically. * megabatch: move padded_local_size derivation inside, pass global dim Address PR review: instead of having three callers (muon/dion2/normuon) each compute ceil(global_dim / world_size) and pass padded_local_size, have callers pass the unsharded global size along comm_dim and let megabatch_orthogonalize_async derive padded_local_size itself. Kills the three-way duplication and consolidates the FSDP2-contiguous-chunk assumption in one place. Also add a local sanity assert (padded_local_size >= original_local_size) that catches a non-contiguous sharding strategy without a collective. Test updated to pass global_comm_dim_size; both 2-GPU parametrizations still pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
megabatch_orthogonalize_asynchangs at NCCLdist.all_to_allwhenever a managed 2D weight's sharded global dim is smaller thanworld_sizeor doesn't divide evenly to fill all ranks under FSDP2's contiguous chunking. Some ranks receive empty(0, …)padding-only shards; the per-rankinput_chunksthen have inconsistent sizes on the wire and the alltoall deadlocks after the first backward.Concrete repro (the case that surfaced this)
(num_attention_heads=18, hidden=2304)(a sparse-3B model's per-head attention gate).fully_shardoverdp_shard_size=8._chunk_with_empty(t, 8, dim=0)returns 6 real(3, 2304)chunks plus 2 empty(0, 2304)chunks. After init: ranks 0–5 hold(3, 2304)shards; ranks 6–7 hold(0, 2304). Insidemegabatch_orthogonalize_async:```python
input_chunks = [torch.stack(U_work[r * per_rank : (r + 1) * per_rank]) for r in range(world_size)]
dist.all_to_all(output_chunks, input_chunks, group=process_group)
```
Each rank constructs
input_chunksfrom its own local data, so per-target chunk shapes diverge across ranks ((per_rank, 3, 2304)vs(per_rank, 0, 2304)).dist.all_to_allrequires per-pair sizes to match — they don't.NCCL watchdog signature for 32 same-shape params packed under
per_rank=4:```
Ranks 0–5: WorkNCCL(SeqNum=N, ALLTOALL, NumelIn=8×4×3×2304 = 221184)
Ranks 6–7: WorkNCCL(SeqNum=N+1, ALLTOALL, NumelIn=8×7×96×2304 = 12386304)
```
The 6/2 watchdog split exactly matches
torch.chunk(18, 8, dim=0). Padding-only ranks' zero-sized alltoall completes locally so they advance to the next pending collective and hang there.Independent of optimizer (NorMuon and Dion2 both hit it) and of any upstream-loss specifics; it's a structural property of the megabatch sharded path.
Fix
Pad each rank's local shard along
comm_dimto a rank-consistentpadded_local_size = max(local_sizes)before the alltoall (one extradist.all_reduceMAX), thennarrowthe post-alltoall result back to the rank's original local size. Newton-Schulz preserves zero rows (they contribute nothing to UᵀU), so padding doesn't change the orthogonalization of the real rows. Mirrors what FSDP2 itself does for its own all-gather/reduce-scatter (padded_sharded_param_sizeintorch/distributed/fsdp/_fully_shard/_fsdp_param.py).Test plan
tests/test_megabatch_empty_shard.pyspawns 4 NCCL ranks viatorch.multiprocessing.spawnand exercises three configurations:(global_dim=5, world_size=4)— rank 3 holds(0, …); analogous to the(18, 8)production case at smaller scale.(global_dim=2, world_size=4)— ranks 2 and 3 both hold(0, …).(global_dim=15, world_size=4)— non-divisible but no empty ranks (sanity check that the padding logic stays a no-op when uniform).mainwithout the fix and passes with it. All 3 cases pass with the fix; full existing suite (97 tests) continues to pass.