-
Notifications
You must be signed in to change notification settings - Fork 64
InstanceNorm always has training attribute set to True #1262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I think you are right that it should be PyTorch's issue setting the parameter. Could you print the fx graph, and/or try exporting with torch.export.export()? |
If I print with
I get
If I print with
I get
So it does look like the |
@BowenBao I remember you looked at this issue before. Do you have anything to add? |
There are several problems here. The ultimate main goal is to match the behavior of the exported onnx with pytorch.
|
@BowenBao Shouldn't calling |
In this case not enough. Probably need changing However, it might have unwanted effects on the operator's behavior. I feel this is a legacy issue in pytorch. |
Seeing this problem for BatchNorm as well. I guess it's affecting all normalization ops that are done this way in pytorch. |
@yuanyao-nv could you provide the sample repro for BatchNorm? Back to InstanceNorm, I'm curious if there were unwanted decompositions happening during dynamo tracing. Normally we should be receiving aten::instance_norm instead of aten::batch_norm. |
Turns out aten::instance_norm is another one of those normally unskippable decomposition. It is slightly complicated since there are mutations involved when updating running mean & var. |
@BowenBao A BatchNorm example can be found in this model:
Env: I can also upload the model if you need. |
@yuanyao-nv thanks for repro, this is also originated from
|
@BowenBao That's good to know. Can you please also share the command you used for this printout? |
|
Stack from [ghstack](https://github.com/ezyang/ghstack) (oldest at bottom): * __->__ #1284 - Fixes #1280, #1262. Avoid exporting as onnx::BatchNormalization with training=True. - Fixes mismatch in unittest.
How about these two lines ? Shouldn't they be fixed ? Can we just use the return value of BatchNorm in the previous line for the mean and var? |
This issue is now fixed by #1284 and pytorch/pytorch#120866
We can keep tracking the batchnorm outputs issue in #1256, since now they are irrelevant to instance norm after the fix. |
I'm exporting a small ONNX model consisting of just a InstanceNorm2d op. The exported model contains a BatchNorm node with the training attribute set to True.
Here's my script:
If I understand it correctly, the origin of this issue should be in the torch repo since onnxscript just takes whatever value is passed for the
training
input ofaten_native_batch_norm()
?Thanks.
The text was updated successfully, but these errors were encountered: