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

[BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias#20292

Merged
szha merged 22 commits intoapache:masterfrom
PawelGlomski-Intel:pglo/deconvBackport
Sep 29, 2021
Merged

[BACKPORT][BUGFIX][FEATURE] Add oneDNN 1D and 3D deconvolution support and fix bias#20292
szha merged 22 commits intoapache:masterfrom
PawelGlomski-Intel:pglo/deconvBackport

Conversation

@PawelGlomski-Intel
Copy link
Copy Markdown
Contributor

@PawelGlomski-Intel PawelGlomski-Intel commented May 21, 2021 •
edited
Loading

Description

Backport of #20107 and #20137

Currently, deconvolution 2D is implemented with oneDNN convolution primitives. Deconvolution forward pass is implemented with convolution backward primitives that do not include bias. This requires you to manually add the bias, which is currently broken (#19768).

This change implements oneDNN deconvolution primitives to deconvolution, fixing (#19768).
As I couldn't reproduce (#12579) for the 2D case, I enabled the 2D part of this test.

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

@mxnet-bot
Copy link
Copy Markdown

mxnet-bot commented May 21, 2021

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: [miscellaneous, edge, windows-cpu, unix-cpu, unix-gpu, website, windows-gpu, centos-cpu, clang, sanity, centos-gpu]

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.

@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-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress labels May 21, 2021
@PawelGlomski-Intel
Copy link
Copy Markdown
Contributor Author

PawelGlomski-Intel commented May 25, 2021

@mxnet-bot run ci [centos-gpu, unix-cpu, unix-gpu, windows-cpu]

@mxnet-bot
Copy link
Copy Markdown

mxnet-bot commented May 25, 2021

Jenkins CI successfully triggered : [unix-cpu, centos-gpu, unix-gpu, windows-cpu]

@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 25, 2021
@PawelGlomski-Intel
Copy link
Copy Markdown
Contributor Author

PawelGlomski-Intel commented May 25, 2021

@mxnet-bot run ci [unix-cpu]

@mxnet-bot
Copy link
Copy Markdown

mxnet-bot commented May 25, 2021

Jenkins CI successfully triggered : [unix-cpu]

@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 25, 2021
@PawelGlomski-Intel
Copy link
Copy Markdown
Contributor Author

PawelGlomski-Intel commented May 27, 2021

@mxnet-bot run ci [unix-cpu]

@mxnet-bot
Copy link
Copy Markdown

mxnet-bot commented May 27, 2021

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 27, 2021
bartekkuncer
bartekkuncer approved these changes May 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@bartekkuncer bartekkuncer left a comment

Choose a reason for hiding this comment

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

LGTM

@PawelGlomski-Intel
Copy link
Copy Markdown
Contributor Author

PawelGlomski-Intel commented May 31, 2021

@szha can you preview review?

szha
szha reviewed Jun 1, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@szha szha left a comment

Choose a reason for hiding this comment

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

thanks for the forward-port. Could you help make sure that the functionality is tested from the mx.npx module instead of from the legacy Symbol API?

@barry-jin barry-jin mentioned this pull request Sep 21, 2021
Merged
6 tasks
@PawelGlomski-Intel
[v1.x][BUGFIX] Implement oneDNN deconvolution primitives to deconvolu…
7c9edad 
…tion 2D (apache#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
@PawelGlomski-Intel
[v1.x][FEATURE] Add MKLDNN Deconvolution 1D and 3D support (apache#20137
bb95c34 
)
@PawelGlomski-Intel
Use MXNET_USE_ONEDNN
bcb4837
@PawelGlomski-Intel
Fix test
0dc6ea4
@PawelGlomski-Intel
Apply formatter
ca8528f
@PawelGlomski-Intel
Add native support for 3D deconvolution
858ff66
@PawelGlomski-Intel
Remove outdated check
94bcdb6
@PawelGlomski-Intel
Replace math.prod with np.prod
e450b94
@PawelGlomski-Intel
Check convolution layout only when it has value
e7326be
@PawelGlomski-Intel
Remove outdated check
b8d512f
@PawelGlomski-Intel
Change tests
9a7061f
@PawelGlomski-Intel
Increase default workspace size to mach convolution
4232c91
@PawelGlomski-Intel
Fix deconv workspace size
1c6103c
@PawelGlomski-Intel
Increase default deconv workspace size in python API
1a48697
@PawelGlomski-Intel
Disable 3D tests for GPU
30072b3
@PawelGlomski-Intel
Add deconv arguments checks
7abdd75
@PawelGlomski-Intel
Remove next_impl calls until it is fixed
a0dbd07
@PawelGlomski-Intel
Share workspace
991858b
@PawelGlomski-Intel
Fix documentation
ef2e095
@PawelGlomski-Intel
Add test_deconv_dilation
c1b6b04
@PawelGlomski-Intel
Fix check
5392c79
@PawelGlomski-Intel
Fix include order
166f83c
@PawelGlomski-Intel
Copy link
Copy Markdown
Contributor Author

PawelGlomski-Intel commented Sep 23, 2021

@mxnet-bot run ci [miscellaneous, website]

@mxnet-bot
Copy link
Copy Markdown

mxnet-bot commented Sep 23, 2021

Jenkins CI successfully triggered : [website, miscellaneous]

@PawelGlomski-Intel
Copy link
Copy Markdown
Contributor Author

PawelGlomski-Intel commented Sep 23, 2021

@szha Can you please review (excluding mkldnn files)?

@szha
Copy link
Copy Markdown
Member

szha commented Sep 29, 2021

@PawelGlomski-Intel thank you!

This was referenced Sep 29, 2021
Closed
Closed
@PawelGlomski-Intel PawelGlomski-Intel mentioned this pull request Oct 13, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@szha szha Awaiting requested review from szha szha is a code owner

1 more reviewer

@bartekkuncer bartekkuncer bartekkuncer approved these changes

Reviewers whose approvals may not affect merge requirements

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.

3D Deconvolution CPU support MaskRCNN deconv layer does not really use the bias test_mkldnn.test_Deconvolution failed in CI

6 participants

@PawelGlomski-Intel @mxnet-bot @szha @barry-jin @bartekkuncer @mseth10