fix(monkey_patch): let the norm flags cover the whole vision tower - #1443
Open
yupengtang wants to merge 2 commits into
Open
yupengtang wants to merge 2 commits into
yupengtang wants to merge 2 commits into
Conversation
When a model instance is passed in, five patchers gate the per-layer norms on the corresponding flag but patch the vision tower's own norms unconditionally: - llama4, mllama layernorm_pre / layernorm_post (layer_norm) - gemma3, paligemma post_layernorm (layer_norm) - glm4v_moe post_conv_layernorm / post_layernorm (rms_norm) So layer_norm=False (or rms_norm=False for glm4v_moe) still leaves Liger norms on those models. The class-level branch of the same functions already gets this right (`if layer_norm and model is None:`, `if rms_norm:`), and smolvlm, internvl, muse_glimmer and glm4v gate the tower norm the same way as their encoder layers, so the instance path is the odd one out rather than a deliberate exception. Defaults are unchanged - both flags default to True on all five - and the convergence tests pass layer_norm=True explicitly, so nothing that runs today changes behaviour.
This branch has not been deployed
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.
Summary
Five patchers gate the per-layer norms of a vision tower on the corresponding flag, but patch the tower's own norms unconditionally. Passing the flag as
Falsetogether with amodel=instance therefore still swaps Liger norms in:llama4,mllamalayer_normvision_model.layernorm_pre,layernorm_postgemma3,paligemmalayer_normvision_model.post_layernormglm4v_moerms_normvisual.post_conv_layernorm,post_layernormThe class-level branch of the very same functions gets it right (
if layer_norm and model is None:in mllama/gemma3/paligemma,if rms_norm:in glm4v_moe), so one function answers the same flag differently depending on whether you hand it a model. Andsmolvlm,internvl,muse_glimmerandglm4valready gate the tower norm exactly like their encoder layers, which is why these five read as oversights rather than deliberate exceptions.Both flags default to
Trueon all five, so nothing changes unless a caller explicitly opts out.Testing Done
Reproduced on transformers 5.9.0 / torch 2.11.0 before the change, patching an instance with the flag off:
gemma3, llama4 and glm4v_moe behave the same way. After the change every one of those reads
untouched.pytest test/transformers/test_monkey_patch.py: 63 passed / 1 skipped onmain, 68 passed / 1 skipped here. The five added tests each fail onmainand pass with the fix; the skip ismuse_glimmermissing from the installed transformers.ruff check .andruff format --check .with the v0.14.11 that.pre-commit-config.yamlpins: clean.test_mini_models_multimodal.pyfiles pin"rms_norm": Trueandkwargs["layer_norm"] = Truefor these models, glm4v_moe is not in the convergence set at all, and this change only alters theFalsebranch.Also ran on GPU. This branch against
main, both on the same A100 40GB in one job:maintest_mini_models_multimodal.pybf16 (-k "mllama or paligemma or gemma3 or llama4 or glm4v")test_monkey_patch.pyFull
make testscope, separately on an A100 80GB: 4496 passed, 18 failed, 931 skipped, 15 xfailed. Re-running justtest/chunked_loss/test_grpo_loss.pyonmaingives the same 18 failing test ids (luspo/cispocases that miss tolerance on sm80), and that file does not importmonkey_patch.Hardware Type: A100 40GB and A100 80GB, plus CPU for the unit suite. The patchers are pure Python and
test_monkey_patch.pyhas no CUDA gating, so that suite is the one that actually exercises them.