Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support #20137

Merged
merged 2 commits into from
May 18, 2021
Merged

[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support #20137

merged 2 commits into from
May 18, 2021

Conversation

PawelGlomski-Intel
Copy link
Contributor

@PawelGlomski-Intel PawelGlomski-Intel commented Apr 7, 2021 •
edited
Loading

Description

Adds Deconvolution 1D and 3D support with MKLDNN #19904

Checklist

Essentials

  • PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

Changes

  • Adds Deconvolution 3D and 1D support with MKLDNN
  • Adds 3D Deconvolution test
  • Fixes 1D Deconvolution test

@mxnet-bot
Copy link

Hey @PawelGlomski-Intel , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [centos-gpu, website, centos-cpu, miscellaneous, windows-cpu, windows-gpu, clang, edge, unix-cpu, unix-gpu, sanity]

Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

@lanking520 lanking520 added the pr-work-in-progress PR is still work in progress label Apr 7, 2021
PawelGlomski-Intel
PawelGlomski-Intel commented Apr 7, 2021
View reviewed changes
tests/python/mkl/test_mkldnn.py
@@ -471,8 +471,8 @@ def check_convolution_training(stype):
@with_seed()
def test_Deconvolution():
def check_Deconvolution_training(stype):
for shape in [(3, 3, 10, 10)]: # testing only 2D for now
data_tmp = np.random.randint(256, size=shape)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was randint(256, size=shape) here for a reason? All other tests have normal(-0.1, 1, size=shape) for input values, could this be the reason why was this test flaky? I couldn't reproduce #12579 in my version of the test.

mozga-intel
mozga-intel reviewed Apr 23, 2021
View reviewed changes
src/operator/nn/mkldnn/mkldnn_deconvolution-inl.h Outdated
const uint32_t num_group) {
std::vector<int> order(desc.data.ndims);
std::iota(std::begin(order), std::end(order), 0);
const int offset = static_cast<int>(num_group > 1);
Copy link
Contributor

@mozga-intel mozga-intel Apr 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This offset returns true or false; Why do we need to do static_cast to int? Basically; O0-optimization will treat it like a: mov DWORD PTR [rbp-4], 0, when a boolean type will have mov BYTE PTR [rbp-1], 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted it to be clear that I am using boolean value as an integer and to avoid implicit conversion later on.

@PawelGlomski-Intel PawelGlomski-Intel changed the title [v1.x][FEATURE][WIP] Add MKLDNN Deconvolution 1D and 3D support [v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support May 11, 2021
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels May 11, 2021
@PawelGlomski-Intel
Copy link
Contributor Author

@mxnet-bot run ci [unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu]

@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-awaiting-review PR is waiting for code review and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels May 11, 2021
@PawelGlomski-Intel
Copy link
Contributor Author

PawelGlomski-Intel commented May 18, 2021 •
edited
Loading

@szha Can you please review this?

@szha szha merged commit b3b92c0 into apache:v1.x May 18, 2021
@szha
Copy link
Member

szha commented May 18, 2021

Merged. Thank you @PawelGlomski-Intel

PawelGlomski-Intel added a commit to PawelGlomski-Intel/incubator-mxnet that referenced this pull request May 21, 2021
@PawelGlomski-Intel
[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (apache#20137
c0bad03 
)
@PawelGlomski-Intel PawelGlomski-Intel mentioned this pull request May 21, 2021
Merged
9 tasks
PawelGlomski-Intel added a commit to PawelGlomski-Intel/incubator-mxnet that referenced this pull request Aug 31, 2021
@PawelGlomski-Intel
[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (apache#20137
5b73873 
)
PawelGlomski-Intel added a commit to PawelGlomski-Intel/incubator-mxnet that referenced this pull request Sep 20, 2021
@PawelGlomski-Intel
[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (apache#20137
8d99801 
)
PawelGlomski-Intel added a commit to PawelGlomski-Intel/incubator-mxnet that referenced this pull request Sep 22, 2021
@PawelGlomski-Intel
[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (apache#20137
bb95c34 
)
szha pushed a commit that referenced this pull request Sep 29, 2021
@PawelGlomski-Intel
[BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution suppor…
ab22211 
…t and fix bias (#20292)

* [v1.x][BUGFIX] Implement oneDNN deconvolution primitives to deconvolution 2D (#20107)

* Use mkldnn deconvolution primitive in deconvolution

* Apply clang-format

* Refactor deconvolution version 1

* Refactor deconvolution version 2 and use permute_axes in IOLogicalSwapDesc

* Refactor deconvolution version 3

* Enable Deconvolution2D test

* Fix sanity

* Fix windows builds

* Fix deconvolution with bias test

* [v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (#20137)

* Use MXNET_USE_ONEDNN

* Fix test

* Apply formatter

* Add native support for 3D deconvolution

* Remove outdated check

* Replace math.prod with np.prod

* Check convolution layout only when it has value

* Remove outdated check

* Change tests

* Increase default workspace size to mach convolution

* Fix deconv workspace size

* Increase default deconv workspace size in python API

* Disable 3D tests for GPU

* Add deconv arguments checks

* Remove next_impl calls until it is fixed

* Share workspace

* Fix documentation

* Add test_deconv_dilation

* Fix check

* Fix include order
@szha szha mentioned this pull request Nov 3, 2021
Closed
6 tasks
samskalicky pushed a commit to samskalicky/incubator-mxnet that referenced this pull request Nov 4, 2021
@PawelGlomski-Intel
[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (apache#20137
8600b07 
)
@samskalicky samskalicky mentioned this pull request Nov 4, 2021
Merged
6 tasks
josephevans pushed a commit that referenced this pull request Nov 4, 2021
@samskalicky @PawelGlomski-Intel
[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (#20137) (#…
8173b85 
…20726)

Co-authored-by: Paweł Głomski <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mozga-intel mozga-intel mozga-intel left review comments

@anko-intel anko-intel anko-intel approved these changes

Assignees
No one assigned
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@PawelGlomski-Intel @mxnet-bot @szha @mozga-intel @anko-intel @lanking520 @mseth10