Fix silent hook_normalized/hook_scale no-op on NormalizationBridge native-autograd path#1527
Open
Hetens wants to merge 3 commits into
Open
Fix silent hook_normalized/hook_scale no-op on NormalizationBridge native-autograd path#1527Hetens wants to merge 3 commits into
Hetens wants to merge 3 commits into
Conversation
…ansformerLensOrg#1526) hook_scale / hook_normalized were fired on detached observation values and their return values discarded, so forward edits silently no-oped and backward hooks never fired on the 13 architectures using use_native_layernorm_autograd. - backward hooks attached: fall back to the python-norm path (grad-connected) - forward hook edits (detected by identity: returning non-None replaces the tensor): reconstruct output from the hooked values, grad-connected - observation-only hooks (run_with_cache) and hook-free forwards: unchanged, bit-identical native HF output - both fallbacks emit a UserWarning since numerics differ at rounding scale
7 tasks
jlarson4
reviewed
Jul 22, 2026
| self, hidden_states: torch.Tensor, input_dtype: torch.dtype | ||
| ) -> torch.Tensor: | ||
| """Apply weight/bias in float32 before casting back (matches HF precision).""" | ||
| hidden_states = hidden_states * self.weight |
Collaborator
There was a problem hiding this comment.
@Hetens great work on this! Just this one small note from me.
The Gemma family of models applies an offset of +1 to their weight. Without a check for that here, we will produce incorrect results. Should be an easy fix, just add w = (1.0 + self.weight) if getattr(self.config, "rmsnorm_uses_offset", False) else self.weight. We will want to also add a test that covers this specific case.
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.
Description
Fixes #1526.
On the native-autograd path (
use_native_layernorm_autograd=True, hardwired in 13 adapters incl. Gemma 3, NeoX/Pythia, OLMo, Phi, GPT-OSS),NormalizationBridgefiredhook_scale/hook_normalizedon detached observation values and discarded their return values: forward edits silently no-oped and backward hooks never fired, while the default python-norm path honored both.Per the dispatch design agreed in #1526:
Nonereplaces the tensor, matchingHookPointsemantics); the output is reconstructed from the hooked values and stays grad-connected to the input. Whenhook_scaleis edited,hook_normalizedis recomputed from the edited scale first, mirroring the python path's ordering.run_with_cachecaching hooks returnNone) and hook-free forwards: unchanged, bit-identical native HF output and exact autograd. Guarded by a dedicated regression test that fails on any warning.UserWarning(the "solution 1" stopgap), since fallback numerics differ from HF's at float-rounding scale.Known limitation (documented in a comment): a hook that mutates the tensor in place and returns
Noneis not detected as an edit. Return the tensor from the hook to edit.Based on
mainper maintainer request in #1526 (patch release).normalization.pyis currently identical onmainanddev, so this merges cleanly upward. The_last_input_before_normdead-code cleanup from #1526 will follow as a separate PR todev.Type of change
Checklist:
Notes on the checklist:
UserWarnings introduced fire only in the situations that previously silently produced wrong results, which is the point of the fix.Validation
main(verified by swapping inorigin/main'snormalization.py):5 failed, 4 passed. The 5 failures are exactly the bug-demonstrating tests; the 4 passes are the behavior-preservation guards.tests/unit -m "not slow"suite green: 3035 passed, 0 failed, 32 skipped, 9 xfailed (20 min, CPU).make format/make check-formatclean,uv run mypy .clean (no# type: ignoreadded).Found, diagnosed, and fixed with the assistance of Claude (Anthropic's AI assistant), working under my direction; I reviewed the changes and the validation runs are from my machine.