Skip to content

peft: fix multi-LoRA activation-recompute replay and grouped-expert adapter DDP flagging - #27

Merged
yushengsu-thu merged 2 commits into
bridgefrom
fix/multi-lora-recompute-and-expert-ddp
Aug 13, 2026
Merged

peft: fix multi-LoRA activation-recompute replay and grouped-expert adapter DDP flagging#27
yushengsu-thu merged 2 commits into
bridgefrom
fix/multi-lora-recompute-and-expert-ddp

Conversation

@yushengsu-thu

Copy link
Copy Markdown
Collaborator

Two one-line-class fixes for silent multi-LoRA training failures on MoE models, found by GPU triage on the miles tinker stack (GPT-OSS 20B, expert-only LoRA, 4xH200).

Fix 1 — activation-recompute replay never happens for multi-LoRA (peft/recompute.py)

maybe_enable_recompute_inputs_grad ships the input-grad mechanism that makes activation-checkpoint replay work for frozen-base PEFT (a checkpointed region is only replayed grad-enabled when its input requires grad). Its name matching only recognized single-LoRA params (.adapter.); multi-LoRA slot params live in an adapters nn.ModuleList (.adapters.<slot>.) and were classified as trainable base weights, so the patch was skipped. Under --recompute-granularity full (and selective moe with expert-only targets) the checkpointed region gets grad-free inputs, CheckpointFunction.backward is never invoked, and every adapter gradient is identically zero — silent no-op training at a truthful grad_norm=0.0.

Fix: extend the matching to .adapters. via a shared helper. Single-LoRA classification is byte-identical.

Fix 2 — grouped-expert adapter grads never reduced across TP (peft/utils.py)

GroupedExpertLinearAdapter.__init__ set allreduce = not (EP > 1) and never set tensor_model_parallel. With EP=1, ETP=1, TP>1 the TP-duplicated adapter weights land in the dense DDP bucket (reduced over dp_cp only), so cross-TP gradient reduction never happens: rank>0 grads stay identically zero and the TP replicas of the adapter silently diverge from step 1.

Fix: allreduce=False unconditionally. Expert adapter weights are sharded over ETP (not TP), so their grads must be reduced over the expert data-parallel group, which folds in the TP peers whenever ETP < TP; when ETP == TP the expert-DP group equals dp_cp, so previously-working configs (including EP > 1) are unchanged. Comment states the invariant.

GPU evidence (4xH200, GPT-OSS 20B bf16 expert-only LoRA via miles tinker stack @ tinker-frontend d5bb5e062; TP=2+SP, EP=1, ETP=1, 2 adapters)

Check before (bridge @ 3fc31b0) after (this branch)
--recompute-granularity full: SDK grad_norm per step 0.0 on every step, both adapters (silent no-op) 45.07 / 29.13 (rank-8 adapter), 119.21 / 30.08 (rank-16)
--recompute-granularity full: trainer logprob max-abs delta after optim_step no real learning signal 1.01 / 2.25 and 2.45 / 1.36
adapter forward grad-enabled during checkpoint replay never yes — e.g. 96 grad-enabled wrapper forwards per step (replay pass) vs 96 no-grad (checkpoint pass)
TP=2 adapter main_grad across ranks (sha256, post-reduce pre-step) rank1 identically zero on all 96 params, every step; replicas fully divergent (0/96 hashes match by step 2) bit-identical: 96/96 param hashes match on every optim step, both slots, across all runs (17 optim calls total)
poison-window 5-phase GPU acceptance (tinker_sdk_poison_window.py) strict probe-determinism precondition already unmet on this MoE config: back-to-back forwards differ by 0.16-0.21 max-abs-dlogprob with NO optim step (base bridge, probe_stability diagnostic) same pre-existing forward nondeterminism (0.09-0.14 — not a regression); all 5 phases PASS with every mechanism assertion kept and probe/grad-norm tolerances sized to the measured noise: typed rejects in 0.1s, clocks held, discard leaves no residue (recovery grad_norm 157.53 vs ref 157.96 and 151.12 vs 151.19), isolation clean, late-chunk discard held

Logs and dumps persisted on cluster h200-sci-k8s under /personal/bridge-fix-0812/ (prior failing-run evidence: /personal/triage-positive{,2,3}, passing no-recompute baseline: /personal/triage-positive4).

Tests

  • tests/unit_tests/peft/test_recompute.py: multi-LoRA (.adapters.<i>.) params are recognized (regression: patch used to be skipped); trainable-base early-out still holds; existing single-LoRA test unchanged.
  • tests/unit_tests/peft/test_utils.py: GroupedExpertLinearAdapter weights always carry allreduce=False + correct partition_dim/partition_stride (parametrized over input_is_parallel), with EP=1 in the config — the exact regression scenario.
  • Full tests/unit_tests/peft/ on the H200 box: 362 passed, 9 pre-existing failures identical at base commit 3fc31b0 (env-related, files untouched by this PR).

Note for miles

miles' launch-time guard refusing full recompute (radixark/miles#2273, commit c0def7864) was added assuming an unfixed bridge; it can be relaxed to a bridge-version check once this merges. The guard is not yet on tinker-frontend, so validation ran unmodified miles @ d5bb5e062.

Comment thread src/megatron/bridge/peft/recompute.py Outdated
…d patch

maybe_enable_recompute_inputs_grad only matched single-LoRA names
(.adapter.), so multi-LoRA slot params (.adapters.<slot>.) were
classified as trainable base weights and the TransformerBlock
input-grad patch was skipped. Under --recompute-granularity full (and
selective moe with expert-only targets) the checkpointed region then
received grad-free inputs, CheckpointFunction.backward was never
invoked, and every adapter grad stayed identically zero: training was
a silent no-op.

