Making MKL-DNN default on MXNet master #13681
Conversation
…tor-mxnet into feature/mklnn-default-make
|
@xinyu-intel No, we need not modify USE_MKLDNN flag in config.mk file. USE_MKLDNN=1 is set on select kernel/processors in Makefile and CMakefile. |
|
mseth is out for the next two weeks so I am continuing his PR on #13744. @pengzhao-intel @TaoLv please review this version (all I did was retrigger as it fails on what I believe is a flaky test on windows-gpu stage. documented here #13743) |
|
@azai91 could you let @mseth10 adding you into collaborator list so that we can continue work in this PR? https://help.github.com/articles/inviting-collaborators-to-a-personal-repository/ |
|
that would be ideal. would be a lot cleaner to just retrigger on this PR compared with starting a new one just for one additional commit. however, seth is out in india without his laptop until 1/11. I tried to find ways to push directly onto his repo / retrigger from the jenkins GUI with no avail. @sandeep-krishnamurthy, since you are a committer, do you have permission to retrigger the last commit on this PR from the jenkins GUI? |
|
@azai91 I have added you as a collaborator to my mxnet repo. You should be able to re-trigger now. Sorry for the delayed reaponse. |
|
@mseth10 @azai91 Thank you. Now this PR looks good to me. Before merging, I want to double check with @szha , @marcoabreu if we need adjust the night build releasing process for this change? I assume that https://pypi.org/project/mxnet/#history |
|
it is explicitly turned off (https://www.dropbox.com/s/fk2jipmiivfjog0/Screenshot%202019-01-02%2010.11.43.png?dl=0). |
|
Thanks for the info @azai91 . I think the PR is good to merge. @TaoLv @sandeep-krishnamurthy could you help merge the code? |
|
@azai91 Thank you for the information. |
|
Add next step in here #12591 (comment) |
| @@ -20,7 +20,7 @@ mxnet_option(USE_F16C "Build with x86 F16C instruction support" ON) | |||
| mxnet_option(USE_LAPACK "Build with lapack support" ON) | |||
| mxnet_option(USE_MKL_IF_AVAILABLE "Use MKL if found" ON) | |||
| mxnet_option(USE_MKLML_MKL "Use MKLDNN variant of MKL (if MKL found)" ON IF USE_MKL_IF_AVAILABLE AND (NOT APPLE)) | |||
| mxnet_option(USE_MKLDNN "Use MKLDNN variant of MKL (if MKL found)" ON IF USE_MKL_IF_AVAILABLE AND (NOT APPLE)) | |||
| mxnet_option(USE_MKLDNN "Use MKLDNN variant of MKL (if MKL found)" ON IF USE_MKL_IF_AVAILABLE AND (NOT APPLE) AND (NOT MSVC) AND (CMAKE_SYSTEM_PROCESSOR MATCHES x86_64)) | |||
There was a problem hiding this comment.
Actually CMAKE_SYSTEM_PROCESSOR will not work for cross compilation. You could reuse a variable CMAKE_CROSSCOMPILING for this as shown here.
There was a problem hiding this comment.
Do you suggest we check for
SYSTEM_ARCHITECTURE STREQUAL "x86_64" AND NOT CMAKE_CROSSCOMPILING
instead of
CMAKE_SYSTEM_PROCESSOR MATCHES x86_64?
Also, will CMAKE_HOST_SYSTEM_PROCESSOR MATCHES x86_64 help in case of cross compilation?
There was a problem hiding this comment.
Yes, I think SYSTEM_ARCHITECTURE STREQUAL "x86_64" AND NOT CMAKE_CROSSCOMPILING should work. But you need to add the trick for CMAKE_CROSSCOMPILING from here as well:
# workaround to store CMAKE_CROSSCOMPILING because is getting reset by the project command
if(CMAKE_CROSSCOMPILING)
set(__CMAKE_CROSSCOMPILING ${CMAKE_CROSSCOMPILING})
set(__CMAKE_CROSSCOMPILING_OVERRIDE ON)
endif()
project(mxnet C CXX)
if(__CMAKE_CROSSCOMPILING_OVERRIDE)
set(CMAKE_CROSSCOMPILING ${__CMAKE_CROSSCOMPILING})
endif()
There was a problem hiding this comment.
@lebeg I checked, CMAKE_SYSTEM_PROCESSOR works for cross compilation. CMAKE_CROSSCOMPILING is TRUE only for ARM v6, v7, v8, and for all these cases CMAKE_SYSTEM_PROCESSOR exists and CMAKE_SYSTEM_PROCESSOR MATCHES x86_64 returns FALSE. Hence, I don't think any change is needed. Please correct me if I am missing anything.
There was a problem hiding this comment.
Note the semantics of mxnet_option. If the condition is not satisfied, this option will be turned off ,not setting the default value off.
|
Does this update have anything to do with this nightly test failure (cmake + mkldnn)? |
* mkldnn is default makefile and explicitly turn off for buidls * add endif * retrigger * retrigger * build mkldnn as static lib * update makefile to statically build mkldnn * build static mkldnn * fix static name * fix static name * update static for mac * rename mkldnn dep in ci * remove moving mkldnn dynamic lib * retrigger * remove commented code * retrigger * remove mkldnn dnaymic for unitest * retrigger * retrigger * force static for mkldnn lib * turn of mkldnn on arm builds * remove dynamic mkldnn bind * update jenkins to use only mkldnn * remove last flag * turn mkldnn by default on mac * move mkldnn files for GPU MKLDNN build * copy lib mxnet in gpu build * only link windows * add mkldnn.mk * try force linking * retrigger * retrigger * remove mkldnn dynanmic check * use ifndef * remove test mkldnn install * fix spacing * fix index * remove cp of mkldnn since statically linked * add libmkldnn.a to list of files to pack * include mkl_ml * add mkldnn to pack * add libiomp to ci pack * move static libs * fix typo * pack mkldnn * retrigger * add linux artifacts * move libmkldnn in gpu cmake build * move libmkldnn and libiomp5 on gpu workspace * move linked files * fix typo * fix typo * add artifacts for tensorrt * move mkldnn lib in scala build * move mkldnn lib on cpu scala * create dir for binding * rename libmkldnn in scala * move mklml dep in scala builds * move mkl to another linked folder * move libmkl to another dir * add libmklml * move mkldnn * move mkldnn on centos * specify new dynamic path * retrigger * remove mkldnn dynamic lib * remove moving mkldnn artifact * add ld path * retrigger * Revert "remove moving mkldnn artifact" This reverts commit 16cca19. * Revert "remove mkldnn dynamic lib" This reverts commit d510436. * update makefile * Revert RPATH change and trigger CI * correcting use-mkldnn flags for two tests * mkldnn default on linux for starters * reverting naming rules of pack_lib * adding mkldnn=0 flags to centos non mkldnn builds * adding mkldnn=0 flags to ubuntu gpu non mkldnn builds * removing mkldnn binary operation for ubuntu gpu cmake non mkldnn build * removing mkldnn binary operation for centos non-mkldnn unittest * adding explicit USE_MKLDNN=0 flags for clang builds * adding explicit USE_MKLDNN=0 flags for cpu ubuntu builds * removing mkldnn binaries from non mkldnn builds scala gpu * adding explicit flag mkldnn=0 for tensorrt gpu build * adding explicit flag mkldnn=0 for ubuntu cmake asan * adding centos cpu mkldnn tests to CI * adding CentOS GPU MKLDNN build and unittest * not keeping mkldnn default for mac os * setting mkldnn default for x86_64 only * running docs with mkldnn=0 flag * removing CentOS CPU Scala MKLDNN test * setting mkldnn default for x86_64 only * not making mkldn default on windows * removing Centos MKLDNN tests from CI * retrigger * retrigger * retrigger
* mkldnn is default makefile and explicitly turn off for buidls * add endif * retrigger * retrigger * build mkldnn as static lib * update makefile to statically build mkldnn * build static mkldnn * fix static name * fix static name * update static for mac * rename mkldnn dep in ci * remove moving mkldnn dynamic lib * retrigger * remove commented code * retrigger * remove mkldnn dnaymic for unitest * retrigger * retrigger * force static for mkldnn lib * turn of mkldnn on arm builds * remove dynamic mkldnn bind * update jenkins to use only mkldnn * remove last flag * turn mkldnn by default on mac * move mkldnn files for GPU MKLDNN build * copy lib mxnet in gpu build * only link windows * add mkldnn.mk * try force linking * retrigger * retrigger * remove mkldnn dynanmic check * use ifndef * remove test mkldnn install * fix spacing * fix index * remove cp of mkldnn since statically linked * add libmkldnn.a to list of files to pack * include mkl_ml * add mkldnn to pack * add libiomp to ci pack * move static libs * fix typo * pack mkldnn * retrigger * add linux artifacts * move libmkldnn in gpu cmake build * move libmkldnn and libiomp5 on gpu workspace * move linked files * fix typo * fix typo * add artifacts for tensorrt * move mkldnn lib in scala build * move mkldnn lib on cpu scala * create dir for binding * rename libmkldnn in scala * move mklml dep in scala builds * move mkl to another linked folder * move libmkl to another dir * add libmklml * move mkldnn * move mkldnn on centos * specify new dynamic path * retrigger * remove mkldnn dynamic lib * remove moving mkldnn artifact * add ld path * retrigger * Revert "remove moving mkldnn artifact" This reverts commit 16cca19. * Revert "remove mkldnn dynamic lib" This reverts commit d510436. * update makefile * Revert RPATH change and trigger CI * correcting use-mkldnn flags for two tests * mkldnn default on linux for starters * reverting naming rules of pack_lib * adding mkldnn=0 flags to centos non mkldnn builds * adding mkldnn=0 flags to ubuntu gpu non mkldnn builds * removing mkldnn binary operation for ubuntu gpu cmake non mkldnn build * removing mkldnn binary operation for centos non-mkldnn unittest * adding explicit USE_MKLDNN=0 flags for clang builds * adding explicit USE_MKLDNN=0 flags for cpu ubuntu builds * removing mkldnn binaries from non mkldnn builds scala gpu * adding explicit flag mkldnn=0 for tensorrt gpu build * adding explicit flag mkldnn=0 for ubuntu cmake asan * adding centos cpu mkldnn tests to CI * adding CentOS GPU MKLDNN build and unittest * not keeping mkldnn default for mac os * setting mkldnn default for x86_64 only * running docs with mkldnn=0 flag * removing CentOS CPU Scala MKLDNN test * setting mkldnn default for x86_64 only * not making mkldn default on windows * removing Centos MKLDNN tests from CI * retrigger * retrigger * retrigger
Description
This PR continues work of PR #13464 and aims to make MKL-DNN default on MXNet master for Linux kernels with Intel/AMD processors..
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments