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

[FEATURE] Add oneDNN support for npx.reshape and np.reshape #20563

Merged
merged 2 commits into from
Sep 17, 2021
Merged

[FEATURE] Add oneDNN support for npx.reshape and np.reshape #20563

merged 2 commits into from
Sep 17, 2021

Conversation

agrabows
Copy link
Contributor

Description

Function reshape() from modules mxnet.numpy and mxnet.numpy_extension will now be executed by oneDNN primitives as it is done in module mxnet.ndarray.

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

Changes

  • Add oneDNN execution attributes to NNVM_REGISTER_OP(_npx_reshape)

Comments

Using two NNVM_REGISTER_OP() functions (one for mxnet.ndarray, second for mxnet.numpy/mxnet.numpy_extension) instead of one with .add_alias() because special values in npx.reshape's and nd.reshape's new_shape/target_shape parameters have different functionality. For example:

in nd.reshape():

  • "-3" use the product of two consecutive dimensions of the input shape as the output dimension.

in npx.reshape():

  • "-3" will skip current dimension if and only if the current dim size is one.

Performance comparison:
image

  • ~50% peformance speedup for mx.np.reshape() and mx.npx.reshape().
  • Slight loss of performance for mx.nd.reshape() is caused by adding 0-shape checks to oneDNN path (necessary for mx.np and mx.npx). It is a big deal only for small tensors.

@mxnet-bot
Copy link

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

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 labels Aug 31, 2021
@agrabows
Copy link
Contributor Author

@mxnet-bot run ci [windows-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [windows-gpu]

@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 Aug 31, 2021
bgawrych
bgawrych approved these changes Sep 1, 2021
View reviewed changes
Copy link
Contributor

@bgawrych bgawrych left a comment

Choose a reason for hiding this comment

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

LGTM

mozga-intel
mozga-intel reviewed Sep 1, 2021
View reviewed changes
src/operator/nn/mkldnn/mkldnn_convolution.cc Outdated Show resolved Hide resolved
src/operator/nn/mkldnn/mkldnn_reshape.cc Outdated Show resolved Hide resolved
src/operator/nn/mkldnn/mkldnn_convolution.cc Outdated Show resolved Hide resolved
@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-review PR is waiting for code review pr-awaiting-testing PR is reviewed and waiting CI build and test labels Sep 2, 2021
@agrabows
Copy link
Contributor Author

agrabows commented Sep 3, 2021

@mxnet-bot run ci [edge]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [edge]

@agrabows
Copy link
Contributor Author

agrabows commented Sep 3, 2021

@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 Sep 3, 2021
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-awaiting-review PR is waiting for code review labels Sep 14, 2021
@mseth10 mseth10 added pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Sep 14, 2021
@agrabows
Copy link
Contributor Author

@mxnet-bot run ci [unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu]

@agrabows
Copy link
Contributor Author

@mxnet-bot run ci [centos-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [centos-cpu]

@agrabows
Copy link
Contributor Author

@mxnet-bot run ci [centos-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [centos-gpu]

@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 Sep 17, 2021
@agrabows
Copy link
Contributor Author

@mxnet-bot run ci [centos-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [centos-gpu]

@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 Sep 17, 2021
@akarbown akarbown merged commit 8832c42 into apache:master Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mozga-intel mozga-intel mozga-intel approved these changes

@bgawrych bgawrych bgawrych approved these changes

@akarbown akarbown akarbown 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
@agrabows @mxnet-bot @mozga-intel @bgawrych @akarbown @mseth10 @agrabow