Add B200-Calibrated Sparse-Routing Adaptive Wave Sizing - #381
Conversation
Size block_m from realized per-expert token counts under skewed routing instead of the uniform mean, minimizing a wall-cost objective (padded rows weighted by SM100 UMMA M-utilization efficiency). Gated by DG_MEGA_MOE_IMBALANCE_AWARE_BLOCK_M; strict fallback guarantees the chosen block_m is never coarser than the default, so the balanced case is unchanged. Adds host-side unit tests, a Zipf --skew-alpha router bias for A/B benchmarking, and an on-GPU baseline-vs-adaptive harness.
| int calls_since_refresh = 0; | ||
| bool has_cached_delta = false; | ||
| }; | ||
| static thread_local AdaptiveRecvStatsCache cache; |
There was a problem hiding this comment.
🔴 critical: Keep an adaptive cache per receive-counter tensor: When one host thread invokes multiple calibrated MegaMoE layers with distinct receive-stat tensors, this single thread_local cache is overwritten on every layer. Alternating A/B counters therefore always makes pointer_changed true, so no delta is ever valid, adaptivity never activates, and every invocation performs a synchronous device-to-host copy. Cache entries need to be maintained independently per counter tensor.
🤖 v6
There was a problem hiding this comment.
Fixed in 5daaaab. The cache is now a bounded thread-local map keyed by TensorImpl identity plus the CUDA data pointer. Each entry retains the tensor identity, so alternating A/B layers keep independent snapshots. The new regression alternates two counters on one host thread; B200 job 2310282 passed and showed the adaptive wave-8 config after independent snapshots.
| const bool should_refresh = pointer_changed or not cache.has_cached_delta or | ||
| cache.calls_since_refresh >= kRefreshInterval; |
There was a problem hiding this comment.
🟡 warning: Back off sampling after zero-delta windows: If a calibrated rank receives no tokens, delta_sum == 0 leaves has_cached_delta false, and this condition forces another synchronous CUDA-to-CPU copy on every subsequent launch instead of waiting 256 launches. Highly sparse routing can therefore impose a stream synchronization on every call; failed or zero samples should still observe the refresh interval.
🤖 v6
There was a problem hiding this comment.
Fixed in 5daaaab. Only the initialization snapshot requests an immediate second sample. A zero or negative/reset delta clears the cached delta and then observes the normal 256-launch refresh interval, instead of synchronizing on every call. The 100%-masked zero-delta validation passed on 8xB200 in job 2310282.
| @@ -92,13 +92,41 @@ static int get_num_wave_pool_tokens( | |||
| ); | |||
There was a problem hiding this comment.
🟡 warning: Block_m is shrunk under skew via a purely THEORETICAL wall-cost/MMA-efficiency model and the broad min(eff, mean_tpe) fallback. That still permits pushing block_m into the small-M region the project DELISTED in history (surviving comment here says B200 measured up to 31% regressions under skew; the sibling adaptive-wave path was adopted instead). Gate/calibrate block_m adaptivity to the narrow tokens/expert bands and active-ratio that actually WON on-GPU A/B -- or keep it opt-in off-by-default pending an on-GPU perf gate. Otherwise the adaptive path can be SLOWER than the mean baseline in production.
🤖 v4
There was a problem hiding this comment.
The production imbalance-aware block_m selector was removed in af06265 after the B200 regressions. The current helper returns the upstream mean tokens/expert unchanged, except for the explicit benchmark-only force override. Production adaptivity changes only the narrowly calibrated wave size.
| const auto num_sf_ring_tokens = static_cast<int>(l1_acts_sf.size(0)); | ||
|
|
||
| // Heuristics | ||
| // NOTES: `host_recv_stats` are the sampled per-local-expert token counts used |
There was a problem hiding this comment.
🟡 warning: The block_m selector consumes the SAME cumulative per-expert receive counter that adaptive-wave sizing uses, but LACKS the adaptive-wave guard (only use realized stats after a real dispatch; never on the first call/after a counter reset; fall back to default). Block_m is a whole-run/joint-with-wave tiling; a stale/meaningless first-iteration/garbage counter can mis-select block_m and keep switching it across iterations. The config-invariance harness misses this because it FORCES distinct block_m rather than running the auto-selector. Apply the same sampling/guard/fallback discipline.
🤖 v4
There was a problem hiding this comment.
This no longer applies to the current head: block_m does not consume receive statistics. The only production consumer is adaptive wave sizing, and mega.hpp passes it only a validated per-window delta after the initial snapshot/reset guard.
| constexpr int kImbalanceFactor = 2; | ||
|
|
||
| // Count L1 blocks per expert assuming tokens are evenly spread across experts | ||
| const float num_expected_tokens_per_expert = static_cast<float>(num_tokens * num_topk) / num_experts_per_rank; |
There was a problem hiding this comment.
🟡 warning: Block_m is not an independent perf knob: the candidate block_m feeds the wave/pool-token sizing (alignment kLCMCandidateBlockM, pool-token clamp, num_expected_m_blocks/kImbalanceFactor). Changing block_m under skew simultaneously changes the chosen experts-per-wave in an unintended/contradictory way, switching the whole config non-monotonically. Harmonize the block_m + wave choice via a single cost objective/preference/order, and assert the config/performance choice is consistent/sensible (not just that output is bit-identical).
🤖 v4
There was a problem hiding this comment.
The coupled production block_m policy was removed in af06265. Production block_m is the upstream choice; the opt-in adaptive policy changes only wave 16 to wave 8 in the single measured tier. The remaining force controls are benchmark-only calibration hooks.
| const float mean_tpe = static_cast<float>(num_tokens) * num_ranks * num_topk / num_experts; | ||
|
|
||
| // Benchmark/calibration override. This deliberately bypasses both the | ||
| // imbalance gate and strict fallback so every production tier can be timed |
There was a problem hiding this comment.
🔵 suggestion: The FORCE_BLOCK_M override returns EARLY in the SAME helper that the imbalance path shares, after mapping block_m to a per-tier eff. The three-way precedence FORCE > IMBALANCE > mean (and FORCE bypassing BOTH the env gate and the strict fallback, plus the independent ADAPTIVE_WAVE flag) is undocumented, untested and fragile. Document/test the full env/flag precedence, e.g. by extending the existing config-invariance harness.
🤖 v4
There was a problem hiding this comment.
The current config-invariance harness exercises all 6 forced block_m tiers and all 6 forced wave tiers, and the source documents both controls as benchmark-only. The old production imbalance precedence discussed here was removed in af06265; forced wave still intentionally returns before the adaptive policy for calibration.
| hidden == 7168 and intermediate_hidden == 2048; | ||
| const bool use_adaptive_stats = | ||
| get_env<int>("DG_MEGA_MOE_ADAPTIVE_WAVE", 0) != 0 and | ||
| is_calibrated_shape and |
There was a problem hiding this comment.
🟡 warning: thread_local AdaptiveRecvStatsCache keys only on device_ptr and numel. With a caching allocator, a freed tensor's device address can be reused by a different logical counter tensor with identical numel, causing a delta to be computed against a stale snapshot. The negative-delta guard catches decreases, but a monotonic-looking address collision would silently produce a wrong delta. Risk is low (opt-in + single gated shape), but consider documenting the assumption or adding a cheaper guard such as validating the tensor's storage generation/data version.
🤖 v3
There was a problem hiding this comment.
Addressed in 5daaaab. The cache key now includes both the logical TensorImpl identity and CUDA data pointer, and each bounded cache entry retains a shallow tensor reference. This prevents both identities and the underlying CUDA allocation from being recycled into a false hit while the entry is live.
| constexpr int kRefreshInterval = 256; | ||
|
|
||
| const bool pointer_changed = cache.device_ptr != device_ptr or | ||
| cache.previous_cumulative.size() != |
There was a problem hiding this comment.
🔵 suggestion: kRefreshInterval = 256 is a magic constant with no env override. Under a workload whose routing skew shifts within 256 iterations, the sampled distribution (and thus the wave choice) lags. Acceptable for the stated calibration point, but a brief comment explaining why 256 (vs e.g. 64) was chosen would aid future tuning.
🤖 v3
There was a problem hiding this comment.
Addressed in 5daaaab. The source and ADAPTIVE_WAVE.md now explain that 256 was selected for the stationary B200 calibration to amortize the synchronous D2H copy while periodically detecting distribution changes.
| // The production default remains the strict fallback. Adaptive sizing uses | ||
| // the previous iteration's realized local-expert receive counts, so it is | ||
| // opt-in and never guesses during the first call or after a counter reset. | ||
| if (recv_stats == nullptr or get_env<int>("DG_MEGA_MOE_ADAPTIVE_WAVE", 0) == 0) |
There was a problem hiding this comment.
🔵 suggestion: get_env("DG_MEGA_MOE_ADAPTIVE_WAVE") is read here and again in mega.hpp on every launch. If get_env performs a real getenv each call this is a small redundant cost on the steady-state path; if it is cached this is fine. Worth confirming get_env is cheap.
🤖 v3
There was a problem hiding this comment.
Confirmed that get_env calls getenv. Fixed in 5daaaab by removing the second read from the heuristic: a non-null recv_stats pointer now means the API opt-in shape/band gate and delta validation already succeeded.
| @@ -0,0 +1,88 @@ | |||
| # MegaMoE Adaptive Wave Sizing | |||
There was a problem hiding this comment.
🔵 suggestion: The PR/branch title 'imbalance aware block m' is misleading: the imbalance-aware block_m policy was actually removed, and the calibration override deliberately bypasses the imbalance gate. The delivered feature is 'opt-in adaptive wave sizing'. Consider retitling for changelog/history clarity. Also note the doc says 'EP 8, 256 experts' while the code gate checks num_experts_per_rank==32 and num_ranks==8 (=256 total); consistent but the gate never validates total num_experts directly.
🤖 v3
There was a problem hiding this comment.
The PR title is already updated to Add B200-Calibrated Sparse-Routing Adaptive Wave Sizing. Commit 5daaaab also makes the calibrated gate explicitly require num_experts == 256 in addition to 8 ranks and 32 experts/rank.
🤖 ds-review-bot Code Reviewv6The adaptive cache cannot support multiple MegaMoE counters on one thread and can repeatedly synchronize on zero-token ranks. These issues can disable the feature or materially regress its intended workloads. v4MR review result: imbalance-aware block_m (re-land). This branch was removed from the line in history after B200 A/B measured up to 31% regressions under skew, and the project instead adopted calibrated, narrow adaptive-WAVE sizing (DG_MEGA_MOE_ADAPTIVE_WAVE) which (a) never guesses on the first call/after a counter reset and (b) only enters calibrated tokens/expert bands that actually WON on-GPU A/B. The present re-land restores an OPT-IN (DG_MEGA_MOE_IMBALANCE_AWARE_BLOCK_M) block_m selector (recv_stats host snapshot is already wired in the impl/apis) that PURELY shrinks the tile-M under skew using a theoretical wall-cost/MMA-efficiency model + a broad min(eff, mean_tpe) strict fallback. It is joined at the hip with the wave/pool sizing and the FORCE_BLOCK_M override, but the joint behaviour, env/flag priority, stale-cumulative-stats guard, and perf calibration are all UNTESTED/unharmonized (the config-invariance harness only checks OUTPUT identity, which any block_m trivially satisfies). The core reconstructed logic of the re-land (for reference, in this repo's removed commit, 7 files: heuristics/mega_moe.hpp, apis/mega.hpp, impls/..., IMBALANCE_BLOCK_M.md, scripts/bench_imbalance_block_m.sh, tests/test_imbalance_block_m.py, tests/test_mega_moe.py) is: VERDICT: REQUEST CHANGES / conditional-on-calibration. The opt-in performance claim is unproven on real silicon and the original was delisted for regressions; merge only after a WAVE-style narrow/calibrated re-landing with on-GPU A/B wins and the guards below. v3Despite the branch name 'imbalance aware block m', this PR is a conservative, low-risk change. It removes an earlier imbalance-aware block_m shrink policy (which regressed up to 31% on B200 under skew) and replaces it with: (1) an opt-in adaptive wave-sizing policy (DG_MEGA_MOE_ADAPTIVE_WAVE=1) narrowly gated to a single calibrated shape/token band; (2) benchmark-only overrides (DG_MEGA_MOE_FORCE_BLOCK_M, DG_MEGA_MOE_FORCE_EXPERTS_PER_WAVE) that fail loudly on invalid values; (3) new test modes and A/B benchmarking scripts. The production default path is behaviorally unchanged: the adaptive branch only activates when the env var is set AND host recv stats are available AND the shape/band match; otherwise the refactored (but behavior-identical) upstream logic runs. The cumulative->delta conversion in mega.hpp is handled correctly, guarding against counter resets, pointer changes, and zero-sum deltas. The --validate-config-invariance test asserts bit-exact output and receive-stat equality across default, adaptive, all 6 forced block_m tiers, and all 6 forced wave tiers, which is exactly the right check for a heuristic-only change. I verified the DG_MEGA_MOE_FORCE_BLOCK_M -> fake tokens/expert mapping against the block config table and it is correct for all six tiers (16->8.0, 32->16.0, 64->32.0, 96->64.0, 128->96.0, 192->128.0), each landing in the intended branch. Documentation is honest and detailed, including rejected candidates and win rates. Overall: approve after addressing the thread_local cache aliasing note and considering a title change; no blocking correctness issues found in the production path. Files reviewed: 7 📍 未定位到 diff 的评论🔵 suggestion |
|
Review update:
Upstream compatibility note: #377 merged while this review was in progress. It replaces the old MegaMoE wave scheduler with a dynamic interleaved task scheduler and removes num_experts_per_wave entirely. The PR therefore now conflicts with main. A mechanical rebase would either delete this feature or reintroduce the superseded scheduler, so I have intentionally not forced such a rebase. The adaptive idea would need to be re-evaluated against the new scheduler rather than transplanted unchanged. |
MegaMoE processes a fixed number of local experts in each L1→L2 scheduler wave.
The upstream heuristic derives that number from the mean tokens per expert. That
is a good general fallback, but it cannot see the realized routing distribution.
Set
DG_MEGA_MOE_ADAPTIVE_WAVE=1to enable the opt-in B200 FP8×FP4 policy. The policy:cumulative_local_expert_recv_statsfrom the preceding window;synchronous device-to-host copy on the steady-state path;
intermediate 2048) and
127.5 < expected tokens/expert <= 128.5band;balanced and moderate-skew routing retain the upstream size;
outside the calibrated tier, or when the requested tier exceeds ring capacity.
The deliberately narrow gate is based on same-process, order-balanced 8×B200
measurements. A broader lower bound was rejected after the 96 tokens/expert
point regressed 1.48% under high skew. A balanced-routing wave-12 candidate was
also removed after an independent 6x30 repeat regressed 1.28% with 0/6 wins.
Other broad candidate policies looked promising in process-per-config sweeps,
but did not survive interleaved A/B validation.
Validation
Correctness and configuration invariance:
Robust baseline/adaptive A/B (four measurements per side by default,
alternating order, taking the slowest of all eight ranks in each repetition,
then reporting the median):
B200 performance
The final-source validation used 8×B200, six alternating baseline/adaptive
measurements per case, and 30 profiled kernel calls per measurement. Distributed
latency is reduced as
median_repeat(max_rank(latency)): each repetition firsttakes the slowest of all eight ranks, then the six slowest-rank samples are
reduced by their median.
The common-token sweep used 256 experts, top-k 8, EP 8, hidden 7168, and
intermediate hidden 2048. Values below are observed baseline/adaptive deltas:
Only the 512-token, alpha-1.5 cell changes the production kernel configuration
(upstream wave 16 to adaptive wave 8). Its exact-delivery-source result was
352.642 → 348.225 µs, a 1.268% speedup with 5/6 wins. The same wave-8 branch
was positive across six independent runs and multiple B200 nodes: 0.634%,
1.365%, 2.231%, 1.580%, 2.072%, and 1.268% (median 1.473%).
Every other cell retains the upstream block and wave configuration. Their
approximately -1.23% to +0.93% variation is the measured same-configuration
noise band and is not attributed to the policy. Two broader candidates were
explicitly rejected: 384-token high-skew wave 8 regressed 1.481% with 0/6 wins,
and 512-token balanced wave 12 regressed 1.276% with 0/6 wins in an independent
repeat.
Forced-wave calibration is available separately:
DG_MEGA_MOE_FORCE_EXPERTS_PER_WAVEandDG_MEGA_MOE_FORCE_BLOCK_Marebenchmark-only overrides. They fail loudly on invalid values and are not used by
the production adaptive policy.
co-author with agent.