Fix Gemma-4 vision pooler float16 overflow (26B-A4B / 31B NaN loss)#931
Fix Gemma-4 vision pooler float16 overflow (26B-A4B / 31B NaN loss)#931danielhanchen merged 6 commits intomainunslothai/unsloth-zoo:mainfrom gemma4-vision-pooler-fp16unslothai/unsloth-zoo:gemma4-vision-pooler-fp16Copy head branch name to clipboard
Conversation
Gemma4VisionPooler.forward scales pooled activations by sqrt(hidden_size) in the input dtype on transformers 5.5.0 through 5.9.x (and the yanked 5.10.0). For the 26B-A4B / 31B vision tower (hidden 1152, no clipped linears) fp16 activations near 2300 scale past the fp16 max of 65504, producing inf then NaN loss through embed_vision. This engages under forced-float32 mode, which keeps the unquantized vision tower in fp16 with autocast off. Mirror the upstream >= 5.10.1 fix (huggingface/transformers#46277) as a temporary patch: run the pooler in fp32, let standardization promote to fp32, and cast last_hidden_state back to the pixel_values dtype in Gemma4VisionModel.forward. Source-pattern classification skips fixed releases entirely and leaves unrecognized future sources untouched; transformers without gemma4 no-op via ImportError. bf16 and fp32 inputs are bit-identical to upstream.
…path Cast last_hidden_state back to the patch embedder input_proj weight dtype (upstream's inputs_embeds.dtype) instead of the raw pixel dtype, gated to fp16 towers, so fp32 processor pixels into an fp16 tower no longer leak fp32 features into the fp16 embed_vision projection, and bf16 / fp32 setups stay bit-identical even with mixed input dtypes. Handle tuple returns from return_dict=False. Install the vision wrapper before the pooler wrapper so the pooler marker is the commit point, and re-install a missing vision wrapper next to a marked pooler instead of reporting already. Classify the buggy pattern before the fixed one. Strengthen the standardize=False test to exact upstream semantics and add coverage for mixed dtypes, tuple returns, repair, and unavailable source.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8993715c46
ℹ️ 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".
| hidden_states = hidden_states.to(target_dtype) | ||
| if is_tuple: | ||
| output = (hidden_states,) + output[1:] | ||
| else: | ||
| output.last_hidden_state = hidden_states |
There was a problem hiding this comment.
Keep captured final hidden states in the cast-back dtype
When callers set output_hidden_states=True, Transformers' @capture_outputs wrapper builds output.hidden_states (and ties its final entry to last_hidden_state) inside _original_vision_forward before this wrapper runs. This code then casts only output.last_hidden_state, leaving output.hidden_states[-1] as the fp32 pooled result while the documented final state is fp16; consumers using the hidden-state tuple therefore see inconsistent dtype/value semantics relative to the upstream fixed implementation. Update the corresponding final entry after the cast as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed on transformers 5.5.0: with output_hidden_states=True the capture wrapper tied hidden_states[-1] to the pre-cast fp32 tensor while last_hidden_state was cast to fp16. Fixed in 31a78e8: the wrapper now casts that entry too (and swaps the pre-cast tensor inside nested tuples on the return_dict=False path), with a regression test.
| embedder = getattr(vision_model, "patch_embedder", None) | ||
| weight = getattr(getattr(embedder, "input_proj", None), "weight", None) | ||
| if torch.is_tensor(weight): | ||
| return weight.dtype |
There was a problem hiding this comment.
Derive the cast-back dtype from the actual embeddings
With an fp16 vision tower running under bf16 autocast, input_proj produces bf16 inputs_embeds even though its weight remains fp16. The pooler is already safe in that mode, but this helper returns the fp16 weight dtype, so the vision wrapper downcasts its bf16 output; for standardize=False, pooled values above 65504 become inf, recreating the overflow in a configuration that was previously safe. Capture the actual patch-embedder output dtype (as upstream does with inputs_embeds.dtype) rather than assuming it equals the parameter dtype.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed: autocast recasts non-fp32 args, so under bf16 autocast an fp16-weight input_proj emits bf16 and the weight-based target wrongly downcast it (saturation risk for standardize=False). Fixed in 31a78e8: a transient forward hook on the patch embedder captures the actual output dtype (upstream's inputs_embeds.dtype); the weight dtype is only a fallback when the hook sees nothing. Regression test added for the bf16-autocast-over-fp16-weights path.
Capture the actual patch embedder output dtype with a transient forward hook instead of assuming the input_proj weight dtype: under bf16 autocast an fp16-weight Linear emits bf16 (autocast recasts non-fp32 args), so the weight-based target would downcast a previously safe bf16 path and could saturate standardize=False outputs. The weight dtype remains only as a fallback when the hook sees nothing. Also keep captured outputs consistent: when output_hidden_states=True ties hidden_states[-1] to the pre-cast final state, cast that entry too, and swap the pre-cast tensor inside nested tuples on the return_dict=False path.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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 review |
|
Codex Review: Didn't find any major issues. Swish! 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". |
| if "hidden_states.float()*self.root_hidden_size" in source: | ||
| return "fixed" | ||
| return "unknown" | ||
| pass |
| and torch.is_tensor(pixel_values[0]): | ||
| return pixel_values[0].dtype | ||
| return None | ||
| pass |
| pooler_forward._unsloth_vision_pooler_fp16 = True | ||
| pooler_cls.forward = pooler_forward | ||
| return "patched" | ||
| pass |
| _patch_gemma4_vision_pooler_fp16(pooler_cls, vision_cls) | ||
| except Exception as e: | ||
| return raise_error("Gemma4VisionPooler.forward", e) | ||
| pass |
| pass | ||
|
|
||
|
|
||
| def patch_Gemma4VisionPoolerFP16(): |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40ea2009b4
ℹ️ 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".
| if isinstance(embedder, torch.nn.Module): | ||
| def _capture_embed_dtype(module, hook_args, hook_output): | ||
| if torch.is_tensor(hook_output): | ||
| seen["dtype"] = hook_output.dtype | ||
| handle = embedder.register_forward_hook(_capture_embed_dtype) |
There was a problem hiding this comment.
Isolate dtype capture for concurrent forwards
When the same vision model is called concurrently under different autocast contexts (for example, one fp16 request and one bf16-autocast request), both temporary hooks are registered on the shared patch_embedder and each closure observes both calls. The first call to finish can therefore use the other request's dtype for target_dtype; in particular, an fp16 request may return bf16 features and then fail at the downstream fp16 embed_vision projection. Store this per invocation without a shared module hook (or serialize/remove cross-call visibility).
Useful? React with 👍 / 👎.
…unslothai#931/unslothai#933/others) Merge origin/main into the streaming branch. Only unsloth_zoo/mlx/trainer.py conflicted, in MLXTrainingConfig.__init__'s wholesale-copy detection: unslothai#922 (true grad-norm reporting + compiled global-norm clipping) replaced this branch's _MLX_CONFIG_OPTIONAL_COPY_FIELDS-based check with its own _appended_fields set {compile_max_variants, report_grad_norm}, and neither set alone covers the merged config. Reconciled to the union: warmup copy-detection now tolerates every appended field (the positional optional-copy suffix plus the mid-order compile_max_variants). unslothai#922's report_grad_norm landed as the new last field, so it joins _MLX_CONFIG_OPTIONAL_COPY_FIELDS to keep every post-image_size field copy-optional and preserve the positional-copy suffix invariant. Validation on the merged tree: 242 unit + 16 metal + 25 recorded-validation checks pass (grad-norm and streaming/prefetch loops coexist).
Problem
Fine-tuning Gemma-4 26B-A4B or 31B Vision with
dtype = torch.float16produces NaN training loss from step 1 on transformers 5.5.0 through 5.9.x (and the yanked 5.10.0). Text and audio are unaffected, as are E2B / E4B vision and all bf16 runs.Root cause is an upstream transformers bug:
Gemma4VisionPooler.forwardscales pooled activations bysqrt(hidden_size)in the input dtype:The 26B / 31B vision tower (hidden 1152,
sqrt = 33.94, no clipped linears) produces fp16 activations around 2300, which scale to roughly 79k, past the fp16 max of 65504, giving inf and then NaN throughembed_vision. This engages under forced-float32 mode, which keeps the unquantized vision tower in fp16 with autocast off. Localized empirically on a 26B fp16 run: the pooler input was finite (amax 2330), its output inf, and every module before it finite. Upstream fixed this in transformers 5.10.1 via huggingface/transformers#46277.Fix
patch_Gemma4VisionPoolerFP16intemporary_patches/gemma4.pymirrors the upstream fix with two pass-through wrappers:Gemma4VisionPooler.forward: upcasts a float16hidden_statesinput to fp32. The buggy pooler's ops (masked_fill, avg pool, scale) all preserve input dtype, so this matches the fixed pooler's fp32 output up to fp16 rounding of the pooled activations. Dtype promotion then carries fp32 through the caller's standardization, where thestd_biassubtraction cancels the large values (buffer amax is 53760, inside fp16 range).Gemma4VisionModel.forward: castslast_hidden_stateback to the encoder's working dtype, taken from the patch embedder'sinput_projweight (the embedder casts pixels to it, so this equals upstream'sinputs_embeds.dtypeeven when callers pass fp32 processor pixels to an fp16 tower). Only engages for fp16 towers and also handlesreturn_dict = Falsetuple returns.Design points:
Gemma4VisionModel.forward(merge_with_config_defaults,capture_outputs) are preserved, as is signature introspection viafunctools.wraps.Gemma4VisionPoolerorGemma4VisionModel(verified by grepping generatedunsloth_compiled_cachemodules from 52 notebook-matrix runs plus fresh validation runs).embed_vision, so the fp32 pooler output composes there too and fixes the same overflow for fp16 vLLM multimodal on old transformers.Testing
CPU (
tests/test_gemma4_vision_pooler_fp16.py, 19 tests): overflow reproduction, fp32-reference match, standardize on and off, mixed fp32-pixels/fp16-tower, fp32-pixels/bf16-tower untouched, bf16/fp32 bit-identical, tuple returns, kwarg calls, idempotency, repair path, drift and no-source classification, gradient flow, and a real-source canary that fails loudly if upstream rewrites the pooler. Green on transformers 4.57.6 (no gemma4, no-op), 5.5.0 (patches), and 5.14.1 (skips), and the classification was checked against every 5.x release tag: 5.5.0-5.10.0 classify buggy, 5.10.1-5.14.1 classify fixed.GPU (B200, transformers 5.5.0, 3-step LoRA notebook replicas with inference and merged save):
Also reproduced the mixed-dtype path on real 5.5.0 classes: fp32 pixels into an fp16
Gemma4VisionModelreturn fp16 finite features after the patch.Note: the package currently pins
transformers<=5.5.0, which is exactly the buggy release users get; if the pin is later lifted into 5.6-5.9 the patch covers those too, and from 5.10.1 it automatically stands down. The complementary notebook-side fix would be bumping the Gemma-4 vision notebooks off the==5.5.0pin once supported.