[Torch] Add support for max_pool1d #5142
Conversation
| input_data = torch.rand(input_shape).float() | ||
| verify_model(MaxPool1D1().float().eval(), input_data=input_data) | ||
| verify_model(MaxPool1D2().float().eval(), input_data=input_data) | ||
| verify_model(MaxPool1D3().float().eval(), input_data=input_data) |
There was a problem hiding this comment.
| @@ -213,12 +213,33 @@ def _impl(inputs, input_types): | |||
| pool_size = _infer_shape(inputs[1]) | |||
| strides = _infer_shape(inputs[2]) | |||
| padding = _infer_shape(inputs[3]) | |||
|
|
|||
| dilation = _infer_shape(inputs[4]) | |||
There was a problem hiding this comment.
does pooling have dilation argument?
There was a problem hiding this comment.
In https://pytorch.org/docs/stable/nn.html#maxpool1d, MaxPool1d and MaxPool2d have dilation argument.
| @@ -987,6 +987,10 @@ Array<te::Tensor> Pool1DCompute(const Attrs& attrs, | |||
| << " or 4-D input (e.g. NCWc on for vector instructions)" | |||
| << " or 5-D input (e.g. NCWnc for tensor accelerators)"; | |||
|
|
|||
| if (param->padding.size() == 1) { | |||
| padding.push_back(padding[0]); | |||
There was a problem hiding this comment.
Why do we need this? Does 1D pooling require two pad values (left & right)?
There was a problem hiding this comment.
Yes, If we don't have this, the max_pool1d will fail in https://github.com/apache/incubator-tvm/blob/7c5ff50873e91e9ad27b5f08847c27d58e8b5c4c/topi/include/topi/nn/pooling.h#L679-L680
| @@ -363,9 +363,35 @@ class MaxPool2D2(Module): | |||
| def forward(self, *args): | |||
| return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0]) | |||
|
|
|||
| class MaxPool2D3(Module): | |||
There was a problem hiding this comment.
Don't we need a test for padding and stride in Maxpool?
There was a problem hiding this comment.
I am talking about the same point as https://github.com/apache/incubator-tvm/pull/5142/files#r397046667
Yes, you should have a test, but no need to write a wrapper class
There was a problem hiding this comment.
I am talking about the same point as https://github.com/apache/incubator-tvm/pull/5142/files#r397046667
Yes, you should have a test, but no need to write a wrapper class
Thanks. I misunderstood your review suggestion :-)
|
Thanks @wyc-ruiker |
* [Torch] Add support for max_pool1d * add test * fix line-too-long * remove wrapper class
* [Torch] Add support for max_pool1d * add test * fix line-too-long * remove wrapper class
Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.
add max_pool1d in #5133, @masahi @alexwong