Fix the incorrect MKLDNN/MKL logic in cmake #14829

yinghu5 · 2019-04-29T08:25:53Z

Fix the mxnet cmake build systems to make MKLDNN, MKL as BLAS logic correct.

3 files : incubator-mxnet\cmake\ChooseBLAS.cmake
\incubator-mxnet\cmake\Modules\FindMKL.cmake
and Update incubator-mxnet\3rdparty\mshadow\cmake\mshadow.cmake to set BLAS #374
are updated.

Description

(Brief description on what this PR is about)

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
[ x] Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Feature1, tests, (and when applicable, API doc)
Feature2, tests, (and when applicable, API doc)

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

fix the mxnet cmake build systems to make MKLDNN, MKL logic correct.

unlock MKLDNN and MKL dependency

yinghu5 · 2019-04-29T08:30:20Z

We planned to update the 3 cmakefiles : ChooseBLAS.cmake, FindMKL.cmake and mshadow.cmake. The logic diagram is as the pdf. Red parts are original logic and green parts are new.

cmake_mxnet_fixed.pdf

change use BLAS logic
unlock the MKLDNN and MKL dependency

and we test both windows and linux,and get the build passed,
cmake_test.docx

thanks

pengzhao-intel · 2019-04-29T12:40:50Z

@larroy @TaoLv @azai91

pengzhao-intel · 2019-04-29T12:43:36Z

@yinghu5 do we need the build test in CI for these change?

roywei · 2019-04-29T15:44:12Z

@mxnet-label-bot add[CMake]

azai91 · 2019-04-29T17:37:45Z

agree with pengzhao. we should add test to CI to check if flags link MKL and MKLDNN correctly.

pengzhao-intel · 2019-04-30T04:29:11Z

@zixuanweeei please help verify the change in local

Add WIN_CPU_MKLDNN WIN_CPU_MKLDNN_MKL

update for mkl/mkldnn cmake test

custom_steps.compile_windows_cpu_mkldnn(), custom_steps.compile_windows_cpu_mkldnn_mkl(), custom_steps.compile_windows_cpu_nomkldnn_mkl()

WIN_CPU_MKLDNN

WIN_CPU_NOMKLDNN_MKL

yinghu5 · 2019-04-30T13:56:40Z

thank you @pengzhao-intel @azai91 , I try to add CI test first windows. and run into two issues.

add WIN_CPU_MKLDNN. But it failed quickly at cmake configure .
The C++ compiler
"C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/bin/x86_amd64/cl.exe"
is not able to compile a simple test program.

It fails with the following output:
Change Dir: C:/jenkins_slave/workspace/build-cpu-mkldnn/build/CMakeFiles/CMakeTmp
Run Build Command:"jom" "/NOLOGO" "cmTC_14b0a\fast"
jom: parallel job execution disabled for Makefile

	C:\ProgramData\chocolatey\lib\jom\tools\jom.exe -f CMakeFiles\cmTC_14b0a.dir\build.make /nologo -L CMakeFiles\cmTC_14b0a.dir\build

other configure like WIN_CPU_MKLDNN_MKL and WIN_CPU_NOMKLDNN_MKL can pass the steps. but here need MKL are installed in MKL_ROOT=C:\Program Files (x86)\IntelSWTools\compilers_and_libraries\windows\mkl.
Please help make sure the CI windows have MKL_ROOT installed.

pengzhao-intel · 2019-04-30T22:16:19Z

@marcoabreu @KellenSunderland @larroy to help for the CI issue.

larroy · 2019-05-01T00:17:16Z

#14028

larroy · 2019-05-01T00:19:17Z

Can you please adjust ci/build_windows.py with the changes required for your modifications?

larroy · 2019-05-01T00:21:44Z

Is the control flow chart autogenerated if so how? does it reflect our current state or where we want to be at?

larroy · 2019-05-01T00:22:05Z

@lebeg

larroy · 2019-05-01T00:23:12Z

How can we be of additional help? Is because MKL is not found in win? I guess MKL is not installed in our windows AMI. @Chancebair

Chancebair · 2019-05-02T08:54:34Z

MKL is not installed via the AMI baking process

lebeg · 2019-05-02T10:44:15Z

But mklml is downloaded via the cmake script, see here. And MKL_ROOT is set in this line.

lebeg · 2019-05-02T10:54:30Z

cmake/Modules/FindMKL.cmake

@@ -43,55 +43,7 @@ endif()
 # ---[ Root folders
 set(INTEL_ROOT "/opt/intel" CACHE PATH "Folder contains intel libs")

-if(USE_MKLDNN)


Though now it is convoluted in 1 file there is a configuration difference between full MKL and MKLML. For example, the librt library is present only in full MKL, OpenMP library needs to be configured explicitly for MKLML. Previously, I made an attempt to separate it to 2 different files, maybe you can reuse some code from there #11148

@Chancebair . @lebeg thank you a lot for the review. Yes, we actually need full MKL package install on CI machine. There are mainly two factors , which

MKLDNN will don't depend on the MKLML in the future,

second, but important, MXNET will use more MKL functions other than in MKLML.

So we decide to unlock MKLDNN (MKLML) and MKL dependency, which means,
use MKLDNN and USE_BLAS=mkl completely independent.

MKLDNN is on, it will download MKLML automatically. doesn't use full MKL package.

when USE_BLAS=mkl, only use the full MKL. (MKL_ROOT). won't detect MKLML (from MKLDNN).

Thanks

TaoLv · 2019-05-04T14:02:15Z

I remember we have build and test for USE_BLAS=mkl in CI. MKL is installed through apt-get on ubuntu: https://github.com/apache/incubator-mxnet/blob/master/ci/docker/install/ubuntu_mkl.sh#L28
Not sure how is it installed on Windows.

TaoLv · 2019-05-05T04:21:10Z

@Chancebair @marcoabreu Can you help to install a full MKL library for window CI? As showed in the changes of this PR, usually full MKL will be installed to C:\Program Files (x86)\IntelSWTools\compilers_and_libraries\windows\mkl on windows. You can get the executable here: http://registrationcenter-download.intel.com/akdlm/irc_nas/tec/15247/w_mkl_2019.3.203.exe

yinghu5 · 2019-05-06T02:46:32Z

@larroy @lebeg @pengzhao-intel @TaoLv @Chancebair @marcoabreu . sorry i close the PR by accident when rebase my branch , and reopen it into #14877. please help to review there. thanks

yinghu5 · 2019-05-06T02:47:59Z

Is the control flow chart autogenerated if so how? does it reflect our current state or where we want to be at?

No, we draw it manually :)

yinghu5 · 2019-05-06T02:48:51Z

Can you please adjust ci/build_windows.py with the changes required for your modifications?

@larroy yes, we also adjust the ci in new #14877. thanks

yinghu5 · 2019-05-06T03:03:41Z

#14028 cmake and build OpenBLAS

@larroy, in the PR, we only change the MKLDNN and MKL part, and keep the OpenBLAS untouched.

About #14028 agree the latest solution, I will update the install guide of windows

If you don't have the Intel Math Kernel Library (MKL) installed, download and install OpenBLAS. Note that you should also download ```mingw64.dll.zip`` along with openBLAS and add them to PATH.
make sure the OpenBL to point to use version 0.3.6 instead.

Chancebair · 2019-05-07T10:03:55Z

@TaoLv I created a ticket with my teams oncall to look into this

pengzhao-intel · 2019-05-08T01:02:06Z

Please continuous review on #14877

yinghu5 added 2 commits April 29, 2019 14:10

Update ChooseBlas.cmake

e767131

fix the mxnet cmake build systems to make MKLDNN, MKL logic correct.

Update FindMKL.cmake

c1a3e7c

unlock MKLDNN and MKL dependency

yinghu5 requested a review from szha as a code owner April 29, 2019 08:25

pengzhao-intel changed the title ~~Yinghu5 cmake 1~~ Fix the incorrect MKLDNN/MKL logic in cmake Apr 29, 2019

pengzhao-intel mentioned this pull request Apr 29, 2019

[Discussion] 1.5.0 Roadmap #14619

Closed

marcoabreu added the CMake CMake related bugs/issues/improvements label Apr 29, 2019

yinghu5 added 6 commits April 30, 2019 16:17

Update build_windows.py

024c10e

Add WIN_CPU_MKLDNN WIN_CPU_MKLDNN_MKL

Update Jenkinsfile_unix_gpu

23cc19d

update for mkl/mkldnn cmake test

Update Jenkins_steps.groovy

f0d5e41

Update Jenkinsfile_windows_cpu

34685fa

custom_steps.compile_windows_cpu_mkldnn(), custom_steps.compile_windows_cpu_mkldnn_mkl(), custom_steps.compile_windows_cpu_nomkldnn_mkl()

Update Jenkins_steps.groovy

870667a

WIN_CPU_MKLDNN

Update build_windows.py

0996b82

WIN_CPU_NOMKLDNN_MKL

yinghu5 closed this Apr 30, 2019

yinghu5 reopened this Apr 30, 2019

lebeg reviewed May 2, 2019

View reviewed changes

pengzhao-intel mentioned this pull request May 4, 2019

fixing logic in USE_MKLDNN #14857

Closed

7 tasks

yinghu5 closed this May 5, 2019

yinghu5 deleted the yinghu5-cmake-1 branch May 5, 2019 06:32

yinghu5 mentioned this pull request May 5, 2019

Fix the incorrect MKLDNN/MKL logic in cmake #14877

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the incorrect MKLDNN/MKL logic in cmake #14829

Fix the incorrect MKLDNN/MKL logic in cmake #14829

yinghu5 commented Apr 29, 2019

yinghu5 commented Apr 29, 2019

pengzhao-intel commented Apr 29, 2019

pengzhao-intel commented Apr 29, 2019

roywei commented Apr 29, 2019

azai91 commented Apr 29, 2019

pengzhao-intel commented Apr 30, 2019

yinghu5 commented Apr 30, 2019 •

edited

Loading

pengzhao-intel commented Apr 30, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019 •

edited

Loading

Chancebair commented May 2, 2019

lebeg commented May 2, 2019

lebeg May 2, 2019

yinghu5 May 4, 2019 •

edited

Loading

TaoLv commented May 4, 2019

TaoLv commented May 5, 2019

yinghu5 commented May 6, 2019

yinghu5 commented May 6, 2019

yinghu5 commented May 6, 2019

yinghu5 commented May 6, 2019

Chancebair commented May 7, 2019

pengzhao-intel commented May 8, 2019

Fix the incorrect MKLDNN/MKL logic in cmake #14829

Fix the incorrect MKLDNN/MKL logic in cmake #14829

Conversation

yinghu5 commented Apr 29, 2019

Description

Checklist

Essentials

Changes

Comments

yinghu5 commented Apr 29, 2019

pengzhao-intel commented Apr 29, 2019

pengzhao-intel commented Apr 29, 2019

roywei commented Apr 29, 2019

azai91 commented Apr 29, 2019

pengzhao-intel commented Apr 30, 2019

yinghu5 commented Apr 30, 2019 • edited Loading

pengzhao-intel commented Apr 30, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019

larroy commented May 1, 2019 • edited Loading

Chancebair commented May 2, 2019

lebeg commented May 2, 2019

lebeg May 2, 2019

Choose a reason for hiding this comment

yinghu5 May 4, 2019 • edited Loading

Choose a reason for hiding this comment

TaoLv commented May 4, 2019

TaoLv commented May 5, 2019

yinghu5 commented May 6, 2019

yinghu5 commented May 6, 2019

yinghu5 commented May 6, 2019

yinghu5 commented May 6, 2019

Chancebair commented May 7, 2019

pengzhao-intel commented May 8, 2019

yinghu5 commented Apr 30, 2019 •

edited

Loading

larroy commented May 1, 2019 •

edited

Loading

yinghu5 May 4, 2019 •

edited

Loading