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

[FEATURE] Enable dynamic linking with MKL and compiler based OpenMP #20474

Merged
merged 16 commits into from
Oct 13, 2021
Merged

[FEATURE] Enable dynamic linking with MKL and compiler based OpenMP #20474

merged 16 commits into from
Oct 13, 2021

Conversation

akarbown
Copy link
Contributor

OneMKL 2021.3 fixed linking OpenMP while using SDL and MKL_THREADING_LAYER set to GNU.

Description

OneMKL 2021.3 fixes the issue described here. Thus, it enables linking with MKL dynamic libraries without having multiple OneMPs in a single process. It is possible due to linking MxNET with oneMKL Single Dynamic Library (SDL) and then setting the appropriate threading layer at run time in a function mkl_threading_layer() (or through environment variable MKL_THREADING_LAYER).

Connected with: [#19610], [#18255] and [#17794].

Changes

  1. Add oneMKL 2021.3 to ubuntu docker images.
  2. Enable MKL SDL (MKL_USE_SINGLE_DYNAMIC_LIBRARY) as the default linking when MKL version is grower than 2021.2 and static linking is turned off. (Bug no: MKLD-11109, OneMKL release notes) .
  3. Otherwise, MKL static libraries are taken into account and used to build MxNET library.
  4. Add support of the new oneMKL file structure in the FindBLAS.cmake file (fix comes from the cmake 3.20: #6210 ).

Comments

Does using oneMKL 2021.3 as the recommended one should be mentioned in the documentation?

@mxnet-bot
Copy link

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

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 Jul 30, 2021
@akarbown akarbown changed the title [FEATURE] Enables dynamic linking with MKL and compiler based OpenMP [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP Jul 30, 2021
leezu
leezu reviewed Jul 30, 2021
View reviewed changes
src/initialize.cc Outdated
#if defined( __INTEL_LLVM_COMPILER)
mkl_set_threading_layer(MKL_THREADING_INTEL);
#else
mkl_set_threading_layer(MKL_THREADING_GNU);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work with Windows? Intel developer's reference states "for GNU threading on Linux* operating system only" https://software.intel.com/content/www/us/en/develop/documentation/onemkl-developer-reference-c/top/support-functions/single-dynamic-library-control/mkl-set-threading-layer.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I forgot about Windows! I'll exclude it in a moment.

@akarbown akarbown force-pushed the compiler-based-openmp2 branch from 56c1404 to b3c66c6 Compare July 30, 2021 19:56
@akarbown
Copy link
Contributor Author

akarbown commented Aug 2, 2021

@mxnet-bot run ci[sanity]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [sanity]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 2, 2021

@mxnet-bot run ci [all]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 2, 2021

@mxnet-bot run ci[all]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 2, 2021

@leezu, could you help me with rerunning 'sanity' check in this PR? I've checked it locally and I don't see any issues. I suppose that this sanity check failed because of the timeout (SIGTERM). Is it possible?

@leezu
Copy link
Contributor

leezu commented Aug 2, 2021

@josephevans can we extend the max time for sanity? This PR triggers rebuild of the Docker used for Sanity, and apparently thus timeouts

@akarbown
Copy link
Contributor Author

akarbown commented Aug 3, 2021

@mxnet-bot run ci [unix-cpu]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 5, 2021

@mxnet-bot run ci[website]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [website]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 5, 2021

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

@mxnet-bot
Copy link

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

@akarbown
Copy link
Contributor Author

akarbown commented Aug 6, 2021

@mxnet-bot run ci[unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 6, 2021

@mxnet-bot run ci[unix-gpu, miscellaneous]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu, miscellaneous]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 6, 2021

@mxnet-bot run ci[miscellaneous]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [miscellaneous]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 6, 2021

@mxnet-bot run ci[miscellaneous]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [miscellaneous]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 9, 2021

@mxnet-bot run ci[miscellaneous]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [miscellaneous]

@akarbown
Copy link
Contributor Author

akarbown commented Aug 9, 2021

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

@mxnet-bot
Copy link

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

@akarbown
Copy link
Contributor Author

akarbown commented Aug 9, 2021

@mxnet-bot run ci[unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu]

akarbown added 10 commits October 12, 2021 22:03
@akarbown
Cleaning MKL find_path cmake directories
6f628a7
@akarbown
[WIP] Adding github runner for MAC OS to check MKL specific changes
ae6686b 
This is a temporary change to check if adding MKL runtime support
won't crash MacOS.
@akarbown
clang format + mkl workflow rename
59db10d
@akarbown
Fixing some formatting + installing patchelf
32aa4cf
@akarbown
setting up Mac OS rpath for MKL libraries
dde35e4
@akarbown
Run only mkl tests
4b80278
@akarbown
Fix for finding MKL libraries on MacOs by FindBLAS.cmake
30f90b5 
Turn off SDL for MKL on MacOS as it need fixes.
@akarbown
Enable linking MxNET with MKL static libraries on MacOS
5195792 
Add proper mkl_threading flags for Mac Os.
Enable all tests that are for MacOS + MKL tests.
Rebuild numpy with MKL BLAS (instead of OpenBLAS).
@akarbown
Excluding MKL bf16 tests as CI MacOs machines seems not to have avx512
ca1bcbe 
support.
@akarbown
Remove forcing rpath and some unnecessary comments
3cd9340
@akarbown akarbown force-pushed the compiler-based-openmp2 branch from 1041620 to 3cd9340 Compare October 12, 2021 20:10
@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 12, 2021
@akarbown
Copy link
Contributor Author

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

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu, unix-gpu, 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 Oct 13, 2021
@akarbown
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 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! Thanks!

@leezu leezu merged commit abd293f into apache:master Oct 13, 2021
@leezu
Copy link
Contributor

leezu commented Oct 13, 2021

Thank you @akarbown!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@leezu leezu leezu approved these changes

@barry-jin barry-jin barry-jin approved these changes

@bgawrych bgawrych bgawrych approved these changes

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

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

@marcoabreu marcoabreu Awaiting requested review from marcoabreu marcoabreu 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.
8 participants
@akarbown @mxnet-bot @leezu @mozga-intel @barry-jin @szha @bgawrych @mseth10