fix(peft): correct dense MultiLoRALinear on replicated base linears - #25
Merged
yushengsu-thu merged 1 commit intoJul 26, 2026
Merged
Conversation
Found by a GLM-5.2 multi-LoRA e2e smoke (MLA q/kv down-projections are TELinear parallel_mode='duplicated'); Qwen3-style models never exercise this case because all their LoRA targets are TP-parallel linears. Two defects, both specific to replicated bases: * The per-slot token spans covered the full micro-batch, but a replicated base under sequence parallelism consumes the SP shard directly (no gather), so the grouped GEMM read out of bounds — a device-side assert, not a shape error. The spans are now intersected with this rank's contiguous token window (the same invariant the MoE slot routing uses for its SP narrow), with the host-side total cached once per micro-batch in set_tokens_per_adapter_slot so the forward needs no device sync. * _gather_output keyed only on input_is_parallel, leaving the adapter output as a [tokens, out/tp] shard while a replicated base produces the full [tokens, out]. It now mirrors ParallelLinearAdapter's lin_out_gather_output: gather for row-parallel AND replicated bases. The TP collective between the two grouped GEMMs stays unconditional: ParallelLinearAdapter shards A on the rank axis whenever input_is_parallel=False, replicated bases included, so the mid all-gather is required there too. Verified: 74 unit tests pass (4 new for the window narrowing), and a GLM-5.2_5layer multi-LoRA e2e (TP=EP=4, ETP=1, SP, thd, dp-attention rollout) completes 3 training steps with per-step adapter upserts and (tp, pp, ep) checkpoint shards, run with CUDA_LAUNCH_BLOCKING=1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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.
Why
A GLM-5.2_5layer multi-LoRA e2e smoke (the first MLA model through the multi-LoRA path) crashed in the first training forward. The dense
MultiLoRALinearmishandles replicated base linears —TELinear parallel_mode='duplicated', i.e. the MLA q/kv down-projections of GLM-5 / DeepSeek-style models. Qwen3-style models never reach these paths (all their LoRA targets are TP-parallel linears), which is why the existing e2e coverage missed them.This was found on #23's branch and pushed there as
592f5e27, but #23 was merged atf15719c1without it — this PR is that same commit cherry-picked onto the currentbridgetip.Two defects, both specific to replicated bases
Per-slot token spans covered the full micro-batch, but a replicated base under sequence parallelism consumes the SP shard directly (TE stores
parallel_mode=Nonefor duplicated linears, so the adapter attribute derivation disables the SP gather). The grouped GEMM's offsets then exceed the row count — an out-of-bounds read surfacing asCUDA error: device-side assert triggered, not a shape error. The spans are now intersected with this rank's contiguous token window (_narrow_token_counts_to_window, the same invariant the MoE slot routing uses for its SP narrow). The host-side total is cached once per micro-batch inset_tokens_per_adapter_slot, so the forward needs no device sync._gather_outputkeyed only oninput_is_parallel, leaving the adapter output as a[tokens, out/tp]shard while a replicated base produces the full[tokens, out]. It now mirrorsParallelLinearAdapter'slin_out_gather_output: gather for row-parallel and replicated bases.Deliberately unchanged: the TP collective between the two grouped GEMMs stays unconditional.
ParallelLinearAdaptershards A on the rank axis wheneverinput_is_parallel=False— replicated bases included — so the mid all-gather is required there too (gating it onbase_linear_is_parallelwas tried and breaks GEMM2 with a contraction mismatch).Testing
(tp, pp, ep)checkpoint shards at the realized coordinates — executed underCUDA_LAUNCH_BLOCKING=1so no async assert hides.torch._grouped_mmdevice-side assert), reproduced and localized withCUDA_LAUNCH_BLOCKING=1.