Repository navigation
Support shared-outer expert LoRA in Tinker - #3899
Open
yueming-yuan wants to merge 5 commits into
Open
yueming-yuan wants to merge 5 commits into
yueming-yuan wants to merge 5 commits into
Conversation
yueming-yuan
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy,
guapisolo,
jybsuper,
maocheng23 and
yushengsu-thu
as code owners
October 2, 2026 20:12
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
yueming-yuan
force-pushed
the
yueming/multilora-shared-outer
branch
from
October 9, 2026 22:20
4ba90fe to
04feccc
Compare
yueming-yuan
force-pushed
the
yueming/multilora-shared-outer
branch
from
October 27, 2026 21:53
04feccc to
ada1fd2
Compare
yueming-yuan
force-pushed
the
yueming/multilora-shared-outer
branch
from
October 27, 2026 22:10
ada1fd2 to
bb82e23
Compare
yueming-yuan
changed the base branch from
main
to
yueming/multilora-grad-accumulation
October 27, 2026 22:12
yueming-yuan
force-pushed
the
yueming/multilora-shared-outer
branch
from
October 28, 2026 05:36
ee5d3d7 to
81c3bb2
Compare
yueming-yuan
force-pushed
the
yueming/multilora-shared-outer
branch
2 times, most recently
from
October 28, 2026 05:56
d17e98f to
25e8bb9
Compare
yueming-yuan
force-pushed
the
yueming/multilora-shared-outer
branch
from
October 28, 2026 06:09
25e8bb9 to
1ce16d1
Compare
This branch has not been deployed
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.
Enable
--experts-shared-outer-lorasfor multi-LoRA/Tinker. Each tenant keeps independent factors and Adam state; sampling snapshots use the existing shared-outer adapter format.Record the expert layout in checkpoint metadata and reject restores under the other layout. Checkpoints written before the field existed are per-expert, since multi-LoRA rejected the shared-outer flag until this PR. Boundary: checkpoints saved by earlier heads of this branch (before 4ba90fe) carry no field and load as per-expert too; the H200 validation checkpoints below are the only such files and are scratch.
Stacked on #4480, which carries the per-request gradient accumulation fix this PR originally included (re-syncing retained expert gradients scaled earlier requests by 1/EP). This PR keeps the shared-outer part: the shared factors are replicated across EP, so their gradients are summed across EP once per request (
allreduce_expert_parallel_replicated_grads).Pins Megatron-Bridge to 2d0aa270 in all three Dockerfiles: radixark/Megatron-Bridge#45 as merged, on top of #50's fused-layer TP gradient fixes. The previous pin, 8cd3466d, rejects
experts_shared_outer_lorasfor MultiLoRA. The H200 table below was measured on this branch at bc020cb with Bridge#45 at cd417d03, i.e. without those fixes; it is kept as the pre-fix record and is not validation of the current heads. At the current head,tests/e2e/lora/test_shared_outer_slots.pypasses against Bridge 2d0aa270 on 2× B200 (TP1/EP2), and at Bridge#45's head on TP1/EP2 and TP2/EP2. No Megatron-Core or SGLang changes required. SGLang's fused shared-expert combination remains unsupported.H200 validation: Qwen3.5-35B-A3B, 3 tenants, rank16/alpha32, train TP2/EP4 + inference TP4. Each tenant trained on a distinct fixed 8,192-token DAPO sequence with CE and Adam lr=1e-5 for 50 steps.
Sampling used 512-token prefixes and a resumed step-51 snapshot. All tenants generated successfully. Checkpoint restore matched all 1,920 tensors/tenant exactly, including FP32 masters and Adam state. Repeated-forward logprobs varied even without an update: ~0.02 mean absolute difference after training, and ~0.08 in a separate serial zero-adapter control. Checkpoint tensors are exact; forward determinism has not been established or localized.
Other checks: 36 Bridge GPU cases passed; EP2 and TP2/EP2 integration passed accumulation, norm, isolation, cross-slot restore and resumed Adam. The old accumulation path fails the same test. Pre-commit and all CPU CI shards passed (15,693 tests). GPU CI scope:
run-ci-multi-lora.