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

[Relay][Topi][AutoTVM] Winograd support for Conv3D #5186

Merged
merged 28 commits into from
Apr 5, 2020

Conversation

jwfromm
Copy link
Contributor

@jwfromm jwfromm commented Mar 31, 2020

This PR adds winograd functions and schedules to Conv3D in Topi. I also add an alter op layout pass to pretransform weights along with the associated relay functions. While working writing the relay operators, I noticed quite a bit of inconsistency across various convolution operators and decided to organize them and reduce code duplication. Now, make functions for all convolution variants are shared where possible so that each ND variant doesn't need its own. I also moved all type relationship templates to convolution.h rather than have them split across two files.

Note that unlike conv2d, it is quite common for conv3d to have non-uniform kernel sizes (8x3x3) for example. Thus, this PR includes two variations of conv3d_winograd, one for NxNxN kernels that transforms all axes and one for kernels where KD!=KH,KW where only height and width are transformed.

I've found that for some workloads this new winograd function can be up to 4X faster than direct convolution.

@jwfromm
Copy link
Contributor Author

jwfromm commented Mar 31, 2020

@merrymercy @icemelon9 @comaniac @masahi this PR is likely most relevant to you guys, can you take a look and let me know what you think?

Copy link
Member

@icemelon icemelon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial reviews. will come back for more reviews soon

src/relay/op/nn/convolution.cc Outdated Show resolved Hide resolved
src/relay/op/nn/convolution.cc Outdated Show resolved Hide resolved
src/relay/op/nn/convolution.cc Outdated Show resolved Hide resolved
src/relay/op/nn/convolution.cc Outdated Show resolved Hide resolved
src/relay/op/nn/convolution.h Outdated Show resolved Hide resolved
src/relay/op/nn/convolution.cc Outdated Show resolved Hide resolved
include/tvm/relay/attrs/nn.h Outdated Show resolved Hide resolved
Copy link
Member

@icemelon icemelon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. just some minor comments

python/tvm/relay/op/nn/nn.py Outdated Show resolved Hide resolved
python/tvm/relay/op/nn/nn.py Outdated Show resolved Hide resolved
@icemelon icemelon merged commit 02eb183 into apache:master Apr 5, 2020
@icemelon
Copy link
Member

icemelon commented Apr 5, 2020

Thanks @jwfromm @masahi @FrozenGene. This is now merged.

@jwfromm jwfromm deleted the winograd3d branch April 6, 2020 16:54
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Apr 16, 2020
* Functional conv3d winograd working.

* Formatted python code.

* registered conv3d winograd compute and started adding relay without_weight_transform operator.

* Add topi testing for conv3d winograd.

* Format file.

* small tweak to unrolling to prevent build sticking.

* Refactoring convolution ops in relay.

* Refactored relay convolutions.

* Bug fixes.

* Fixed static bug in convolution.

* Added conv3d alter op layout and related support.

* Bug fixes and testing done.

* Fix a few autotvm bugs.

* Drop silly debug print.

* Removed debug_skip_region.

* Add variant of conv3d_winograd that doesn't transform depth.

* initial infrastructure done for depthless conv.

* Fix no_depth schedule bugs.

* automatic topi switching between depth and depthless winograd.

* Fixed bug in schedule.

* lint fixes.

* Removed indents in convolution.cc

* missed a few indents oops.

* fixed flop count.

* One more small tweak.

* Change kernel pack inner axes order.

* Style changes.

* Comment fixes.
zhiics pushed a commit to neo-ai/tvm that referenced this pull request Apr 17, 2020
* Functional conv3d winograd working.

* Formatted python code.

* registered conv3d winograd compute and started adding relay without_weight_transform operator.

* Add topi testing for conv3d winograd.

* Format file.

* small tweak to unrolling to prevent build sticking.

* Refactoring convolution ops in relay.

* Refactored relay convolutions.

* Bug fixes.

* Fixed static bug in convolution.

* Added conv3d alter op layout and related support.

* Bug fixes and testing done.

* Fix a few autotvm bugs.

* Drop silly debug print.

* Removed debug_skip_region.

* Add variant of conv3d_winograd that doesn't transform depth.

* initial infrastructure done for depthless conv.

* Fix no_depth schedule bugs.

* automatic topi switching between depth and depthless winograd.

* Fixed bug in schedule.

* lint fixes.

* Removed indents in convolution.cc

* missed a few indents oops.

* fixed flop count.

* One more small tweak.

* Change kernel pack inner axes order.

* Style changes.

* Comment fixes.
dpankratz pushed a commit to dpankratz/incubator-tvm that referenced this pull request Apr 24, 2020
* Functional conv3d winograd working.

* Formatted python code.

* registered conv3d winograd compute and started adding relay without_weight_transform operator.

* Add topi testing for conv3d winograd.

* Format file.

* small tweak to unrolling to prevent build sticking.

* Refactoring convolution ops in relay.

* Refactored relay convolutions.

* Bug fixes.

* Fixed static bug in convolution.

* Added conv3d alter op layout and related support.

* Bug fixes and testing done.

* Fix a few autotvm bugs.

* Drop silly debug print.

* Removed debug_skip_region.

* Add variant of conv3d_winograd that doesn't transform depth.

* initial infrastructure done for depthless conv.

* Fix no_depth schedule bugs.

* automatic topi switching between depth and depthless winograd.

* Fixed bug in schedule.

* lint fixes.

* Removed indents in convolution.cc

* missed a few indents oops.

* fixed flop count.

* One more small tweak.

* Change kernel pack inner axes order.

* Style changes.

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

Successfully merging this pull request may close these issues.

5 participants