Stop an older unsloth_zoo turning a fully masked microbatch into NaN - #10892
Stop an older unsloth_zoo turning a fully masked microbatch into NaN#10892danielhanchen wants to merge 6 commits into
Conversation
unsloth_zoo's chunked fused cross entropy computes
divisor = n_items if n_items is not None else (labels != ignore_index).sum()
so a microbatch with no trainable labels and no caller-supplied n_items divides by
zero. Loss and every gradient come back NaN, the optimizer state is poisoned, and the
rest of the run is lost with no error.
Both halves really do co-occur. unsloth_zoo's own _unsloth_get_batch_samples clears
num_items_in_batch when nothing downstream will divide by it, and a sample truncated
before its assistant turn is fully masked, so a small per_device_train_batch_size can
make one an entire microbatch. Reproduced end to end on Qwen3-0.6B: loss 12.32 then
NaN for the rest of the run.
Fixed in unsloth_zoo itself by unslothai/unsloth-zoo#616. This is here for version
skew, a current unsloth against an older pinned unsloth_zoo, and it no-ops the moment
zoo carries the fix (detected structurally, not by version compare, since the fix is
not in a numbered release yet and dev builds share release metadata).
Patches the autograd Function's forward rather than the module level wrapper.
Generated forwards under unsloth_compiled_cache/ and fused_losses/forward_adapter.py
do `from unsloth_zoo.loss_utils import unsloth_fused_ce_loss`, binding the function
object, so rebinding that name would never reach them. That function resolves
UnslothFusedLoss from its own globals at call time and torch resolves cls.forward at
.apply() time, so a class level patch reaches already imported callers with no cache
invalidation. Verified on both routes: Qwen3 (import hook) and Gemma-3 (codegen, with
unsloth_compiled_module_gemma3 already imported) go NaN -> 0.0 with the cache files
untouched, including under UNSLOTH_COMPILE_OVERWRITE=0.
The replacement keeps the original signature, because apply_autograd_function builds
.apply()'s positional argument list from inspect.signature(forward), and clears the
cached mapping. It short-circuits only the broken case and delegates everything else,
so it does not reimplement chunking or the backward contract. Emptiness is decided on
the effective labels, after the causal shift and after the attention mask is folded
in, since a batch can be non-empty before shifting and empty after.
Tested against a vulnerable unsloth_zoo 2026.9.3: NaN before, finite zero with zero
gradients after, normal dense and sparse batches still bit-match an unchunked
reference, idempotent, and uninstalling restores the NaN. Green on torch 2.6.0 with
transformers 4.57.6 / TRL 0.22.2 / PEFT 0.18.0, and on transformers 5.7.0 / TRL 1.3.0
/ PEFT 0.19.1 and 0.20.0.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
for more information, see https://pre-commit.ci
A positive n_items means the divisor is safe, so the patch delegated whenever n_items was not None. But an explicit ZERO is as unusable as None and it is reachable: _unsloth_get_batch_samples only nulls its count when model_accepts_loss_kwargs is False, and with it True the count survives and is 0 for a fully masked gradient-accumulation window. That zero reached forward and divided the zero summed loss by itself, so the loss stayed NaN. Reproduced on a vulnerable unsloth_zoo 2026.9.3 with the patch installed: n_items=None gave 0.0 while tensor(0), int 0 and a DataParallel-gathered tensor([0, 0]) all gave NaN. All three are now 0.0, and a positive explicit divisor still delegates and matches an unchunked reference. The zero check mirrors forward()'s own handling of a gathered count (ravel()[0] when numel != 1). An inconsistent caller that passes 0 alongside real labels still delegates, which is what unsloth_zoo itself does; inventing a divisor there would silently rescale the loss.
Opening comment-reduction pass. No code change.
|
@codex security review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
for more information, see https://pre-commit.ci
Final comment-reduction pass. No code change.
|
Confirmed unsloth_zoo/fused_losses/cross_entropy_loss.py still divides by a zero count on a fully masked microbatch, so the shim has a real target until the zoo fix lands everywhere. Will get this reviewed. |
|
Closing this. It was a bridge for people on an older pinned The justification also turned out to be weaker than I first thought. If the underlying divide-by-zero is worth closing later, the right place is the kernel in unsloth_zoo, not a runtime patch here. |
Problem
unsloth_zoo's chunked fused cross entropy computes
A microbatch with no trainable labels at all and no caller-supplied
n_itemstherefore divides by zero. Loss and every gradient come back NaN, the optimizer state is poisoned, and the rest of the run is lost with no error raised.Both halves really do co-occur:
n_itemsis legitimatelyNone. unsloth_zoo's own_unsloth_get_batch_samplesclearsnum_items_in_batchwhen nothing downstream will divide by it (tests/test_loss_normalization_contract.pydocuments this).per_device_train_batch_sizecan make one an entire microbatch.Reproduced end to end on Qwen3-0.6B with every 4th sample fully masked at
batch_size=1: loss 12.32, then NaN for the remainder of the run.Why here
The real fix is in unsloth_zoo (unslothai/unsloth-zoo#616). This exists for version skew -- a current
unslothagainst an older pinnedunsloth_zoo-- and no-ops the moment zoo carries the fix. Detection is structural rather than a version compare, because the fix is not in a numbered release yet and dev builds and backports share release metadata.Why it patches the autograd Function
Generated forwards under
unsloth_compiled_cache/, andfused_losses/forward_adapter.py, dofrom unsloth_zoo.loss_utils import unsloth_fused_ce_loss, binding the function object into their own globals. Rebinding that name afterwards would never reach them.That function resolves
UnslothFusedLossfrom its own module globals at call time, and torch resolvescls.forwardat.apply()time, so a class level patch reaches already-imported callers with no cache invalidation. Verified on both routes into the kernel:forward_adapterunsloth_compiled_module_gemma3imported first)Both also pass with
UNSLOTH_COMPILE_OVERWRITE=0.The replacement preserves the original signature, because
apply_autograd_functionbuilds.apply()'s positional argument list frominspect.signature(forward), and clears the cached mapping afterwards. It short-circuits only the broken case and delegates everything else, so it does not reimplement chunking, the compile probe, or the backward contract. Emptiness is decided on the effective labels, after the causal shift and after the attention mask is folded in, since a batch can be non-empty before shifting and empty after.Testing
Against a vulnerable
unsloth_zoo 2026.9.3, 13/13:0.0with zero finite gradients aftern_items, still bit-match an unchunked referencemaskargument path, a trainable bias, andshift_labels=Falseuninstall()restores the original NaN, proving the patch is what fixed itDependency matrix, all green:
compileall,ruff --select E9,F63,F7,F82clean; the new function ast-parses on Python 3.9 through 3.13.Not affected, and not touched: GRPO uses its own autograd Function (
UnslothEfficientGRPO), so vLLM /fast_inferenceruns do not reach this kernel, andunsloth_zoo/mlx/has no reference tofused_lossesat all.