Fix inceptionv3 #1446
Fix inceptionv3 #1446
Conversation
| @@ -163,7 +163,7 @@ def _declaration_conv_NCHWc(wkl, sch, data, kernel): | |||
| out_height = (wkl.height + 2 * HPAD - wkl.hkernel) // HSTR + 1 | |||
| out_width = (wkl.width + 2 * WPAD - wkl.wkernel) // WSTR + 1 | |||
|
|
|||
| DOPAD = (HPAD != 0 and WPAD != 0) | |||
| DOPAD = (HPAD != 0 or WPAD != 0) | |||
There was a problem hiding this comment.
This is redundant.
We can always call nn.pad here and inline in the schedule.
Inline is able to simplify the case without padding to the simplest form.
There was a problem hiding this comment.
I think DOPAD helps avoid unnecessary pad operation. When padding is 0, nn.pad will still copy input to a new tensor.
There was a problem hiding this comment.
Yes, but if we inline the pad stage in schedule. They are the same.
If pad is zero, nn.pad simply copy the tensor. Then inline can eliminate this useless copy.
So we don't need to treat them as two cases.
There was a problem hiding this comment.
@merrymercy I remember inline padding will introduce if/else in conv computing, which actually slows down compare to doing copy beforehand.
There was a problem hiding this comment.
You can take a look at nn.pad. If all padding is zero, no if/else is introduced.
https://github.com/dmlc/tvm/blob/f32841fa839ecf09e512f702d3e9e28c27fe603f/topi/python/topi/nn/pad.py#L46
There was a problem hiding this comment.
oh, yes, you're saying data_pad.inline(), I thought you guys were talking about inline pad into conv.
|
@merrymercy has a good point and may simplify the logic. Since this is a bugfix and the current way is also fine. I am going to merge this first, we can make followup changes |
Fix bugs for conv2d padding and conversion of avg pooling from mxnet to nnvm.
@yzhliu @yidawang