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

[submodule] Remove soon to be obsolete dnnl nomenclature from mxnet #20606

Merged
merged 22 commits into from
Oct 13, 2021
Merged

[submodule] Remove soon to be obsolete dnnl nomenclature from mxnet #20606

merged 22 commits into from
Oct 13, 2021

Conversation

bartekkuncer
Copy link
Contributor

@bartekkuncer bartekkuncer commented Sep 24, 2021 •
edited
Loading

This PR is an answer to the statement from oneDNN:
"oneDNN decided to stop shipping and supporting mkldnn compatibility layer (old names of mkldnn instead of dnnl). The change that removes it is targeting v2.5.

Compatibility layer was introduced in v1.1 when library changed the name. At that time we posted an article how to smoothly transfer from mkldnn to dnnl in source codes and other places. Here's the article: https://oneapi-src.github.io/oneDNN/v2/dev_guide_transition_to_dnnl.html

Who's impacted?

Users who update oneDNN binaries without re-compilation of oneDNN won't see any effect from MKLDNN_somevar env variables.

Users who compile oneDNN from scratch are impacted more:

If user's build system utilizes oneDNN build switches starting with MKLDNN_ prefix, option won't affect the build, and likely CMake would spit a warning message saying such variables weren't used.

If source codes are still utilizing mkldnn namespace or types/APIs that contain mkldnn_ prefix, they won't be compiled successfully.

If end application is linked against libmkldnn.so, it won't be linked successfully.

Finally, MKLDNN_somevar env variables won't take affect in run-time.

We are kindly asking to make sure that mkldnn prefix is not used anywhere on your side, neither in product nor in testing environment.

v2.5 Branch cutoff October 29, 2021 (ww44.5), production release December 10, 2021. Feel free to ask questions. Thank you.".

This change also aims to unify names used to refer to dnnl library in mxnet.

@mxnet-bot
Copy link

Hey @bartekkuncer , 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: [unix-cpu, sanity, edge, centos-cpu, website, clang, windows-gpu, unix-gpu, centos-gpu, windows-cpu, 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 the pr-work-in-progress PR is still work in progress label Sep 24, 2021
bartekkuncer
bartekkuncer commented Sep 24, 2021
View reviewed changes
include/mxnet/ndarray.h Outdated Show resolved Hide resolved
@bartekkuncer bartekkuncer force-pushed the masterTOdnnl2 branch 2 times, most recently from a68a3ea to e6b4434 Compare September 27, 2021 13:56
@bartekkuncer bartekkuncer force-pushed the masterTOdnnl2 branch 3 times, most recently from c7dc6aa to aaae390 Compare September 30, 2021 09:56
@bartekkuncer
Copy link
Contributor Author

@mxnet-bot ci run [miscellaneous]

@mxnet-bot
Copy link

Undefined action detected.
Permissible actions are : run ci [all], run ci [job1, job2]
Example : @mxnet-bot run ci [all]
Example : @mxnet-bot run ci [centos-cpu, clang]

@bartekkuncer
Copy link
Contributor Author

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

@mxnet-bot
Copy link

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

@szha
Copy link
Member

szha commented Oct 4, 2021

Is this change ready?

@bartekkuncer bartekkuncer marked this pull request as ready for review October 4, 2021 07:16
@bartekkuncer
Copy link
Contributor Author

Is this change ready?

Yes.

anko-intel
anko-intel reviewed Oct 4, 2021
View reviewed changes
Copy link
Contributor

@anko-intel anko-intel left a comment

Choose a reason for hiding this comment

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

I have made review for docs directory.
For me it seems that we have to use flags and directories as "dnnl" but in description we should use official library name "oneDNN". Also I am not use if "Intel's OneDNN" is proper name and it should be just "oneDNN"

cpp-package/example/inference/README.md Outdated Show resolved Hide resolved
docs/python_docs/python/tutorials/index.rst Outdated Show resolved Hide resolved
docs/python_docs/python/tutorials/index.rst Outdated Show resolved Hide resolved
docs/python_docs/python/tutorials/performance/backend/dnnl/dnnl_readme.md Outdated Show resolved Hide resolved
docs/python_docs/python/tutorials/performance/backend/dnnl/index.rst Outdated Show resolved Hide resolved
docs/static_site/src/pages/api/faq/env_var.md Outdated Show resolved Hide resolved
docs/static_site/src/pages/api/faq/env_var.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cd/python/pypi/pypi_package.sh Show resolved Hide resolved
ci/docker/runtime_functions.sh Show resolved Hide resolved
@mseth10 mseth10 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress labels Oct 4, 2021
@bartekkuncer
Copy link
Contributor Author

@anko-intel I will apply other changes after all reviews are done.

PawelGlomski-Intel
PawelGlomski-Intel reviewed Oct 4, 2021
View reviewed changes
Copy link
Contributor

@PawelGlomski-Intel PawelGlomski-Intel left a comment

Choose a reason for hiding this comment

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

@anko-intel Should python-side (or user-side in general) changes also use oneDNN then?

python/mxnet/contrib/quantization.py Outdated Show resolved Hide resolved
python/mxnet/contrib/quantization.py Outdated Show resolved Hide resolved
python/mxnet/contrib/quantization.py Outdated Show resolved Hide resolved
@mseth10 mseth10 removed the pr-awaiting-testing PR is reviewed and waiting CI build and test label Oct 4, 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 Oct 13, 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 Oct 13, 2021
@bartekkuncer
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 and removed pr-work-in-progress PR is still work in progress labels Oct 13, 2021
@mseth10 mseth10 added pr-awaiting-review PR is waiting for code review and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 13, 2021
mozga-intel
mozga-intel approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@mozga-intel mozga-intel 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
Copy link
Contributor

mozga-intel commented Oct 13, 2021 •
edited
Loading

@szha @akarbown Could you please review it and help with the merge? Thanks!

@akarbown akarbown merged commit 5e04608 into apache:master Oct 13, 2021
@bartekkuncer bartekkuncer deleted the masterTOdnnl2 branch October 14, 2021 08:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@PawelGlomski-Intel PawelGlomski-Intel PawelGlomski-Intel approved these changes

@vpirogov vpirogov vpirogov left review comments

@akarbown akarbown akarbown approved these changes

@agrabows agrabows agrabows approved these changes

@bgawrych bgawrych bgawrych approved these changes

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

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

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

@eric-haibin-lin eric-haibin-lin Awaiting requested review from eric-haibin-lin eric-haibin-lin is a code owner

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

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

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

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

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.