-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
Why torch.nn.Linear is split into Transpose and Gemm layers in torch.onnx.export()? #3257
Comments
cc: @ezyang @houseroad |
Yes, this is a known issue. To fix this we'll need to introduce a little optimization pass that fuses transposes into Gemm operators. |
I have another question: Why haven't you use |
I found onnx/onnx#18, where |
I think the plan is (was?) to remove |
@fmassa Oh, I see. Thanks. |
Now I found there's another op named " |
It's a bug. Please try #3325 |
@ezyang Sorry, but I couldn't find your PRed branch |
@ezyang self resolved. https://github.com/ezyang/pytorch-1/tree/pr/expand-opt |
@ezyang I have tried it, but the result was that there was |
@imai-lm So, IIUC, your exported ONNX model had Expand in it (i.e., it didn't error on expand)? That shouldn't happen. Did you recompile the C++ bits? If so, I'll try to repro tomorrow. |
@ezyang yes, my exported ONNX model had Expand ops in it, and there was no error. I tried it under different environments (Ubuntu, and macOS) with reinstallation and got the same result. What I tried was the following:
The result model was:
|
Reproduced. Looking into it. |
@imai-lm I just realized what's going on: there's some more post-processing going on after the verbose print, so what you see is not what you actually get. If you look at the actual ONNX file produced it will not have any Expand calls. We'll fix the printing problem shortly. |
@ezyang I confirmed that with the following code. Actually there were no
|
I looked into the output of
torch.onnx.export()
and found that every layers declared astorch.nn.Linear()
was split into two layers;Transpose
thenGemm
. I think it is redundant, becauseGemm
operator of ONNX hastransB
attribute, which transposes the second argument.Why wouldn't you use the attribute and simply translate it to
Gemm
only?The text was updated successfully, but these errors were encountered: