You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/03/24 09:53:21 UTC
[GitHub] [incubator-tvm] wyc-ruiker opened a new pull request #5142: Wyc
wyc-ruiker opened a new pull request #5142: Wyc
URL: https://github.com/apache/incubator-tvm/pull/5142
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](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
add max_pool1d in https://github.com/apache/incubator-tvm/issues/5133, @masahi @alexwong
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397071431
##########
File path: src/relay/op/nn/pooling.cc
##########
@@ -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]);
Review 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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397132370
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool2d(kernel_size=[4, 4], padding=2, stride=2)(args[0])
+
input_data = torch.rand(input_shape).float()
verify_model(MaxPool2D1().float().eval(), input_data=input_data)
verify_model(MaxPool2D2().float().eval(), input_data=input_data)
+ verify_model(MaxPool2D3().float().eval(), input_data=input_data)
+
+def test_forward_maxpool1d():
+ torch.set_grad_enabled(False)
+ input_shape = [1, 3, 10]
+
+ class MaxPool1D1(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool1d(kernel_size=1)(args[0])
+
+ class MaxPool1D2(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool1d(kernel_size=10)(args[0])
+
+ class MaxPool1D3(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool1d(kernel_size=4, padding=2, stride=2)(args[0])
+
+ 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)
Review comment:
done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397074587
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
Review comment:
Don't we need a test for padding and stride in Maxpool?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5142:
[Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397046667
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool2d(kernel_size=[4, 4], padding=2, stride=2)(args[0])
+
input_data = torch.rand(input_shape).float()
verify_model(MaxPool2D1().float().eval(), input_data=input_data)
verify_model(MaxPool2D2().float().eval(), input_data=input_data)
+ verify_model(MaxPool2D3().float().eval(), input_data=input_data)
+
+def test_forward_maxpool1d():
+ torch.set_grad_enabled(False)
+ input_shape = [1, 3, 10]
+
+ class MaxPool1D1(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool1d(kernel_size=1)(args[0])
+
+ class MaxPool1D2(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool1d(kernel_size=10)(args[0])
+
+ class MaxPool1D3(Module):
+ def forward(self, *args):
+ return torch.nn.MaxPool1d(kernel_size=4, padding=2, stride=2)(args[0])
+
+ 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)
Review comment:
No need to write a wrapper class. See https://github.com/apache/incubator-tvm/blob/86079479f0556002adfce2f438ea2a607e318c23/tests/python/frontend/pytorch/test_forward.py#L704-L732
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5142:
[Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397118351
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
Review 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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi merged pull request #5142: [Torch] Add
support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397074097
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
Review comment:
Don't we need a test for padding and stride in Maxpool?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397124944
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
Review 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 :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on issue #5142: [Torch] Add
support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#issuecomment-603510415
Thanks @wyc-ruiker
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397074097
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
Review comment:
Don't we need a test for padding and stride in Maxpool?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397073052
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -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])
Review comment:
In https://pytorch.org/docs/stable/nn.html#maxpool1d, MaxPool1d and MaxPool2d have dilation argument.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] wyc-ruiker commented on a change in pull request
#5142: [Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
wyc-ruiker commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397132458
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
Review comment:
done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5142:
[Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397048418
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -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])
Review comment:
does pooling has dilation argument?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5142:
[Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397049896
##########
File path: src/relay/op/nn/pooling.cc
##########
@@ -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]);
Review comment:
Why do we need this? Does 1D pooling require two pad values (left & right)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5142:
[Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397050020
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -363,9 +363,35 @@ class MaxPool2D2(Module):
def forward(self, *args):
return torch.nn.MaxPool2d(kernel_size=[10, 10])(args[0])
+ class MaxPool2D3(Module):
Review comment:
Remove this wrapper
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5142:
[Torch] Add support for max_pool1d
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5142: [Torch] Add support for max_pool1d
URL: https://github.com/apache/incubator-tvm/pull/5142#discussion_r397048418
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -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])
Review comment:
does pooling have dilation argument?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services