-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Hey @bartekkuncer , Thanks for submitting the PR
CI supported jobs: [edge, unix-cpu, windows-gpu, centos-cpu, sanity, centos-gpu, miscellaneous, windows-cpu, unix-gpu, clang, website] Note: |
CC: @leezu, @TaoLv, @anko-intel, @szha, @akarbown |
@mxnet-bot run ci [centos-gpu, windows-cpu] |
Jenkins CI successfully triggered : [centos-gpu, windows-cpu] |
@mxnet-bot run ci [windows-gpu] |
Jenkins CI successfully triggered : [windows-gpu] |
since this is the master branch, and we're moving from 1.x to 2.0 shouldnt this be the time to make this change? Having a directory named "mkldnn" but using "onednn" everywhere will only cause confusion in the future for those who do not have the historical background. |
@samskalicky Ok, will add it soon. |
Added commit regarding changing name of the submodule directory. |
I think that the commits regarding updating onednns old acronyms used in mxnet in different contexts can be added to separate PR and if there is no objection this one can be already merged into master to avoid creating code with deprecated versions of the flags. |
Can you give an example, which acronyms are you referring to? |
Before v1.1 of oneDNN it was called MKLDNN, then dnnl up until some time ago. Due to that we have a mixture of different acronyms used in the code and unifying them using the most recent one can help avoid some unnecessary confusion. |
8fbfa4e
to
fac82dc
Compare
cf40e65
to
be72ba1
Compare
be72ba1
to
03f8e28
Compare
03f8e28
to
ec69d29
Compare
This change includes: * changing MXNET_USE_MKLDNN flag name to MXNET_USE_ONEDNN * changing USE_MKLDNN flag name to USE_ONEDNN * changing 3rdparty/mkldnn folder name to 3rdparty/onednn * changing include/mkldnn folder name to include/onednn * changing MKLDNN occurences in build and documentation files to ONEDNN * adding Bartosz Kuncer to contributors list
ec69d29
to
60cda22
Compare
@mxnet-bot run ci [unix-gpu] |
Jenkins CI successfully triggered : [unix-gpu] |
@mxnet-bot run ci [unix-gpu] |
Jenkins CI successfully triggered : [unix-gpu] |
@szha @leezu @samskalicky Please help with the merge. |
From #19610:
I prepared three commits regarding this point of main RFC:
changing USE_MKLDNN flag name to USE_ONEDNN to make it consistent with actual library name
I believe that this commit is complete and if there is such a will can be merged into master.
changing MXNET_USE_MKLDNN* flags names to MXNET_USE_ONEDNN* also for consistency reasons
This commit regards changing inner MXNET flag so that it will be consistent with the actual lib name. To avoid creating even bigger mixture of mkldnn/dnnl/onednn acronyms I believe it should be accompanied with another commit changing acronyms used in mxnet function names and comments regarding this particular lib to oneDNN.
changing the 3rdparty/mkldnn folder name to 3rdparty/onednn for consistency.