Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix doubled lm_head gradient in Unsloth_Offloaded_Log_Softmax backward#892

Open
danielhanchen wants to merge 4 commits into
mainunslothai/unsloth-zoo:mainfrom
fix/offloaded-logsoftmax-gradunslothai/unsloth-zoo:fix/offloaded-logsoftmax-gradCopy head branch name to clipboard
Open

Fix doubled lm_head gradient in Unsloth_Offloaded_Log_Softmax backward#892
danielhanchen wants to merge 4 commits into
mainunslothai/unsloth-zoo:mainfrom
fix/offloaded-logsoftmax-gradunslothai/unsloth-zoo:fix/offloaded-logsoftmax-gradCopy head branch name to clipboard

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

What

Unsloth_Offloaded_Log_Softmax (the CPU-offloading / checkpointed log-softmax used for the large / long-completion GRPO logprob path via efficient_log_softmax) recomputes its output in backward, then did:

torch.autograd.backward(output, grad_output)
return (hidden_states.grad, lm_head.grad if ctx.lm_head_requires_grad else None, ...)

torch.autograd.backward writes into the leaf .grad attributes. Returning lm_head.grad as this op's gradient contribution then gets accumulated again by the outer engine's AccumulateGrad, so:

  • lm_head gradient is 2x too large for a single use,
  • more when lm_head is shared across chunks (the normal GRPO path -- the same output embedding feeds every compute_logprobs_chunk call), and
  • any gradient already accumulated on lm_head from a previous micro-batch is corrupted (returned as 2*pre + 2*g instead of pre + g).

Fix

Use torch.autograd.grad(output, inputs, grad_output), which returns each input's local gradient contribution without touching leaf .grad. hidden_states.grad was already correct (its reloaded tensor is a private leaf) and is unchanged.

Reachability

Only on the offload path (index.shape[1] > 1024 or batch_size > 8, i.e. long-completion / large-batch GRPO) and with a trainable lm_head / tied embeddings (full fine-tuning, or LoRA with modules_to_save=["lm_head"]). Standard LoRA freezes lm_head, so the old path returned None and was unaffected.

Verification

Reproduced against the non-offloaded twin chunked_hidden_states_selective_log_softmax as oracle: before the fix, |lm_head.grad| / |ref| was exactly 2.0 (single use) and 3.0 (two shared uses), with pre-existing grad corrupted to 2*pre + 2*g; after the fix all cases match the oracle exactly (ratio 1.0, max abs diff 0.0), and hidden_states.grad stays exact. Added a CPU regression test that pins the pattern (fixed matches plain-autograd oracle for single / shared / pre-existing-grad; the old pattern provably diverges) plus a source guard.

