fuse_layernorm_or_instancenorm: check gamma / beta shapes, not just their rank - #2807
Open
LeSingh1 wants to merge 1 commit into
Open
fuse_layernorm_or_instancenorm: check gamma / beta shapes, not just their rank#2807LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
…heir rank The pass decides a pattern is a layer_norm when gamma and beta have rank len(axes), and an instance_norm when they squeeze to rank 1. Neither test looks at the actual shape, so a gamma that merely broadcasts over those axes is fused into an op it does not fit. Both cases break the conversion of a program that is valid before the pass: - axes=[-1] on a (1, 2, 5) input with gamma of shape (1,) is fused into layer_norm, whose type inference then raises "Expect shape [5] for gamma, but get shape (1,)". - axes=[-2, -1] on a (1, 3, 4, 5) input with gamma of shape (1, 1, 4, 1) squeezes to (4,) and is fused into instance_norm, moving a factor that applied to H onto the channel axis. instance_norm does not validate the length, so this reaches the backend and fails to compile with "Dimension 0 of tensor parameter beta[0] has unexpected length 4; expected 3". Require gamma and beta to have shape x.shape[axes] for layer_norm, using layer_norm's own symbol tolerant comparison, and to be C long for instance_norm.
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.
_try_apply_transformdecides a pattern is alayer_normwhen gamma and beta have ranklen(axes), and aninstance_normwhen they squeeze to rank 1:Neither looks at the shape, so a gamma that merely broadcasts over those axes is fused into an op it does not fit. Both cases break the conversion of a program that is valid before the pass:
layer_norm.
axes=[-1]on a(1, 2, 5)input withgammaof shape(1,)— a normalization with a scalar affine — has rank 1, so it is fused, andlayer_norm.type_inference(which requiresgamma.shape == x.shape[axes]) immediately raises:instance_norm.
axes=[-2, -1]on a(1, 3, 4, 5)input withgammaof shape(1, 1, 4, 1)squeezes to(4,), so it is fused intoinstance_norm(gamma=<len 4>)— moving a factor that applied toHonto the channel axis, which is 3 long.instance_norm.type_inferencedoes not check the length, so this reaches the backend:Fix
Require gamma and beta to have shape
x.shape[axes]forlayer_norm(reusinglayer_norm._is_compatible_shape, so symbolic dims stay allowed), and to beClong forinstance_norm, whereCisx.shape[-3]foraxes=[-2, -1]andx.shape[-1]for the channel-lastaxes=[-3, -2]case.Patterns that do fit are fused exactly as before; the pass just fuses fewer of the ones it cannot represent.
Testing
Three tests in
TestFuseLayerNormOrInstanceNorm: one per case above, plus a control that a proper per-channel gamma is still fused intoinstance_norm. The two negative tests fail onmain. The existing tests in that class are unchanged and still pass.