Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bugfix] Conv3Dtranspose default kernel layout should be IODHW #14340

Merged
merged 10 commits into from
Mar 30, 2023

Conversation

rebel-jangys
Copy link
Contributor

Fix the issue #14326
@masahi

@tvm-bot
Copy link
Collaborator

tvm-bot commented Mar 20, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

  • No users to tag found in teams: bugfix See #10317 for details

Generated by tvm-bot

@masahi
Copy link
Member

masahi commented Mar 21, 2023

Can you look at the failed test?

@rebel-jangys
Copy link
Contributor Author

SInce this PR corrects the conv3d transpose kernel layout, many frontend relay converters and test cases have to be fixed as well.

For example, the below test case intentionally set wrong kernel_layout to fit in original wrong kernel layout implementation.

def get_conv3d_transpose(
x_shape=(1, 32, 8, 8, 8),
k_shape=(32, 16, 3, 3, 3),
groups=1,
padding=(0, 0, 0),
strides=(1, 1, 1),
output_padding=(0, 0, 0),
activation=None,
dtype="float32",
data_layout="NCDHW",
kernel_layout="OIDHW",
):

I’ve fixed pytorch relay converter and some test cases, but I would greatly appreciate any assistance with fix.

@rebel-jangys
Copy link
Contributor Author

cc @apeskov , this PR is the one you commented as TODO

// TODO(@apeskov): WA. conv3dTranspose uses wrong layout specifier. IO instead of OI.

@masahi masahi merged commit 8e2382e into apache:main Mar 30, 2023
@masahi
Copy link
Member

masahi commented Mar 30, 2023

@rebel-jangys Thanks for your hard work, it's merged finally!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants