Skip to content

Stop an older unsloth_zoo turning a fully masked microbatch into NaN - #10892

Closed
danielhanchen wants to merge 6 commits into
mainfrom
fix-zoo-fused-ce-nan
Closed

Stop an older unsloth_zoo turning a fully masked microbatch into NaN#10892
danielhanchen wants to merge 6 commits into
mainfrom
fix-zoo-fused-ce-nan

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Problem

unsloth_zoo's chunked fused cross entropy computes

divisor = n_items if n_items is not None else (labels != ignore_index).sum()

A microbatch with no trainable labels at all and no caller-supplied n_items therefore 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_items is legitimately None. unsloth_zoo's own _unsloth_get_batch_samples clears num_items_in_batch when nothing downstream will divide by it (tests/test_loss_normalization_contract.py documents this).
  • 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 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 unsloth against an older pinned unsloth_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/, and fused_losses/forward_adapter.py, do from 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 UnslothFusedLoss from its own module 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 into the kernel:

route model before after cache
import hook / forward_adapter Qwen3-0.6B NaN 0.0 untouched
codegen (unsloth_compiled_module_gemma3 imported first) gemma-3-270m-it NaN 0.0 untouched

Both also pass with UNSLOTH_COMPILE_OVERWRITE=0.

The replacement preserves the original signature, because apply_autograd_function builds .apply()'s positional argument list from inspect.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:

  • NaN reproduced before, finite 0.0 with zero finite gradients after
  • dense and sparse batches, with and without n_items, still bit-match an unchunked reference
  • the mask argument path, a trainable bias, and shift_labels=False
  • idempotent; uninstall() restores the original NaN, proving the patch is what fixed it
  • no-op verified against a zoo that already carries the fix

Dependency matrix, all green:

torch transformers TRL PEFT
2.6.0 4.57.6 0.22.2 0.18.0
2.6.0 5.7.0 1.3.0 0.19.1 / 0.20.0
2.13.0 latest latest 0.20.0

compileall, ruff --select E9,F63,F7,F82 clean; 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_inference runs do not reach this kernel, and unsloth_zoo/mlx/ has no reference to fused_losses at all.

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.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T03:34:48.855327Z d070cfd Manual request
🔒 Security Review Completed 2026-09-14T03:35:04.755435Z d070cfd Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

pre-commit-ci Bot and others added 3 commits September 14, 2026 03:22
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.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: d070cfd7ee

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: d070cfd7ee

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

pre-commit-ci Bot and others added 2 commits September 14, 2026 03:53
Final comment-reduction pass. No code change.
@danielhanchen

Copy link
Copy Markdown
Member Author

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.

@danielhanchen

Copy link
Copy Markdown
Member Author

Closing this. It was a bridge for people on an older pinned unsloth_zoo who could not pick up unsloth-zoo#616, and that PR is now closed too, so the bridge has nothing to bridge to.

The justification also turned out to be weaker than I first thought. _filter_fully_masked in unsloth_zoo/dataset_utils.py already drops fully masked rows before training, and it runs after truncation, so standard text SFT never reaches the divide-by-zero this patched. The remaining surface is the vision and OCR collators plus streaming datasets. Set against a monkey patch running on every import unsloth indefinitely, that trade does not hold up.

If the underlying divide-by-zero is worth closing later, the right place is the kernel in unsloth_zoo, not a runtime patch here.

@danielhanchen
danielhanchen deleted the fix-zoo-fused-ce-nan branch September 14, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant