FrozenGene commented on issue #5230: Adding support for TFLite QnnSubtract operator. URL: https://github.com/apache/incubator-tvm/pull/5230#issuecomment-611884419 > > > Correct. So here, TFLite converts Conv to depthwise conv, where depthwise conv has input_channels = groups = 1 and depth multiplier > 1 > > > This PR converts this type of depthwise conv back to normal conv. Exactly the same way that you are suggesting. (There is not even a way to run Relay depthwise conv with groups = 1) > > > Let us know if that is ok. > > > > > > Yes. Convolution should be better I expect. However, I wish we could have one performance comparation result between these two ways. So that we decide whether we keep depthwise convolution with multiplier greater than 1 or like this pr converting back to normal convolution finally. > > @FrozenGene I think I have not clarified the issue why we are converting the kernel layout for HWOI to HWIO when number of input channels is 1. > > First, let us take the example of the current scenario > > ``` > params['kernel_layout'] = 'HWOI' > ``` > > Assuming you have a network you mentioned before with depthwise convolution with input_channel = 1 and try to parse a Relay graph using the `from_tflite` API you get an error like this > > ``` > v0.0.4 > fn (%input: Tensor[(1, 76, 64, 1), uint8], %v_param_1: Tensor[(64, 1), uint8], %v_param_2: Tensor[(1, 64, 1), uint8], %v_param_3: Tensor[(9, 5, 1, 96), uint8], ...) { > %0 = qnn.subtract(%input, %v_param_1, ...); > %1 = qnn.mul(%0, %v_param_2, ...); > %2 = qnn.conv2d(%1, %v_param_3, ..., data_layout="NHWC", kernel_layout="HWOI", out_dtype="int32") in particular dimension 2 conflicts 96 does not match 1dimension 3 conflicts 1 does not match 96; unable to unify: `Tensor[(9, 5, 96, 1), uint8]` and `Tensor[(9, 5, 1, 96), uint8]`; ; > ``` > > The change > > ``` > params['kernel_layout'] = 'HWIO' if input_c == 1 else 'HWOI' > ``` > > fixes this issue. > > This error happens because input_channel is 1 and in the [Conv2dRel](https://github.com/apache/incubator-tvm/blob/master/src/relay/op/nn/convolution.h#L132-L391) function fails as there are checks for `param->groups > 1`. So this fix is mainly to get the current implementation working. > > In order to do the benchmarking, you are suggesting we need to change the underlying `Conv2dRel` functionality which I think should be part of another PR and RFC perhaps. Oh...I understand it fully now. Thanks for clarifying. If we don't change the Conv2dRel, yes, we can not do benchmark comparison currently. I am fine with this change.
---------------------------------------------------------------- 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] With regards, Apache Git Services
