fix(autograd/MatMul4Bit): save the packed weight via save_for_backward (#2034) - #2035
Open
Anai-Guo wants to merge 1 commit into
Open
fix(autograd/MatMul4Bit): save the packed weight via save_for_backward (#2034)#2035Anai-Guo wants to merge 1 commit into
Anai-Guo wants to merge 1 commit into
Conversation
bitsandbytes-foundation#2034) MatMul4Bit.forward stored the packed weight as a plain ctx attribute (ctx.tensors = (None, B)) instead of through save_for_backward. In the ordinary case this is harmless -- the weight is a stable Parameter. It breaks for a caller that re-materialises the weight (weight offloading, layer streaming) into a recycled device buffer under gradient checkpointing: torch.utils.checkpoint discards and recomputes *saved* tensors, so the recompute is expected to hand backward a fresh copy of the weight. A raw ctx attribute is invisible to that mechanism -- the reference taken in the original forward survives into backward and, by then, points at a buffer refilled with a different layer, producing silently wrong gradients (forward stays bit-exact). bf16 through the same harness is unaffected because MmBackward0 already uses save_for_backward. Switch to ctx.save_for_backward(B) in forward and read it back from ctx.saved_tensors in backward. The QuantState stays on ctx.state since it is not a tensor. No behavioural change in the ordinary path.
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.
What
MatMul4Bit.forwardsaves the packed 4-bit weight as a plainctxattribute(
ctx.tensors = (None, B)) instead of throughctx.save_for_backward(B), andbackwardreads it back fromctx.tensors. This PR switches tosave_for_backward/ctx.saved_tensors. TheQuantStatestays onctx.statebecause it is not a tensor.
Fixes #2034.
Why
In the ordinary case the
ctx-attribute approach is harmless — the weight is astable
Parameter, so holding a reference to it is exactly as correct as savingit, and costs nothing.
It stops being correct for a caller that re-materialises the weight — weight
offloading, layer streaming, anything that writes a layer's bytes into a recycled
device buffer just before use.
torch.utils.checkpointdiscards and recomputessaved tensors, so under checkpointing such a caller expects the recompute to
hand
backwarda fresh copy of the weight. A plainctxattribute is invisibleto that mechanism: the reference taken during the original forward survives into
backwardand, by then, points at a buffer that has been refilled with adifferent layer. The result is silently wrong gradients — the forward stays
bit-exact and the loss curve looks healthy.
bf16 through the same buffer-recycling harness is unaffected, because it goes
through
MmBackward0, which already usessave_for_backward. This change bringsMatMul4Bitin line with that and with PyTorch's documented mechanism for savingtensors that must survive a checkpoint recompute.
Change
No behavioural change in the ordinary training path (stable
Parameterweight,no in-place mutation between forward and backward → no version-counter interaction).
Testing
The failure only manifests with a CUDA 4-bit weight re-materialised inside a
torch.utils.checkpoint(use_reentrant=False)region, so it requires a GPU. Theissue includes a self-contained reproducer (three arms: recycled-buffer nf4,
private-buffer nf4 reference, recycled-buffer bf16 control) that I did not run
here for lack of the H100/CUDA setup it targets. Happy to add a CUDA-gated
regression test mirroring that harness if you'd like it in-tree.
🤖 Generated with Claude Code