Add group_conv2d_transpose_nchw to support groups argument - #8799
Closed
Lyken17 wants to merge 1 commit into
Closed
Conversation
Lyken17
requested review from
Huyuwei,
Laurawly,
ZihengJiang,
jcf94,
jwfromm,
kevinthesun,
masahi,
mbrookhart,
vinx13 and
yzhliu
as code owners
August 19, 2021 17:29
Member
|
Merging these functions sounds good to me. Note that we have unified operators in Relay level, it might be fine to lower to different topi operators if this is simpler |
Contributor
Author
|
@vinx13 What is relationship between |
Member
|
tvm/python/tvm/relay/op/strategy/generic.py Line 249 in 596ff5c |
Member
|
@Lyken17 could you add a test case? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As mentioned in #8182,
TranposedConv2dis an important operator in GAN related applications andgroupsshould also be supported for this operator. This PR was set to implement this missing feature.However, there are parts I am still confusing
In topi.nn.conv2d, there are
data, kernel, strides, padding, out_dtype, output_paddingas well asInput, Filter, strides, padding, out_dtype, output_padding, which one should be used for newly added function?In most DL frameoworks (e.g., PyTorch),
conv2dis a unified function with support of various arguments such as padding, dilation, groups. But intopi,conv2d(w/o groups) andgroup_conv2dare two differnet function. While I understand this might be important to backward compability, I would recommend to merge these functions for simplicty.Please comment if you have any thoughts. I will prepare unit tests after the discussion.