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/08/13 07:59:40 UTC
[GitHub] [incubator-tvm] wjliu1998 opened a new pull request #6267: add dilation in x86 NCHWc depthwise conv support
wjliu1998 opened a new pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267
as mentioned in #4962 (https://github.com/apache/incubator-tvm/issues/4962), the NCHWc depthwise convolution does not support dilation > 1.
The problem has been fixed
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] masahi merged pull request #6267: add dilation in x86 NCHWc depthwise conv support
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] wjliu1998 commented on pull request #6267: add dilation in x86 NCHWc depthwise conv support
Posted by GitBox <gi...@apache.org>.
wjliu1998 commented on pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267#issuecomment-673830644
> @wjliu1998 Thanks, please update the PyTorch frontend test below that has been disabled due to the lack of dilation support in x86:
>
> https://github.com/apache/incubator-tvm/blob/master/tests/python/frontend/pytorch/test_forward.py#L1556-L1560
>
> (Remove `[cuda_ctx]`, do the test in the same way as `fcn` above)
Thanks for the comment! The x86 frontend has been enabled!
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] leandron commented on a change in pull request #6267: add dilation in x86 NCHWc depthwise conv support
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267#discussion_r469978920
##########
File path: tests/python/topi/python/test_topi_depthwise_conv2d.py
##########
@@ -268,7 +268,7 @@ def depthwise_conv2d_with_workload_NCHWc(batch, in_channel, in_height, channel_m
filter_width = filter_height
stride_h = stride_w = stride
- assert dilation == 1, "depthwise_conv2d_NCHWc currently does not support dilation."
+ #assert dilation == 1, "depthwise_conv2d_NCHWc currently does not support dilation."
Review comment:
Sure, thanks for accepting the suggestions.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] masahi commented on pull request #6267: add dilation in x86 NCHWc depthwise conv support
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267#issuecomment-673727099
@wjliu1998 Thanks, please update the PyTorch frontend test below that has been disabled due to the lack of dilation support in x86:
https://github.com/apache/incubator-tvm/blob/master/tests/python/frontend/pytorch/test_forward.py#L1556-L1560
(Remove `[cuda_ctx]`, do the test in the same way as `fcn` above)
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] leandron commented on a change in pull request #6267: add dilation in x86 NCHWc depthwise conv support
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267#discussion_r469791300
##########
File path: python/tvm/topi/x86/depthwise_conv2d.py
##########
@@ -122,13 +122,24 @@ def depthwise_conv2d_NCHWc(cfg, data, kernel, strides, padding, dilation,
strides = strides if isinstance(strides, (tuple, list)) else (strides, strides)
HSTR, WSTR = strides
- pad_top, pad_left, pad_down, pad_right = get_pad_tuple(padding, (filter_height, filter_width))
+ #pad_top, pad_left, pad_down, pad_right = get_pad_tuple(padding, (filter_height, filter_width))
dh, dw = dilation if isinstance(dilation, (tuple, list)) else (dilation, dilation)
- assert (dh, dw) == (1, 1), "Does not support dilation"
-
- out_height = (in_height - filter_height + pad_top + pad_down) // HSTR + 1
- out_width = (in_width - filter_width + pad_left + pad_right) // WSTR + 1
+ #assert (dh, dw) == (1, 1), "Does not support dilation"
+
+ dilated_kernel_h = (filter_height - 1) * dh + 1
+ dilated_kernel_w = (filter_width - 1) * dw + 1
+ pad_top, pad_left, pad_down, pad_right = get_pad_tuple(
+ padding, (dilated_kernel_h, dilated_kernel_w))
+ print("padding", pad_top, pad_left, pad_down, pad_right, dilated_kernel_h, padding)
Review comment:
To output messages, it is better to make usage of the logging functionality. See the `logger` object, created in the beginning of this file.
Same comment applies for other `print` statements below.
##########
File path: python/tvm/topi/x86/depthwise_conv2d.py
##########
@@ -122,13 +122,24 @@ def depthwise_conv2d_NCHWc(cfg, data, kernel, strides, padding, dilation,
strides = strides if isinstance(strides, (tuple, list)) else (strides, strides)
HSTR, WSTR = strides
- pad_top, pad_left, pad_down, pad_right = get_pad_tuple(padding, (filter_height, filter_width))
+ #pad_top, pad_left, pad_down, pad_right = get_pad_tuple(padding, (filter_height, filter_width))
Review comment:
If this is not useful anymore, please remove it rather than keeping it commented out here. Same for other commented lines below.
##########
File path: tests/python/topi/python/test_topi_depthwise_conv2d.py
##########
@@ -268,7 +268,7 @@ def depthwise_conv2d_with_workload_NCHWc(batch, in_channel, in_height, channel_m
filter_width = filter_height
stride_h = stride_w = stride
- assert dilation == 1, "depthwise_conv2d_NCHWc currently does not support dilation."
+ #assert dilation == 1, "depthwise_conv2d_NCHWc currently does not support dilation."
Review comment:
If this is not useful anymore, please remove it rather than keeping it commented out here.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] masahi commented on pull request #6267: add dilation in x86 NCHWc depthwise conv support
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267#issuecomment-673889169
Thanks @wjliu1998 @leandron
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] wjliu1998 commented on a change in pull request #6267: add dilation in x86 NCHWc depthwise conv support
Posted by GitBox <gi...@apache.org>.
wjliu1998 commented on a change in pull request #6267:
URL: https://github.com/apache/incubator-tvm/pull/6267#discussion_r469813115
##########
File path: tests/python/topi/python/test_topi_depthwise_conv2d.py
##########
@@ -268,7 +268,7 @@ def depthwise_conv2d_with_workload_NCHWc(batch, in_channel, in_height, channel_m
filter_width = filter_height
stride_h = stride_w = stride
- assert dilation == 1, "depthwise_conv2d_NCHWc currently does not support dilation."
+ #assert dilation == 1, "depthwise_conv2d_NCHWc currently does not support dilation."
Review comment:
Thanks for the review! The comments and logging have been removed!
----------------------------------------------------------------
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