Fix three memory defects in the Megatron path (selective_log_softmax, causal mask, discarded logits) - #283
Open
Wings-Of-Disaster wants to merge 3 commits into
Conversation
…nning it The per-row loop was documented as 'loop to reduce peak mem consumption', but under autograd every row's full-vocab intermediate is saved for backward, so the loop pins one copy of the vocabulary per row (two when return_entropy is set) until the backward pass finishes. At 16k tokens and a 151936-word vocab that is 4.6 GiB of extra resident activation per micro-batch in bfloat16, plus tens of thousands of tiny allocations and kernel launches. Replace the loop with a chunked autograd.Function that computes in float32 and saves only the original-dtype chunk and its labels, recomputing the softmax in backward. Nothing wider than the labels is retained, and the float32 compute also removes the bfloat16 quantization of the log-probabilities (~0.2 ULP at the magnitudes a peaked row produces).
…builds Two defects combined to make drop_causal_4d_mask dead code: - MegatronModel.forward_backward read attention_mask_type off the model config, but neither mcore TransformerConfig nor mcore_bridge ModelConfig declares that attribute, so the processor always received None and its 'attention_mask_type != causal' guard always returned early. - Even when it did fire, the hook runs four pipeline steps after collate_fn, so the [B, 1, S, S] bool mask had already been allocated (256 MiB per micro-batch at S=16384, roughly twice that in peak with the tril/invert temporaries) only to be set to None. Default the type to 'causal' for decoder-only causal_lm and decide in collate_fn itself, so _create_4d_attention_mask is never called on the NPU/MindSpeed path. The hook stays as a fallback for callers that build the mask elsewhere.
reduce_loss() unconditionally puts logits.detach() into the per-microbatch report dict, forward_backward appends every one of them, concatenates the whole set into a second full-size buffer, and then drops it one line later because return_logits defaults to False. At 16k tokens per micro-batch, two micro-batches and a 151936-word vocab that is an 18.6 GiB peak for bfloat16 logits nothing ever reads, held across the backward pass where memory is tightest. Report the logits only when the caller asked for them, mirroring the guard TransformersModel.forward has had since 6921342, and skip None entries when collecting the report dicts so the same path cannot cat a None. The 'logps' guard matters independently: reduce_loss reports None for it when a microbatch's data processing failed under TWINKLE_FAIL_FAST=0.
Wings-Of-Disaster
force-pushed
the
fix/megatron-memory-leaks
branch
from
September 14, 2026 09:28
2489363 to
dc92445
Compare
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.
PR type
PR information
Three independent defects in the Megatron training path. All three cost device
memory at long sequence lengths; the first one also costs accuracy. Reproduced
on
c839a4ewith a Qwen3-4B GRPO configuration (max_train_len=16384,micro_batch_size=1,mini_batch_size=2,mixed_precision=bf16,vocab
151936).1.
selective_log_softmaxpins one full-vocab copy per rowsrc/twinkle/utils/torch_utils.pyThe per-row loop is commented
loop to reduce peak mem consumption. That holdsunder
no_grad, but not under autograd: every row'slog_softmaxoutput (and,with
return_entropy=True, itsexp) is handed tosave_for_backward, so itstays resident until the backward pass finishes. The loop therefore pins exactly
what it was written to avoid, one full-vocab activation per row.
Replaced with a
torch.autograd.Functionthat chunks by a byte budget, computesin float32, and saves only the original-dtype chunk plus its labels, recomputing
the softmax in backward (the standard activation-checkpointing trade).
Side effect, also a fix: the float32 compute removes the bfloat16 quantization
of the log-probability itself. The old bfloat16 branch returned
F.log_softmax(row)in bfloat16, whose ULP is 0.5 at the magnitudes a peakedrow produces.
Behaviour change: log-probs and entropies are returned in float32 for a
bfloat16 input (previously bfloat16). These are per-token tensors, so the cost
is
N * 4bytes and the values entering the loss are no longer quantized.2.
attention_mask_typeis alwaysNone, so the causal-mask hook is dead codesrc/twinkle/model/megatron/megatron.py,src/twinkle/processor/base.pyMegatronModel.forward_backwardpassedgetattr(unwrapped_model.config, 'attention_mask_type', None)to the processor.Neither
megatron.core.transformer.transformer_config.TransformerConfignormcore_bridge.ModelConfigdeclares that attribute (AttnMaskTypeis only anenum), so the value was
Nonefor every model, andInputProcessor.drop_causal_4d_maskreturned early on itsattention_mask_type != 'causal'guard. The dense mask was always built andalways passed down.
Second defect: that hook is pipeline step 7 while
collate_fnis step 4, so evena working hook ran after
_create_4d_attention_maskhad already allocated[B, 1, S, S].The type now defaults to
'causal'forcausal_lm, andcollate_fndecides upfront, so
_create_4d_attention_maskis never called on that path. The hook iskept as a fallback for callers that build the mask elsewhere. The gate stays
NPU-only (
Platform.device_prefix() == 'npu'), i.e. scoped to the backend thatrebuilds its own compressed causal mask, so CUDA behaviour is unchanged.
3. Full-vocab logits are retained, concatenated, then discarded
src/twinkle/model/megatron/megatron.pyMegatronStrategy.reduce_lossputslogits.detach()into every microbatch'sreport dict unconditionally;
forward_backwardappends all of them,torch.catsthe set into a second full-size buffer, and then throws it away one line later
because
return_logitsdefaults toFalse. The detached references also defeatthe scheduler's own release of each microbatch's
output_tensor, so the wholeset is pinned across the backward pass, where memory is tightest. This hits
forward_onlytoo, i.e. the frozen reference model.TransformersModel.forwardhas guarded the same thing withif not (return_logits or loss_require_logits)since6921342; the Megatronbackend never got the equivalent (
git log -S loss_require_logitsonsrc/twinkle/model/megatron/megatron.pyreturns nothing). This adds it.The report dicts are now collected with
.get(...) is not Noneas well:reduce_lossreportsNonefor bothlogitsandlogpswhen a microbatch'sdata processing failed under
TWINKLE_FAIL_FAST=0, which the previousif 'logps' in loss_dicttest let through intotorch.cat.Experiment results
1.
selective_log_softmax, measured withtorch.autograd.graph.saved_tensors_hooksN=16384 rows, V=151936. "1x" = one full-vocab activation in the input dtype
(4.6 GiB in bf16, 9.3 GiB in fp32). "extra" excludes the caller's own logits
storage, i.e. it is memory this function alone keeps alive until backward ends.
save_for_backwardcallsAccuracy against a float64 row-wise reference (N=512, V=151936):
2. Dense 4D mask
_create_4d_attention_maskat B=1, S=16384 on CPU: 256 MiB mask, +499 MiB peakRSS from the
ones/tril/~temporaries, ~70 ms per micro-batch. Afterthe fix the call never happens on the causal NPU path.
3. Discarded logits
2 microbatches x
[1, 16384, 151936]bf16: 9.3 GiB of detached per-microbatchlogits pinned past their backward, plus a 9.3 GiB
torch.catbuffer, discardedby
if not return_logits: logits = None. 18.6 GiB peak, ~14 GiB of it purewaste.
Tests
New CPU-only tests, each verified to fail on
c839a4eand pass on this branch:tests/utils/test_utils.py::TestSelectiveLogSoftmaxtest_backward_saves_no_extra_full_vocab_activation- fails for bf16 (bothentropy settings) and fp32 + entropy. fp32 without entropy passes on both:
that path already pinned nothing extra, and the test says so rather than
overclaiming.
test_scales_with_chunks_not_with_rows- fails on main (1536 saves vs 2 atN=512, V=512).
test_bfloat16_does_not_quantize_large_negative_logps- fails on main.tests/processor/test_processor.py::TestMegatronCausalMasktest_causal_npu_omits_dense_mask- fails on main (dense mask allocated).for non-NPU backends.
Defect 3 has no CPU-testable unit test:
post_loss_functionis a closure insideforward_backwardand needs an initialised Megatron parallel state. Its evidenceis the code path plus the transformers/megatron asymmetry above.
pytest tests/utils tests/processor tests/loss tests/model tests/advantage tests/metric: 281 passed, 6 skipped. The 5 failures intests/lossandtests/modelare pre-existing and environmental (they allocate on a CUDA devicethat is already full) and fail identically against unmodified
c839a4e.