ONNX Export for Max and Average Pooling in CEIL_MODE #16769
Conversation
houseroad
left a comment
houseroad
left a comment
There was a problem hiding this comment.
We don't want to introduce too many expect files, since it has obvious drawbacks. So one expect test for one op sounds reasonable. MaxPool should be already covered. AveragePool we can add a simple one which should be around 20 lines. To test the operators thoroughly, usually we do end to end test to make sure that the exported models are interpreted correctly in Caffe2. So in this case, I would suggest you to add tests in https://github.com/pytorch/pytorch/blob/master/test/onnx/test_pytorch_onnx_caffe2.py to cover different conditions.
|
@houseroad, does this PR look better now that the tests are changed for tests in test_pytorch_onnx_caffe2.py ? |
|
No, the CI still fail, especially, the onnx one. I will take a look when I get time. Also feel free to check what cause the problem in the CI as well. |
|
Some related issues: (onnx/onnx#549) and (onnx/tutorials#39) and (#2898) and (#16336) |
houseroad
left a comment
houseroad
left a comment
There was a problem hiding this comment.
The problem is caused by the division on the integer type.
Please explicitly set the type, otherwise, we will have such problems.
If you use the python 2 environment, you should be able to reproduce it.
|
Thank you @houseroad for your help debugging the issue :) |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| return g.op('Softplus', self) | ||
|
|
||
|
|
||
| def get_pool_ceil_padding(input, kernel_size, stride, padding): |
There was a problem hiding this comment.
Nit: add some comments about the purpose of this function.
7 participants
No description provided.