Fix for lora dgrad flatten in _linear_dgrad_matmul - #76
Open
williammtan wants to merge 2 commits into
Open
Conversation
williammtan
marked this pull request as ready for review
August 6, 2026 02:26
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.
Restore the frozen-weight dgrad flatten in
_linear_dgrad_matmulProblem
One un-flattened matmul made LoRA 5.3x slower than full fine-tuning.
--lora-rank 0)actor_trainlog_probs(fwd-only control)actor_train_tflopsUnder LoRA every base linear is frozen, so every base linear takes
LinearWithFrozenWeight.backward, which computes its input gradient through_linear_dgrad_matmulwith a baretorch.matmul.That bare
matmulis fine for most 3-D inputs and catastrophic for one particular layout. Two tensors of identical shape and dtype, at the Qwen2.5-0.5B output layer (V=151936,H=896, seq 512,micro_batch=1):is_contiguous()alaid out[s, b, V](151936, 151936, 1)aten::mmcviewed from[b, s, V](151936, 77791232, 1)aten::bmmSame shape, same dtype, both reporting contiguous, 565x apart. The
is_contiguous()flag ignores size-1 dims, so it cannot tell these apart; whatmatmulactually needs is for the leading dims to collapse to one dim by stride, andc's do not.miles hits case
con every step. It calls the model withlabels=None(model.py:313,:505) and computes its own PPO loss, which selects thelogits.transpose(0, 1).contiguous()branch ingpt_model.py:848. The backward of that transpose is another transpose, sograd_outputarrives at the frozen output layer as exactly the strided view above. The resultingbmmtreats sequence as the batch dimension: one matrix-vector product per token, each re-reading the entire weight matrix.Profiling
Full fine tune
LoRA
Fix
Flatten
grad_outputto 2-D whendim() > 2, matmul, restore the shape; fp32 branch preserved. Atmicro_batch = 1the reshape is a view and copies nothing.This is a regression restore, not a new optimisation: upstream already guards this. The fork lost it when the backward was extracted into the fork-only
_linear_dgrad_matmulto addMEGATRON_TRUE_ON_POLICY_LINEAR_DGRAD_FP32.Reproducing
miles/examples/lora/run-qwen2.5-0.5B-megatron-lora.sh, with and without this patch.--lora-rank 0)actor_trainlog_probs(fwd-only control)actor_train_tflopsThe patch removes 62.9 s/step, 79% of the LoRA training step. Unpatched LoRA is 5.3x slower than full fine-tuning; patched it is within 12%.
log_probsis forward-only and flat across all three, localising the regression to the backward pass.Ran with 1x GB10, TP=PP=CP=EP=1,
--num-rollout 3, 64 sequences/step. Medians of steps 1-2.