Extend the name matching to .adapters. via a shared helper; single-LoRA
classification is byte-identical. Add regression tests for the
multi-LoRA naming and for the trainable-base early-out.

Signed-off-by: Ethan (Yusheng) Su <yushengsu.thu@gmail.com>
@yushengsu-thu
yushengsu-thu force-pushed the fix/multi-lora-recompute-and-expert-ddp branch from 659c327 to 6a1d514 Compare August 13, 2026 05:15
… groups

Port the upstream (NVIDIA-NeMo/Megatron-Bridge main) fix: the adapter
weights' allreduce flag now follows use_expert_process_groups (EP > 1 or
ETP != TP) instead of EP size alone, and the TP attributes are set via
set_tensor_model_parallel_attributes. The old EP-only condition left
EP=1, TP>1 runs in the dense DDP bucket, so the TP-duplicated adapter
gradients were never reduced across TP peers and replicas silently
diverged (rank1 grads identically zero). GPU-verified on GPT-OSS 20B
expert-only LoRA (TP=2, ETP=1): per-step adapter main_grads bit-identical
across ranks with the fix.
@yushengsu-thu
yushengsu-thu force-pushed the fix/multi-lora-recompute-and-expert-ddp branch from 6a1d514 to afd22d6 Compare August 13, 2026 05:25
@yushengsu-thu

Copy link
Copy Markdown
Collaborator Author

Reworked the utils.py fix per request (afd22d6): the unconditional allreduce=False is reverted and replaced with a port of the upstream (NVIDIA-NeMo/Megatron-Bridge main) form — allreduce = not use_expert_process_groups where use_expert_process_groups = EP > 1 or ETP != TP, plus set_tensor_model_parallel_attributes(weight, True, tp_axis, 1) (which also sets tensor_model_parallel). Outcomes are identical in every configuration (when the condition is false, the expert-DP group equals dp_cp), so the existing GPU evidence (EP=1, ETP=1, TP=2 → allreduce=False → bit-identical cross-rank grads) remains valid. The regression test now covers both sides of the condition: ETP≠TP → expert bucket; ETP==TP → dense (equivalent groups).

@yushengsu-thu
yushengsu-thu merged commit 688d34b into bridge Aug 13, 2026
3 checks passed
yushengsu-thu added a commit to radixark/miles that referenced this pull request Aug 13, 2026
The launch guard added in c0def78 refused --recompute-granularity full
(and selective 'moe' with expert-only targets) unconditionally, because the
Megatron-Bridge PEFT recompute patch of the day matched only single-LoRA
.adapter. names: multi-LoRA .adapters.<slot>. params were classified as
trainable base weights, the TransformerBlock input-grad hook was skipped,
checkpointed layers never replayed grad-enabled, and every adapter gradient
was silently zero at a truthful grad_norm=0.0.

That bridge bug is fixed (radixark/Megatron-Bridge#27, branch bridge @
688d34b8: .adapters. recognition in maybe_enable_recompute_inputs_grad), so
an unconditional refusal now blocks a legitimate memory saver on fixed
deployments. Make the guard conditional: probe the installed bridge's
maybe_enable_recompute_inputs_grad source for the multi-LoRA marker and
refuse the two risky shapes only when the probe reports an unfixed bridge
(unimportable/unreadable bridges fail closed). Source inspection over a
behavioral probe keeps arg validation free of model construction; the error
messages keep pointing at the exact bridge fix. Selective shapes never touch
the probe. Tests pin both guard directions, the never-probed shapes, and the
probe itself against file-backed fixed/unfixed stand-ins; the tinker README
documents the bridge requirement instead of a blanket refusal.
yushengsu-thu added a commit to radixark/miles that referenced this pull request Aug 13, 2026
… grad norm

Found by the fixed-bridge GPU re-validation (4xH200 GPT-OSS 20B expert-only
LoRA, TP=2+SP, EP=1/ETP=1, full recompute): every reported per-slot grad_norm
came out exactly sqrt(2)x the true gradient norm (diag: reduced norm
247.1967 vs local l2 174.7945, ratio 1.41421 on every optim of every slot),
and grad_clip_norm under-scaled by the same factor (post-clip l2 0.7071 for
clip=1.0).

Mechanism: the bridge's grouped-expert adapter weights carry
tensor_model_parallel=True unconditionally (upstream-ported attribute
stamping in radixark/Megatron-Bridge#27). The only supported multi-LoRA MoE
config is expert_tensor_parallel_size=1, where those weights are fully
TP-DUPLICATED whenever TP>1 — so Megatron's attribute-based TP-duplicate
filter admits every rank's identical gradient into the world-reduced norm and
over-counts each logical parameter TP times. This was unobservable before the
bridge fix only because rank1's expert-adapter gradients were identically
zero (the expert-DDP routing bug); once #27 made them real, the double-count
became real too.

Fix at the existing pre-wrap seam: after the LoRA transform, clear
tensor_model_parallel on grouped-expert adapter weights when TP >
expert-TP (the duplicated case), so the stock filter counts each logical
param once (TP rank 0) — semantically identical to run-E of the 0812 matrix,
which was GPU-verified with true norms and synced ranks. DDP expert-bucket
routing keys on 'allreduce' and is untouched; the hook runs pre-wrap so the
fp32 masters copy the corrected attribute at optimizer build. Genuinely
TP-sharded (attention) adapters keep their flag. CPU regression tests pin
the cleared/kept/no-op shapes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant