feat(refit): add ModelExpress Megatron-to-vLLM reshard path - #3632
Open
KavinKrishnan wants to merge 22 commits into
Open
feat(refit): add ModelExpress Megatron-to-vLLM reshard path#3632KavinKrishnan wants to merge 22 commits into
KavinKrishnan wants to merge 22 commits into
Conversation
KavinKrishnan
force-pushed
the
kavink/upstream-mx-megatron-publisher
branch
from
August 13, 2026 21:32
feacdf7 to
537fa3a
Compare
Add the pure Megatron role and shard-geometry helpers needed to describe TP, ETP, and grouped-expert ownership without gathering model weights. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Map native Megatron names and shard geometry to ModelExpress HF aliases, including global expert IDs and fail-closed fused gate/up ordering. Cover the full Qwen3-30B EP8 name set in CPU-only tests. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Remove obsolete v2 and EAGLE-specific material from the extracted helper, document the current ModelExpress seam, and skip integration-only tests when the optional dependency is absent. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Wire ModelExpress's reshard refit path into GRPO as a `refit_transport`
option alongside `nccl_reshard`. The trainer publishes a Megatron reshard
view with HF-named aliases; each vLLM receiver then pulls exactly the
slices its own TP rank needs over NIXL/UCX RDMA, rather than having the
trainer gather and broadcast full tensors.
Publish and receive are strictly sequential and both sit on the critical
path, so `sync_weights` times them separately as
`prepare_for_generation/mx_reshard_{publish,receive}`. Only the receive
half emits MX_REFIT_STAGE records, so without the split the publish cost
is invisible; on Qwen3-30B-A3B it is the larger of the two by roughly 3x.
Version ordering is deliberate: every trainer publishes version N before
any receiver begins pulling it, so a receiver can never observe a fleet
where some trainers still advertise N-1.
Validated on 32 GPUs (GB200, RoCE) against Qwen3-30B-A3B-Instruct
(TP2/EP4/ETP1 trainer, four TP4 vLLM replicas) and Qwen3-4B: 100%
coverage, 0 fallback, 11 consecutive refits, and `gen_kl_error` matching
the pre-existing transport on the dense model.
Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Move the training call into try/finally so cluster and generation shutdown run even when training raises. Previously an exception skipped teardown, leaving NIXL agents with registered memory and Ray actors alive; the process then aborted during interpreter shutdown and buried the original traceback under an unrelated fatal error. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
The functional test gates on `max(train/token_mult_prob_error) < 1.05`,
matching the existing nccl_reshard test, so a refit that installs wrong
or stale weights fails rather than merely running.
The bench configs cover the GB200/RoCE topologies used to validate this
transport. Two environment settings there are load-bearing and easy to
lose:
- UCX_TLS excludes `ud` as well as `tcp`. With a live NCCL process group
in the same process, destroying a NIXL agent aborts in ud_iface.c
("unable to remove iface timer handler"); rdmacm handles wireup, so
dropping UD costs nothing measurable.
- NCCL_CROSS_NIC=0. Each node holds an address on all four RDMA rails,
and NCCL will otherwise pair a local rail-0 HCA against a peer's
rail-3 address. Those are different subnets, so RoCE never connects and
the queue pair dies with IBV_WC_RETRY_EXC_ERR.
`megatron_cfg.checkpoint.async_save` is off: these recipes never write a
checkpoint, and the persistent async-checkpoint worker's
multiprocessing.Manager fork intermittently dies during worker init,
killing the run with an EOFError. It cost 2 of the first 5 MoE runs.
Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
The refit's dominant cost was invisible. MX_REFIT_STAGE records cover only the install, so MX telemetry reported 1.9 s of a 10.25 s refit and looked healthy. Emitting discover_s alongside mx_update_s shows the quorum check is 5.20 s, or 51% of the refit and 2.7x the actual weight transfer. It re-fetches and re-parses 78760 tensor entries from 16 trainer sources on every refit: one list_sources plus a get_metadata round-trip per rank, each returning that rank's whole shard table. None of it changes between steps; only publisher_step does. The cost scales with source count rather than bytes, which is why a 30B MoE with 18432 expert tensors exposed it and a dense 4B (~6400 entries, ~0.4 s) did not. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
The phase split read payload.tensors directly, so any rendezvous payload without a shard table raised AttributeError *after* the weights had already installed, turning a successful refit into a failed one. Reporting must not be able to break the operation it measures. Missing shard tables now count as zero and the emit is guarded, with regression tests for both the absent-field case and the counting itself. Caught by the unit suite, which had never been run against these changes. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
The receiver's per-step version check does not read shard geometry; MX discovers that once in _prepare and keeps it. Requesting the tables anyway rebuilt 78,760 entries across 16 ranks on every refit. Falls back to the full fetch against an MX predating the flag, so the two changes need not land together, and the entry count now comes from the payload's own tally so the metric that exposed this cost does not read zero once the tables are skipped. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
gen_kl_error is a k3 estimator, and the benchmark recipe feeds it at most 128 generated tokens per step, where one outlier token moves it a long way. This raises max_new_tokens 8 -> 128 and changes nothing else, so batch sizes and the refit path are untouched. The result was negative and worth keeping: the estimate did not fall below the 1e-3 guideline, it stabilized at ~1.5e-3 with the spread narrowing from 5.5x to 2.6x. So the exceedance is real rather than sampling noise, and the no-refit reference is still needed to attribute it. See doc 21. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Every other refit transport can check the weights it moved: SGLang has check_weights(compare), the sparse transports emit delta_verify/*. mx_reshard had only end-to-end logprob metrics, which conflate refit fidelity with Megatron-vs-vLLM implementation divergence. MX_REFIT_VERIFY=1 fingerprints every vLLM parameter before and after each install and emits MX_REFIT_VERIFY naming what changed. Fingerprints are two allocation-free int64 reductions over the raw bytes, so they catch a single flipped mantissa bit, where a float statistic can miss one; retaining a pre-refit copy would instead cost ~15 GB per rank. Off by default, since it sits on the refit critical path, and it cannot fail the refit it verifies. Running it found something that matters more than the feature: the benchmark recipes never train. max_new_tokens=8 means no generation can be correct, so every reward is 0, the leave-one-out baseline makes every advantage 0, and Loss is 0.0000 at every step. Confirmed against lr=1e-2, where nothing changed either. So the first refit changes all 435 params and every later refit transports byte-identical weights. Performance rows are unaffected, but from step 2 on the correctness gates cannot tell a working refit from a no-op, because the correct answer is "no change". See doc 22. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
grpo_math_1B_megatron.yaml has no refit_cfg subtree, so Hydra refused the plain `=` overrides for refit_transport and mx_reshard.server_url and the gate died during config parsing, before training. Switch both to `++`. Earlier validation replayed archived TensorBoard data through check_metrics.py, which exercised the assertion but never the launch. Running the gate live on 2 GPUs now reaches the end and passes at token_mult_prob_error 1.0154 < 1.05. Also adds topoA_correctness.yaml, a correctness arm that generates 512 tokens with 4 generations per prompt so rewards can differ within a group. The performance recipes generate 8 tokens, score every sample 0, and never train, which made every post-first refit gate vacuous. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
The ratio check alone passes on a run that never trains. GRPO's leave-one-out baseline zeroes the advantage of any prompt group whose rewards are all equal, so if no group is mixed the second refit re-sends bit-identical weights and `token_mult_prob_error < 1.05` is satisfied by a no-op (doc 22). Assert `max(train/grad_norm) > 0` so the ratio check is only credited on a model that moved, and widen the reward sample from 2x4 to 4x8 so a zero-gradient run is rare rather than a coin flip. The previous shape solved 1 of 8 sequences, and that single sequence produced the entire gradient. Verified against the archived metrics of a live gate run: the assertion passes at grad_norm 5.21, and a negative control with grad_norm zeroed fails the new check while still passing the ratio check, which is the vacuity it is meant to catch. The widened batch shape itself is not yet confirmed on a live run; cluster capacity was reclaimed mid-validation. Also adds a single-node smoke config that runs the gate twice, once BF16 and once with a vLLM MXFP8 rollout, to exercise FP8 on this path for the first time. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
…lure Two things learned by running it. vLLM FP8 generation asserts use_importance_sampling_correction in grpo.py, so without that override the run dies in setup before any refit. With it, the arm reaches the first refit, the transport completes cleanly (898 descriptors, 0 full-pull sources), and the install raises "Cannot copy out of meta tensor" inside ModelExpress's quantized commit path. That is an MX-side bug, so the arm is kept as its reproducer rather than removed. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Four steps proved weights move and the gate holds while they do, but cannot say whether per-refit error accumulates, whether gen_kl_error keeps creeping past ~1.4e-3, or whether discover_s stays flat across many refits. Same 512-token, 4x4 shape as the 4-step arm, twelve steps. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
gen_kl_error mixes runtime divergence with refit error, and only the sum was ever measured. This arm separates them by having vLLM hold the real checkpoint before the first refit, so both sides provably hold identical weights. Result: the first refit changes 0 of 435 parameters on all 16 receiver ranks, so the refit is bit-exact and gen_kl_error at that point is pure Megatron-vs-vLLM divergence: 8.7e-4 and 1.3e-3. The documented < 1e-3 gate therefore sits below the model's own floor, and no refit can pass it. See doc 27. The arm needs a diagnostic escape hatch for load_format, kept out of this branch and archived alongside the evidence, because NeMo-RL forces load_format=dummy whenever a refit transport is set. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
The CI gate had no KL-family assertion, and the obvious candidate, an absolute gen_kl_error < 1e-3, is not safe to add: that metric has a non-zero floor set by Megatron-vs-vLLM kernel differences which grows with model size. Measured with a refit that provably changed no parameter, the floor is 8.7e-4 to 1.3e-3 on Qwen3-30B-A3B, so the bound is below the floor and unpassable there regardless of refit correctness. js_divergence_error is bounded and symmetric, and a single 1e-3 bound holds across every scale measured: 1.6e-4 on 0.6B, 1.3e-4 on 4B dense, 5.0e-4 on 30B MoE. Validated offline against both archived gate runs (pass at 1.5e-4 and 1.6e-4) and against a negative control with the metric inflated to 1.1e-3 (fails). See doc 27. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
summarize_refit_stages.py answers how fast a run was; it cannot say why two identical runs differ. This adds the other cut: per-stage spread across runs, and whether the slowest rank is the same one each time, which separates placement from contention. It deliberately reports stages for the rank that was fleet-critical on each step rather than the median across ranks, because those disagree. On the dense arm the all-rank median of wire_fused_s moves 1.90x between two cold starts while the fleet-critical total moves only 1.03x: the body of the distribution shifts but the tail sets the refit duration. Only the critical rank is on the critical path, so attributing from all-rank medians would point at a stage that is not the problem. topoA_variance.yaml runs the MoE arm at the full 11 steps for doc 19 item 8. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Resolve QKV heads from each live Megatron layer so heterogeneous attention and KV-heads-below-TP layouts reach ModelExpress without zero local KV counts. Keep the root config only as a validated compatibility fallback. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Retain only portable recipes and analysis tools; namespace- and private-image- specific validation manifests remain internal evidence rather than upstream API. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Keep Megatron generation on its native synchronizer while ensuring non-colocated MX and NCCL reshard paths skip the legacy HF refit-info handshake. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
KavinKrishnan
force-pushed
the
kavink/upstream-mx-megatron-publisher
branch
from
August 18, 2026 19:15
9c52bb6 to
6a49789
Compare
KavinKrishnan
marked this pull request as ready for review
August 18, 2026 20:07
The PR description promises portable recipes with no private cluster detail, but three headers still carried a colleague's name, an internal design-doc filename, internal cluster state, and a stale claim that fused grouped-expert install is unimplemented. That claim is no longer true once ModelExpress NVIDIA-NeMo#635 lands its fused MoE capture fix, so a reader would have been told the dense recipe works around a gap that no longer exists. Keep the technical rationale each header carried -- geometry, why a dense arm isolates non-expert resharding, why the KL sample recipe raises max_new_tokens -- and drop the parts that only mean something inside the originating cluster. Also replace the "NVIDIA-NeMo#496" ModelExpress PR reference in canonicalize_grouped_expert_name with a description of the behaviour it depends on, since that number resolves to nothing for a NeMo-RL reader. Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
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.
What this PR does
This PR adds the complete NeMo-RL side of the ModelExpress
mx_reshardrefit path:Each part has a narrow job in making trainer-to-generator refits usable from NeMo-RL:
WeightSynchronizernow starts publishers and receivers, waits for all required ranks, runs the transfer, commits the new version, and shuts both sides down in a safe order. Callers do not need a separate ModelExpress-only control path.linear_qkvmodule avoids2 // 8 = 0and prevents one root-model setting from being incorrectly reused for every layer.This PR depends on ModelExpress #635, which carries the global QKV interval alias builder the new QKV contract requires. Neither half is useful alone: NeMo-RL publishes the geometry, ModelExpress maps the intervals.
Architecture
flowchart LR M[Live Megatron layer] --> G[Per-layer Q/KV geometry resolver] G --> C[Parameter classifier] C --> B[Megatron-Bridge name map] B --> P[ModelExpress publisher] P --> Q[Publisher quorum] Q --> R[vLLM reshard receiver] R --> I[Install and verify] I --> V[Commit one version] W[WeightSynchronizer] --> P W --> Q W --> R W --> V W --> S[Ordered receiver/publisher shutdown]For Q=64, KV=2 and trainer TP8, NeMo-RL now publishes global
num_heads,num_kv_heads,head_dim, andqkv_interleavefrom each livelinear_qkv.config. It does not publishnum_kv_heads_local=0, and it does not pretend every rank owns a replicated KV head. ModelExpress receives each rank's real fused-row range and performs the interval mapping.Why per-layer geometry matters
Nemotron-family models may use different attention geometry in different layers. Reading one root model config would stamp the same Q/KV shape onto every QKV tensor. The resolver therefore walks to the owning live
linear_qkvmodule first and uses the root values only as a compatibility fallback. The chosen geometry must match the tensor's global fused-row count or publication fails closed.Review guide
1. QKV/KV-heads-below-TP contract
Commit:
83c3d1324Review first:
nemo_rl/distributed/mx_megatron_helpers.pynemo_rl/models/policy/workers/megatron_policy_worker.pynemo_rl/weight_sync/mx_reshard_weight_synchronizer.pyPlease check that layer-local config wins, local head fields are emitted only when both counts divide by TP, fused rows are validated, and only the obsolete
num_query_groups % TPguard was relaxed.2. Megatron publisher foundation
Commits:
23b90cf8,eea2ad6f,537fa3adReview parameter role classification, global expert IDs, Megatron-Bridge name order, TP/ETP ranges, and fail-closed handling of unknown or ambiguous fused layouts.
3. Weight-sync and vLLM lifecycle
Review:
mx_reshard_weight_synchronizer.pymx_reshard_publisher.pymx_vllm_reshard_receiver.pymx_reshard_config.pyThe important ordering is publish all trainer ranks, establish receiver quorum, pull/install on all receiver ranks, then commit. Any failed rank prevents version commit. Shutdown releases receivers before publishers so NIXL registrations remain valid until reads finish.
4. Reliability and observability
Review cleanup after exceptions, telemetry isolation, separate quorum timing, and parameter verification in
mx_refit_verify.py. Telemetry must never turn a successful refit into a failed one.5. Functional gate and portable tools
Review
tests/functional/grpo_mx_reshard_refit.sh, the three portable recipe configs,summarize_refit_stages.py, andattribute_variance.py.The gate intentionally uses
js_divergence_errorrather than an absolutegen_kl_errorceiling; Megatron and vLLM kernels have a non-zero baseline KL even when every parameter is identical. It also requires a non-zero gradient so an unchanged model cannot pass vacuously.Private namespace/image-specific cluster manifests are intentionally not included.
Evidence and scope
The pre-GQA integration was validated at 32 GPUs (16 Megatron trainer GPUs and 16 vLLM receiver GPUs) together with the original six ModelExpress #635 commits:
The new KV<TP contract is currently unit-qualified and representative CUDA-tensor-qualified with ModelExpress #635. Q=64/KV=2/head_dim=128 logical TP8 passed exact Q/K/V reconstruction, same-weight parity, changed-weight parity, and projection-output equality. This is not yet a full Megatron-to-vLLM TP8 E2E result and is not full Nemotron Ultra qualification.
Test plan
mx_reshardsynchronizer suite — 14 passed6a4978963