The offloaded log-softmax Function recomputes its output in backward, then called
torch.autograd.backward(output, grad_output) and returned lm_head.grad. That writes the
leaf .grad and returns it as the op's gradient contribution, so the outer engine's
AccumulateGrad adds it again: lm_head gradients came out 2x too large for a single use,
and more when lm_head is shared across chunks (the normal GRPO path), and any grad already
accumulated from a previous micro-batch was corrupted. Switch to torch.autograd.grad, which
returns each input's local gradient without touching leaf .grad. hidden_states.grad was
already correct (its reloaded tensor is a private leaf) and is unchanged. Only reachable on
the offload path (long-completion / large-batch GRPO) with a trainable lm_head or tied
embeddings; frozen lm_head (standard LoRA) returned None and was unaffected.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the use of torch.autograd.backward with torch.autograd.grad in the backward pass of Unsloth_Offloaded_Log_Softmax to correctly compute and return local gradient contributions, preventing gradient double-counting on lm_head. It also adds comprehensive unit tests to verify this behavior. The reviewer recommends adding a check to handle cases where grad_output is None in the backward method to prevent potential runtime errors.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +1397 to +1401
grad_inputs = torch.autograd.grad(
output,
(hidden_states, lm_head) if ctx.lm_head_requires_grad else (hidden_states,),
grad_output,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In PyTorch custom autograd functions, always check if the incoming gradient tensor (grad_output) is None in the backward method and return early if so. This prevents runtime AttributeError or other errors when downstream layers or base models are frozen and do not require gradients.

            if grad_output is None:
                return (None, None, None, None, None, None, None, None)

            grad_inputs = torch.autograd.grad(
                output,
                (hidden_states, lm_head) if ctx.lm_head_requires_grad else (hidden_states,),
                grad_output,
            )
References
  1. In PyTorch custom autograd functions, always check if the incoming gradient tensor is None in the backward method and return early if so. This prevents runtime AttributeError when downstream layers or base models are frozen and do not require gradients.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Function keeps the default materialize_grads=True (it never calls ctx.set_materialize_grads(False)), so PyTorch materializes any unused-output gradient as a zero tensor before calling backward. grad_output is therefore never None here, and the single output (the per-token logprobs) always feeds the loss regardless. A None guard would be unreachable dead code, so leaving it out.

Optimize the CPU-offloading log-softmax Function used by the large-batch or
long-completion GRPO logprob path, on top of the local-input-grads fix:

- Offload hidden_states through a pinned host buffer on a side CUDA stream so
  the D2H copy runs asynchronously and overlaps the no_grad forward; backward
  waits on the recorded copy event before the H2D reload, which is required
  for correctness with pinned non_blocking copies.
- Skip the CPU round-trip entirely when the GPU has generous headroom (free
  memory at least 4x the tensor size); the detached alias adds no allocation
  and the big logits are still checkpointed via recompute.
- Drop the dead post-reload dtype cast: the saved tensor already keeps the
  forward dtype.
- Fall back to the original pageable copy on CPU tensors, missing CUDA, or
  pinned allocation failure, keeping Windows/Mac/CPU behavior unchanged.

Outputs and gradients are bitwise identical to the previous implementation
for both frozen and trainable lm_head, and fwd+bwd time on
B=16,T=2048,H=2048,V=32000 bf16 with 256 chunks is equal or slightly better
in all measured cases. Add CPU tests pinning the fallback numerics and the
event-sync and is_cuda-guard source invariants.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Jul 9, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Jul 9, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

1 similar comment
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 84842eee74

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen

Copy link
Copy Markdown
Member Author

Commit 5c9f364 is a comment-only cleanup (verified AST-identical code, tests unchanged at 18 passing).

@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c9f3646a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread unsloth_zoo/rl_replacements.py Outdated
Comment on lines +1367 to +1369
if tensor_bytes * 4 <= free_bytes:
# Enough headroom: keep the alias on GPU (mem_get_info failure defaults to offload).
saved_hidden_states = detached_hidden_states

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep offloaded hidden states off GPU between chunks

In the padded GRPO path with multiple zipped_inputs chunks, this branch retains each saved hidden-state tensor on CUDA until backward. At the call site the input is a [:, :-1, :] slice, so detached_hidden_states = hidden_states.detach().contiguous() has usually just allocated a full contiguous GPU copy; keeping it here means those copies accumulate across chunks instead of being released after the CPU offload as before. Long-completion/large-batch runs that relied on this offloaded path to fit can now OOM even when each individual tensor satisfies the 4x-free-memory check, because the recomputed logits still need additional temporary memory. Please keep this path offloaded, or make the GPU-retention path opt-in with a check that accounts for all retained chunks and recompute temporaries.

Useful? React with 👍 / 👎.

@danielhanchen danielhanchen Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Fixed in 05d9c63: the keep decision now budgets cumulatively, 4 * (tensor_bytes + retained_this_step) <= free_bytes, capping retention at a quarter of measured free memory with at least three quarters left for the backward recompute. The counter lives in grpo_accumulated_loss scope and is recreated on every call, so no state can leak across steps. Verified with a 16 chunk retention test (old head kept 11 of 16 chunks, fixed head kept 3, all gradients bitwise exact) and the keep-on-GPU perf win is unchanged within noise.

The padded GRPO path calls Unsloth_Offloaded_Log_Softmax once per
zipped_inputs chunk before any backward runs. The per-call check
tensor_bytes * 4 <= free_bytes only sees current free memory, so with N
chunks each sized at the threshold the retained total approaches
free * (1 - (3/4)^N), i.e. nearly all initially free memory, starving
backward's logit recompute.

Track bytes kept this step in a function-local counter and require
4 * (tensor_bytes + retained) <= free_bytes, capping retention at 1/4 of
measured free memory. The counter is recreated on every
grpo_accumulated_loss call, so no state can leak across steps, eval
passes, or exceptions. Single-chunk behavior is unchanged.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 05d9c63ebc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Morty Proxy This is a proxified and sanitized view of the page, visit original site.