Separate Relay Depthwise Conv operator

Development
#1

Currently, Relay conv2d internally decides whether a Relay Conv2d operator is depthwise or not. This makes code somewhat messy - lots of it conditions and indirections, quite confusing HWOI vs HWOI kernel layout. In addition, it is difficult to understand from the debug runtime if the conv operator was depthwise or not.

As far as I understand, code will be much simpler if we just have a separate Relay depthwise conv2d operator. We already have different compute, schedule, InferType etc. Refactoring might not be too painful. What do other folks think?

@kevinthesun @tqchen @yzhliu @FrozenGene @ramana-arm @haichen

#2

The main rationale behind a single conv2d op is to be able to support grouped conv2d, where depthwise and conv2d are special case of the grouped conv2d.

Having three instances(conv2d, grouped conv2d and depthwise) or two won’t remove the additional complexities in handling the conv2d case, since the groups parameters is expected to be supported. So I still think it is useful to keep the conv2d as it is.

It is OK to have different compute to support the possibilities of different conv2d setups.

Perhaps the main reason behind the pain was not clearly specifying the group conv2d semantics. It might be useful to discuss that case, and streamline every part to support group conv2d well.

1 Like
#3

@anijain2305 Thank you for bringing this up. Would you mind listing the current pain point of managing different categories of conv2d in Relay, and listing pros and cons for keeping the current method VS separating?