giuseros commented on a change in pull request #6840:
URL: https://github.com/apache/incubator-tvm/pull/6840#discussion_r517271380
##########
File path: tests/python/topi/python/test_topi_conv1d_transpose_ncw.py
##########
@@ -94,6 +94,9 @@ def test_conv1d_transpose_ncw():
verify_conv1d_transpose_ncw(1, 1, 10, 1, 5, 1, (0, 3), (0,))
verify_conv1d_transpose_ncw(1, 1, 10, 1, 5, 1, (1, 3), (0,))
verify_conv1d_transpose_ncw(1, 1, 10, 1, 5, 1, (2, 3), (0,))
+ verify_conv1d_transpose_ncw(1, 257, 128, 1, 512, 128, 256, (0,))
+ verify_conv1d_transpose_ncw(1, 257, 128, 2, 512, 128, 256, (0,))
+ verify_conv1d_transpose_ncw(1, 257, 128, 257, 512, 128, 256, (0,))
Review comment:
Could you add a test for the case kernel_size==stride? I think this is
common in encoder/decoder networks
##########
File path: python/tvm/topi/cuda/conv1d_transpose_ncw.py
##########
@@ -65,36 +66,43 @@ def conv1d_transpose_ncw(cfg, data, kernel, stride,
padding, out_dtype, output_p
out_width = (inp_width - 1) * stride + kernel_size - pad_left - pad_right
+ output_padding
pad_left = kernel_size - 1 - pad_left
pad_right = kernel_size - 1 - pad_right + output_padding
+ padded_width = pad_left + inp_width + pad_right
dilated_width = stride * (inp_width - 1) + 1
- data = te.compute(
- (batch, inp_channels, pad_left + dilated_width + pad_right),
+ padded_dilated_width = pad_left + dilated_width + pad_right
+
+ padded_data = te.compute(
+ (batch, inp_channels, padded_width),
lambda n, c, x: tvm.tir.if_then_else(
- tvm.tir.all(
- x >= pad_left,
- x < pad_left + dilated_width,
- tvm.tir.indexmod(x - pad_left, stride).equal(0),
- ),
- data[n, c, tvm.tir.indexdiv(x - pad_left, stride)],
- tvm.tir.const(0.0, "float32"),
- ),
- name="data_pad",
- )
-
- dc = te.reduce_axis((0, inp_channels), name="dc")
- dw = te.reduce_axis((0, kernel_size), name="dw")
+ tvm.tir.all(x >= pad_left,
+ x < pad_left + inp_width),
+ data[n, c, x - pad_left],
+ tvm.tir.const(0., "float32")),
+ name='data_pad')
+
+ padded_kernel = te.compute(
+ (inp_channels, out_channels, kernel_size + stride - 1),
+ lambda ci, co, k: tvm.tir.if_then_else(
+ tvm.tir.all(k < kernel_size),
+ kernel[ci, co, kernel_size-k-1],
+ tvm.tir.const(0., "float32")),
+ name='kernel_pad')
+
+ ci = te.reduce_axis((0, inp_channels), name='ci')
+ k = te.reduce_axis((0, (kernel_size + stride - 1)//stride), name='k')
+ border = pad_left * (stride - 1)
+
data_out = te.compute(
(batch, out_channels, out_width),
- lambda b, c, w: te.sum(
- data[b, dc, w + dw].astype(out_dtype)
- * kernel[dc, c, kernel_size - 1 - dw].astype(out_dtype),
- axis=[dc, dw],
- ),
- tag="conv1d_transpose_ncw",
- )
+ lambda b, co, w: te.sum(
+ padded_data[b, ci, (border+w + stride - 1) // stride +
k].astype(out_dtype) *
+ padded_kernel[ci, co, k*stride + tvm.tir.indexmod(stride-w-border,
stride)].astype(out_dtype),
+ axis=[ci, k]), tag="conv1d_transpose_ncw")
Review comment:
Could you add a bit more comments about the algorithm you are using?
##########
File path: python/tvm/topi/cuda/conv1d_transpose_ncw.py
##########
@@ -65,36 +66,43 @@ def conv1d_transpose_ncw(cfg, data, kernel, stride,
padding, out_dtype, output_p
out_width = (inp_width - 1) * stride + kernel_size - pad_left - pad_right
+ output_padding
pad_left = kernel_size - 1 - pad_left
pad_right = kernel_size - 1 - pad_right + output_padding
+ padded_width = pad_left + inp_width + pad_right
dilated_width = stride * (inp_width - 1) + 1
- data = te.compute(
- (batch, inp_channels, pad_left + dilated_width + pad_right),
+ padded_dilated_width = pad_left + dilated_width + pad_right
+
+ padded_data = te.compute(
+ (batch, inp_channels, padded_width),
lambda n, c, x: tvm.tir.if_then_else(
- tvm.tir.all(
- x >= pad_left,
- x < pad_left + dilated_width,
- tvm.tir.indexmod(x - pad_left, stride).equal(0),
- ),
- data[n, c, tvm.tir.indexdiv(x - pad_left, stride)],
- tvm.tir.const(0.0, "float32"),
- ),
- name="data_pad",
- )
-
- dc = te.reduce_axis((0, inp_channels), name="dc")
- dw = te.reduce_axis((0, kernel_size), name="dw")
+ tvm.tir.all(x >= pad_left,
+ x < pad_left + inp_width),
+ data[n, c, x - pad_left],
+ tvm.tir.const(0., "float32")),
+ name='data_pad')
+
+ padded_kernel = te.compute(
+ (inp_channels, out_channels, kernel_size + stride - 1),
+ lambda ci, co, k: tvm.tir.if_then_else(
+ tvm.tir.all(k < kernel_size),
+ kernel[ci, co, kernel_size-k-1],
+ tvm.tir.const(0., "float32")),
+ name='kernel_pad')
+
+ ci = te.reduce_axis((0, inp_channels), name='ci')
+ k = te.reduce_axis((0, (kernel_size + stride - 1)//stride), name='k')
+ border = pad_left * (stride - 1)
+
data_out = te.compute(
(batch, out_channels, out_width),
- lambda b, c, w: te.sum(
- data[b, dc, w + dw].astype(out_dtype)
- * kernel[dc, c, kernel_size - 1 - dw].astype(out_dtype),
- axis=[dc, dw],
- ),
- tag="conv1d_transpose_ncw",
- )
+ lambda b, co, w: te.sum(
+ padded_data[b, ci, (border+w + stride - 1) // stride +
k].astype(out_dtype) *
Review comment:
Are you sure this works for:padding=0, kernel_size=2, stride=2?
In this case `ceil(w,stride)` gives 0, 1, 1, 2, 2, .... While *I think* (but
might be wrong) that you want 0, 0, 1, 1, 2, 2, etc... when you index the
`padded_data` tensor
##########
File path: python/tvm/topi/cuda/conv1d_transpose_ncw.py
##########
@@ -65,36 +66,43 @@ def conv1d_transpose_ncw(cfg, data, kernel, stride,
padding, out_dtype, output_p
out_width = (inp_width - 1) * stride + kernel_size - pad_left - pad_right
+ output_padding
pad_left = kernel_size - 1 - pad_left
pad_right = kernel_size - 1 - pad_right + output_padding
+ padded_width = pad_left + inp_width + pad_right
dilated_width = stride * (inp_width - 1) + 1
- data = te.compute(
- (batch, inp_channels, pad_left + dilated_width + pad_right),
+ padded_dilated_width = pad_left + dilated_width + pad_right
+
+ padded_data = te.compute(
+ (batch, inp_channels, padded_width),
lambda n, c, x: tvm.tir.if_then_else(
- tvm.tir.all(
- x >= pad_left,
- x < pad_left + dilated_width,
- tvm.tir.indexmod(x - pad_left, stride).equal(0),
- ),
- data[n, c, tvm.tir.indexdiv(x - pad_left, stride)],
- tvm.tir.const(0.0, "float32"),
- ),
- name="data_pad",
- )
-
- dc = te.reduce_axis((0, inp_channels), name="dc")
- dw = te.reduce_axis((0, kernel_size), name="dw")
+ tvm.tir.all(x >= pad_left,
+ x < pad_left + inp_width),
+ data[n, c, x - pad_left],
+ tvm.tir.const(0., "float32")),
+ name='data_pad')
+
+ padded_kernel = te.compute(
+ (inp_channels, out_channels, kernel_size + stride - 1),
+ lambda ci, co, k: tvm.tir.if_then_else(
+ tvm.tir.all(k < kernel_size),
+ kernel[ci, co, kernel_size-k-1],
+ tvm.tir.const(0., "float32")),
+ name='kernel_pad')
+
+ ci = te.reduce_axis((0, inp_channels), name='ci')
+ k = te.reduce_axis((0, (kernel_size + stride - 1)//stride), name='k')
Review comment:
Instead of writing `(kernel_size + stride - 1)//stride`, could you write
`ceil(kernel_size/stride)`? At the end, it would make the code more
understandable, and wouldn't hurt performance.
----------------------------------------------------------------
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:
[email protected]
