Balance LayerWise optimizer shards by Newton-Schulz cost, not parameter size - #6379
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
The existing LPT bin-packing sorts params by numel and assigns each to the least-loaded shard by numel. With GTP, this creates severe compute imbalance: a GTP-sharded param with local shape [548, 8192] has the same numel as a non-GTP param of that size, but its Newton-Schulz cost is 64x higher because NS operates on the full all-gathered [35072, 8192] shape. Three changes: 1. _ns_compute_cost() estimates NS cost using the full post-AllGather shape: max(M,N) * min(M,N)^2. 2. Sort and assign by compute cost instead of numel. 3. Persistent shard_compute_loads across buckets (shard_cursors still reset per bucket for correct memory layout) with a per-bucket numel cap (epsilon=0.3) to bound padding. Reduces compute imbalance from 137x to 1.3x in simulation. Measured 19% throughput improvement on Ultra-half (54-layer hybrid Mamba-MoE, 1024 GB200 GPUs, Muon + GTP=64): 2080 -> 1707 ms/iter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
_emit_bucket now assigns by Newton-Schulz cost, but the incremental estimate in the bucket cutter still tracks numel, so the comment claiming it mirrors _emit_bucket is no longer true. Say what it actually does and note that it errs optimistic, producing a slightly larger bucket rather than an invalid layout. Add a test covering the placement change: three GTP-sharded params whose Newton-Schulz cost is 16x the largest dense param but whose numel is the smallest. Numel-ordered placement piles all three onto one shard for a 3.77x compute imbalance; cost-ordered placement spreads them, giving 1.33x. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
f332b35 to
f9e5314
Compare
There was a problem hiding this comment.
LGTM.
Traced the new packing rule against every pre-existing TestSizeMatchingLayout case: the 30% numel cap does force the empty-candidates fallback in a few of them, but since the fallback reduces to the old least-numel rule the placements and asserted buffer sizes are unchanged. The candidate filter also mirrors the real placement arithmetic (pad_param_start(cursor) + numel), so an admitted shard genuinely stays under the cap.
The claim that equal-sized params make the _absorbs estimate exact holds: with equal costs, persistent shard_compute_loads only rotates which shard starts a bucket, so the multiset of shard_cursors — and hence max(shard_cursors) and the bucket size — is unchanged. The cost/imbalance numbers in the new tests and docstrings check out (268,435,456 vs 16,777,216; 1.33x; 1024 + 384 = 1408 vs 768 + 512 = 1280), and test_gtp_params_land_on_different_shards_in_each_bucket really does depend on the loads persisting, so it would fail if they were reset per bucket.
No new global process-group reads in megatron/core.
Nit: the _ns_compute_cost docstring says "for a 2D parameter", but the first branch is the non-2D nelement() fallback that the 1-D cases in TestSizeMatchingLayout rely on — "for a parameter" would match the body.
(Review by inspection; I did not execute the suite.)
The summary line said "for a 2D parameter", but the first branch returns nelement() for anything that is not 2D, which is the path every 1-D case in TestSizeMatchingLayout takes. Describe both branches in the order the body handles them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
|
/ok to test 4c62a5c |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/31605644484 |
…er size (NVIDIA#6379) Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit 8534490)
…er size (NVIDIA#6379) Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit 8534490)
Problem
LayerWiseDistributedOptimizerassigns whole Muon matrices todp_sizeshards, packing them largest-first onto the least-loaded shard. It measures "loaded" bynumel, which breaks under GTP: a GTP-sharded weight stores only its local shard, but Newton-Schulz runs on the full all-gathered matrix, so its real cost isgtp_remat_sizetimes what itsnumelsuggests.Ordering by
numeltherefore starts with the cheapest work and leaves the most expensive matrices to fill in wherever there is room, which piles them onto shards that are already busy. Every rank waits for the slowest, so the Muon step is gated on whichever shard collected them.Changes
_ns_compute_cost()estimates Newton-Schulz cost from the full post-all-gather shape (GTP shards along dim 0):max(M, N) * min(M, N)^2, the dominant term in the orthogonalization.numel.shard_compute_loadspersistently across buckets, so expensive matrices spread over the whole buffer rather than clustering inside each bucket.shard_cursorsstill resets per bucket, which preserves the memory layout. A per-bucket numel cap (_NUMEL_EPSILON = 0.3) bounds the extra padding this can introduce, and assignment falls back to the previous least-numel rule when no shard satisfies the cap.Results
Measured end-to-end on Ultra-half (54-layer hybrid Mamba-MoE, 1024 GB200 GPUs, Muon + GTP=64): 2080 -> 1707 ms/iter, a 19% throughput improvement.
Tests
tests/unit_tests/distributed/test_layer_wise_param_layout.pygainsTestComputeBalancedLayout, built sonumeland cost disagree: three GTP-sharded matrices whose Newton-Schulz cost is 16x the largest dense matrix but whosenumelis the smallest in the set. Numel-ordered placement puts all three on one shard for a 3.77x compute imbalance; cost-ordered placement spreads them across three shards for 1.33x. The test asserts both the spread and the imbalance bound, so it fails against the previous heuristic.The pre-existing cases in
TestSizeMatchingLayoutall use 1-D params, where_ns_compute_costfalls back tonelement()and cost equalsnumel. They therefore pass identically under both heuristics and cannot detect this change; a class docstring now says so.Notes
_emit_bucketsorts a chunk before packing it, while the incremental estimate in the bucket cutter walks params in backprop order. #5415's comment claimed the two mirror each other. They do not, and after this PR they also differ in what they measure. Equal-sized params make both differences vanish; mixed sizes can send params to different shards, so absorbing occasionally enlarges a bucket instead of filling it.@kunlunl raised this in review of #5415 with a counterexample, now pinned as
test_mixed_sizes_can_absorb_into_larger_bucket:dp_size=2, backprop-order numels[192, 192, 256, 128, 128, 192],bucket_size=448emits 1408 elements where closing at the threshold emits 1280. Documented rather than fixed, since an exact estimate would mean re-sorting the chunk on every param, and the target case is equal-sized expert matrices.This helps without GTP too, though less dramatically. Newton-Schulz cost is
numel * min(M, N), sonumelmisvalues a matrix by its aspect ratio: across this model's shapes that factor ranges from 512 to 10240, meaning a tall-thin matrix and a square one of the same byte count do very different amounts of work. Balancingnumelequalises bytes per shard; balancing cost equalises the Newton-Schulz FLOPs that actually gate the step. Without GTP the two orderings largely agree and the gain is small, but it grows withdp_size, where there is less slack to absorb a misranked matrix, and it is never worse.The shared-embedding branch is untouched.
🤖 Generated with Claude Code