You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2021/07/30 11:17:05 UTC
[GitHub] [incubator-mxnet] akarbown opened a new pull request #20474: [FEATURE] Enables dynamic linking with MKL and compiler based OpenMP
akarbown opened a new pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474
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](https://github.com/apache/incubator-mxnet/issues/18255#issuecomment-817830091). 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](https://software.intel.com/content/www/us/en/develop/articles/intel-oneapi-math-kernel-library-onemkl-bug-fixes.html)) .
4. Otherwise, MKL static libraries are taken into account and used to build MxNET library.
5. Add support of the new oneMKL file structure in the FindBLAS.cmake file (fix comes from the cmake 3.20: [#6210](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6210) ).
## Comments ##
Does using oneMKL 2021.3 as the recommended one should be mentioned in the documentation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-924060582
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] barry-jin commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
barry-jin commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r724577019
##########
File path: tools/dependencies/mkl.sh
##########
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This script downloads OneMKL
+# TODO @akarbown: Get more general link to download the OneMKL (instead of the generated one)
Review comment:
It could be better to create an issue to track this instead of adding TODO here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-905257668
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895059739
Jenkins CI successfully triggered : [miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-893771553
@mxnet-bot run ci[website]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-896003060
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-942418558
Thank you @akarbown!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-926007040
@mxnet-bot run ci[centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937905897
@mxnet-bot run ci[windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894091824
Jenkins CI successfully triggered : [unix-gpu, miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-926007122
Jenkins CI successfully triggered : [centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725708632
##########
File path: tools/staticbuild/build_lib.sh
##########
@@ -36,13 +40,18 @@ cmake -GNinja -C $cmake_config \
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
..
ninja
+if [[ ! $PLATFORM == 'darwin' ]] && [[ $BLAS == 'mkl' ]]; then
+ patchelf --set-rpath "/opt/intel/oneapi/mkl/${INTEL_MKL}/lib/intel64/:\$ORIGIN" --force-rpath libmxnet.so
Review comment:
Yes, sure. However, I've just realized that when linking with MKL static libraries there is no need to ``source /opt/intel/oneapi/setvars.sh``. It's mandatory in case of linking with MKL dynamic libraries. Thanks for pointing that out!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-891024573
@mxnet-bot run ci[sanity]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894027257
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-893806687
Jenkins CI successfully triggered : [unix-gpu, clang, miscellaneous, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r693964528
##########
File path: cmake/ChooseBlas.cmake
##########
@@ -155,14 +164,19 @@ elseif(BLAS STREQUAL "MKL" OR BLAS STREQUAL "mkl")
endif()
endif()
# Setting up BLAS_mkl_MKLROOT for non-Ubuntu 20.04 OSes
- find_path(BLAS_mkl_MKLROOT mkl PATHS $ENV{MKLROOT} ${INTEL_HOME_ROOT} ${INTEL_OPT_ROOT})
Review comment:
This find_path also contains checking ${INTEL_OPT_ROOT}/oneapi/mkl directory as OneMKL changed directory structure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-925182440
@akarbown what's the status of this PR? cc @barry-jin who is tracking the 2.0 beta release
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r724924216
##########
File path: tools/dependencies/mkl.sh
##########
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This script downloads OneMKL
+# TODO @akarbown: Get more general link to download the OneMKL (instead of the generated one)
Review comment:
I've just wrote the email to MKL team to get this link. I'll see what I get and depending on that I'll open the issue or just fix it (maybe with the MKL update as 2021.4 has already been released).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725245379
##########
File path: cmake/upstream/FindBLAS.cmake
##########
@@ -522,12 +525,16 @@ if(BLA_VENDOR MATCHES "Intel" OR BLA_VENDOR STREQUAL "All")
get_filename_component(BLAS_mkl_MKLROOT "${BLAS_mkl_MKLROOT}" DIRECTORY)
endif()
endif()
+ # MXNET NOTE: This change comes form the newest file version
+ # https://gitlab.kitware.com/cmake/cmake/-/issues/22295
Review comment:
Good point, I can remove that comment as this is in the upstream version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725240417
##########
File path: tools/staticbuild/build_lib.sh
##########
@@ -36,13 +40,18 @@ cmake -GNinja -C $cmake_config \
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
..
ninja
+if [[ ! $PLATFORM == 'darwin' ]] && [[ $BLAS == 'mkl' ]]; then
+ patchelf --set-rpath "/opt/intel/oneapi/mkl/${INTEL_MKL}/lib/intel64/:\$ORIGIN" --force-rpath libmxnet.so
Review comment:
_> Will oneapi/mkl always be in /opt/intel?_
I think that if the user do not define explicitly other location, the default location for oneMKL supposed to be /opt/intel/oneapi/ at least the way it was installed in the way as it is in the mkl.sh file.
_> Is there a reason for not asking users to fix their run-time search path environment variables instead (which ideally would automatically be set correctly upon installation of oneapi/mkl)?_
I've added it for the sake of the tests so that while running them (in the runtime) libmxnet.dylib could see the MKL libraries. It's not the best solution. Now, I think that maybe it would be better to ``source /opt/intel/oneapi/setvars.sh`` script just before the test execution (in the *.yml file). What do you think? Or did you have someting else in mind?
##########
File path: tools/staticbuild/build_lib.sh
##########
@@ -36,13 +40,18 @@ cmake -GNinja -C $cmake_config \
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
..
ninja
+if [[ ! $PLATFORM == 'darwin' ]] && [[ $BLAS == 'mkl' ]]; then
+ patchelf --set-rpath "/opt/intel/oneapi/mkl/${INTEL_MKL}/lib/intel64/:\$ORIGIN" --force-rpath libmxnet.so
Review comment:
_> Will oneapi/mkl always be in /opt/intel?_
I think that if the user do not define explicitly other location, the default location for oneMKL supposed to be /opt/intel/oneapi/ at least the way it was installed in the way as it is in the mkl.sh file.
_> Is there a reason for not asking users to fix their run-time search path environment variables instead (which ideally would automatically be set correctly upon installation of oneapi/mkl)?_
I've added it for the sake of the tests so that while running them (in the runtime) libmxnet.dylib could see the MKL libraries. It's not the best solution. Now, I think that maybe it would be better to ``source /opt/intel/oneapi/setvars.sh`` script just before the test execution (in the *.yml file). What do you think? Or did you have something else in mind?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-893806424
@mxnet-bot run ci[clang, miscellaneous, unix-cpu, unix-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-905697665
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895342782
@mxnet-bot run ci[unix-cpu, unix-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-904390250
> > Hi @akarbown It looks really good. Thanks! I have a question.
> > Basically, there are two ways: either we'll use MKL_THREADING_GNU or MKL_THREADING_INTEL. Could you please tell me whether there are differences in performance results, or, there is no impact on that?
>
> @mozga-intel, this boils down to comparison between ICX and GCC compilers. The whole point of this PR is mainly to enable using dynamic linking of MKL libraries without hang or different OpenMP symbol conflicts. Thus performance of this 2 compilers is not really an issue here. Unit tests for both compilers run more or less in the same time (on my local machine). I've observed 10 more tests failures for icx than for gcc (unittest_ubuntu_python3_cpu), but it seems to be because of the fact that it wasn't tested before and there were problem with compiling MxNET with icc at all.
Thanks, Anna! I have one more question: it might not exactly precisely aims at an issue like this. Please have a look at the scenario like this: If I have huge matrices then I can use one of those functionalities: either sparse or dense BLAS.
If I try to use the IIL64 interface for huge matrices on a 32-bit architecture, will I get either an error message or everything is okay? (...) 32-bits architecture doesn't support the ILP64 interface, and LP64 works only for a small tensor. Could be possible that it works for dense matrices but for a sparse I will get an error? Or, maybe the matrices will be divided into small matrices [?]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-905697746
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-896880795
@mxnet-bot run ci[centos-gpu, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895059663
@mxnet-bot run ci[miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725261503
##########
File path: tools/dependencies/mkl.sh
##########
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This script downloads OneMKL
+# TODO @akarbown: Get more general link to download the OneMKL (instead of the generated one)
Review comment:
@barry-jin I've created the issue for that #20655. Thus, I'll remove that comment form the code. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-896880883
Jenkins CI successfully triggered : [centos-gpu, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-891325860
@josephevans can we extend the max time for sanity? This PR triggers rebuild of the Docker used for Sanity, and apparently thus timeouts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-898461796
Jenkins CI successfully triggered : [centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-893771616
Jenkins CI successfully triggered : [website]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mozga-intel commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-897506723
Hi @akarbown It looks really good. Thanks! I have a question.
Basically, there are two ways: either we'll use MKL_THREADING_GNU or MKL_THREADING_INTEL. Could you please tell me whether there are differences in performance results, or, there is no impact on that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-941956136
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-891024654
Jenkins CI successfully triggered : [sanity]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r680104976
##########
File path: src/initialize.cc
##########
@@ -93,6 +95,13 @@ LibraryInitializer::LibraryInitializer()
cpu_worker_nthreads_(dmlc::GetEnv("MXNET_CPU_WORKER_NTHREADS", 1)),
mp_cv_num_threads_(dmlc::GetEnv("MXNET_MP_OPENCV_NUM_THREADS", 0)) {
dmlc::InitLogging("mxnet");
+#if MKL_USE_SINGLE_DYNAMIC_LIBRARY
+ #if defined( __INTEL_LLVM_COMPILER)
+ mkl_set_threading_layer(MKL_THREADING_INTEL);
+ #else
+ mkl_set_threading_layer(MKL_THREADING_GNU);
Review comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894185572
@mxnet-bot run ci[miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-905257721
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-925190404
> @akarbown what's the status of this PR? cc @barry-jin who is tracking the 2.0 beta release
IMHO, it's done! @szha suggested to check if it works on Mac. As I haven't got any MacOS close at hand I'm trying to do it as github action.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895918437
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937496105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937905979
Jenkins CI successfully triggered : [windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725708632
##########
File path: tools/staticbuild/build_lib.sh
##########
@@ -36,13 +40,18 @@ cmake -GNinja -C $cmake_config \
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
..
ninja
+if [[ ! $PLATFORM == 'darwin' ]] && [[ $BLAS == 'mkl' ]]; then
+ patchelf --set-rpath "/opt/intel/oneapi/mkl/${INTEL_MKL}/lib/intel64/:\$ORIGIN" --force-rpath libmxnet.so
Review comment:
Yes, sure. However, I've just realized that when linking with MKL static libraries there is no need to ``source /opt/intel/oneapi/setvars.sh``. It's mandatory in case of linking with MKL dynamic libraries. Thanks for pointing that out on that!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725695236
##########
File path: tools/staticbuild/build_lib.sh
##########
@@ -36,13 +40,18 @@ cmake -GNinja -C $cmake_config \
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
..
ninja
+if [[ ! $PLATFORM == 'darwin' ]] && [[ $BLAS == 'mkl' ]]; then
+ patchelf --set-rpath "/opt/intel/oneapi/mkl/${INTEL_MKL}/lib/intel64/:\$ORIGIN" --force-rpath libmxnet.so
Review comment:
I assume `source /opt/intel/oneapi/setvars.sh` is also what users would be expected to do if they install MKL on Mac? If so, I think that'll be more robust than hardcoding the rpath
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-891085500
@mxnet-bot run ci[all]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-939390250
@mxnet-bot run ci [windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-942198027
@mxnet-bot run ci[centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895918398
@mxnet-bot run ci[unix-cpu, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel edited a comment on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mozga-intel edited a comment on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-904390250
> > Hi @akarbown It looks really good. Thanks! I have a question.
> > Basically, there are two ways: either we'll use MKL_THREADING_GNU or MKL_THREADING_INTEL. Could you please tell me whether there are differences in performance results, or, there is no impact on that?
>
> @mozga-intel, this boils down to comparison between ICX and GCC compilers. The whole point of this PR is mainly to enable using dynamic linking of MKL libraries without hang or different OpenMP symbol conflicts. Thus performance of this 2 compilers is not really an issue here. Unit tests for both compilers run more or less in the same time (on my local machine). I've observed 10 more tests failures for icx than for gcc (unittest_ubuntu_python3_cpu), but it seems to be because of the fact that it wasn't tested before and there were problem with compiling MxNET with icc at all.
Thanks, Anna! I have one more question: it might not exactly aim at an issue like this. Please have a look at the scenario like this: If I have huge matrices then I can use one of those functionalities: either sparse or dense BLAS.
If I try to use the IIL64 interface for huge matrices on a 32-bit architecture, will I get either an error message or everything is okay? (...) 32-bits architecture doesn't support the ILP64 interface, and LP64 works only for a small tensor. Could be possible that it works for dense matrices but for a sparse I will get an error? Or, maybe the matrices will be divided into small matrices [?]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894269287
@mxnet-bot run ci[miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-896002984
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-924944582
Jenkins CI successfully triggered : [centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725211061
##########
File path: tools/staticbuild/build_lib.sh
##########
@@ -36,13 +40,18 @@ cmake -GNinja -C $cmake_config \
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
..
ninja
+if [[ ! $PLATFORM == 'darwin' ]] && [[ $BLAS == 'mkl' ]]; then
+ patchelf --set-rpath "/opt/intel/oneapi/mkl/${INTEL_MKL}/lib/intel64/:\$ORIGIN" --force-rpath libmxnet.so
Review comment:
Will oneapi/mkl always be in `/opt/intel`? Is there a reason for not asking users to fix their run-time search path environment variables instead (which ideally would automatically be set correctly upon installation of oneapi/mkl)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-941956136
Jenkins CI successfully triggered : [unix-cpu, unix-gpu, centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937496105
Jenkins CI successfully triggered : [centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu merged pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu merged pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895918904
@mxnet-bot run ci[centos-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-891101041
@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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-905286788
> > > Hi @akarbown It looks really good. Thanks! I have a question.
> > > Basically, there are two ways: either we'll use MKL_THREADING_GNU or MKL_THREADING_INTEL. Could you please tell me whether there are differences in performance results, or, there is no impact on that?
> >
> >
> > @mozga-intel, this boils down to comparison between ICX and GCC compilers. The whole point of this PR is mainly to enable using dynamic linking of MKL libraries without hang or different OpenMP symbol conflicts. Thus performance of this 2 compilers is not really an issue here. Unit tests for both compilers run more or less in the same time (on my local machine). I've observed 10 more tests failures for icx than for gcc (unittest_ubuntu_python3_cpu), but it seems to be because of the fact that it wasn't tested before and there were problem with compiling MxNET with icc at all.
>
> Thanks, Anna! I have one more question: it might not exactly aim at an issue like this. Please have a look at the scenario like this: If I have huge matrices then I can use one of both modes: either sparse or dense BLAS. What would I get, If I tried to call the IIL64 interface for huge matrices on a 32-bit architecture? Will I get an error message or will everything be okay? (...) 32-bits architecture doesn't support the ILP64 interface, and LP64 works only for a small tensor.
@mozga-intel Thanks for the questions!
MxNET MKL ILP64 (USE_INT64_TENSOR_SIZE) is a cmake dependent option and depends on the architecture. Thus, if you do not force that option to 'ON' it for sure on 32-bit architecture is 'OFF' (please look at CMakeLists.txt l.313). FindBLAS.cmake for 32-bit architecture sets BLA_VENDOR to 'Intel10_32' and depending on the SDL option links the following libraries:
1. SDL=1: ```Found BLAS: /opt/intel/oneapi/lib/ia32/libmkl_rt.so;-lpthread;-lm;-ldl```
2. SDL=0: ```Found BLAS: -Wl,--start-group;/opt/intel/oneapi/lib/ia32/libmkl_intel.so;/opt/intel/oneapi/lib/ia32/libmkl_intel_thread.so;/opt/intel/oneapi/lib/ia32/libmkl_core.so;-Wl,--end-group;/opt/intel/oneapi/lib/ia32/libiomp5.so;-lpthread;-lm;-ldl```
Those libraries are linked independently of the following options: BLA_VENDOR or USE_INT64_TENSOR_SIZE. The error won't appear as the architecture and the libraries are chosen correctly (as far as I've checked).
I've asked you on the IM for the command line you want me to run and it would be really nice to get it. However, I don't see 32-bit testing and building scenario in the MxNET but I might have overlooked it - could you please provide me also which configuration: OS/docker you thought about?
I've checked that MxNET has some building issues on the 32-bit architecture thus I do not think that spending more time on that issue is in the scope of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r701445202
##########
File path: cmake/ChooseBlas.cmake
##########
@@ -155,14 +164,19 @@ elseif(BLAS STREQUAL "MKL" OR BLAS STREQUAL "mkl")
endif()
endif()
# Setting up BLAS_mkl_MKLROOT for non-Ubuntu 20.04 OSes
- find_path(BLAS_mkl_MKLROOT mkl PATHS $ENV{MKLROOT} ${INTEL_HOME_ROOT} ${INTEL_OPT_ROOT})
Review comment:
Fixed as suggested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] barry-jin commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
barry-jin commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r724577019
##########
File path: tools/dependencies/mkl.sh
##########
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This script downloads OneMKL
+# TODO @akarbown: Get more general link to download the OneMKL (instead of the generated one)
Review comment:
It could be better to create an issue to track this instead of adding TODO here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937763996
@mxnet-bot run ci[centos-gpu, windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895438649
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895342859
Jenkins CI successfully triggered : [unix-cpu, unix-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] barry-jin commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
barry-jin commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-939136434
@mxnet-bot run ci [windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-939136457
Jenkins CI successfully triggered : [windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-941956078
@mxnet-bot run ci[unix-cpu, unix-gpu, centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-939390275
Jenkins CI successfully triggered : [windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-942418558
Thank you @akarbown!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-912274630
@mxnet-bot run ci[centos-gpu, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937024513
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-926111477
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-898461742
@mxnet-bot run ci[centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu merged pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu merged pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-891680953
@mxnet-bot run ci [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mozga-intel edited a comment on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mozga-intel edited a comment on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-904390250
> > Hi @akarbown It looks really good. Thanks! I have a question.
> > Basically, there are two ways: either we'll use MKL_THREADING_GNU or MKL_THREADING_INTEL. Could you please tell me whether there are differences in performance results, or, there is no impact on that?
>
> @mozga-intel, this boils down to comparison between ICX and GCC compilers. The whole point of this PR is mainly to enable using dynamic linking of MKL libraries without hang or different OpenMP symbol conflicts. Thus performance of this 2 compilers is not really an issue here. Unit tests for both compilers run more or less in the same time (on my local machine). I've observed 10 more tests failures for icx than for gcc (unittest_ubuntu_python3_cpu), but it seems to be because of the fact that it wasn't tested before and there were problem with compiling MxNET with icc at all.
Thanks, Anna! I have one more question: it might not exactly aim at an issue like this. Please have a look at the scenario like this: If I have huge matrices then I can use one of both modes: either sparse or dense BLAS. What would I get, If I tried to call the IIL64 interface for huge matrices on a 32-bit architecture? Will I get an error message or will everything be okay? (...) 32-bits architecture doesn't support the ILP64 interface, and LP64 works only for a small tensor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937764071
Jenkins CI successfully triggered : [centos-gpu, windows-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] barry-jin commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
barry-jin commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-938204263
> Now it seems the change seems to be tested and checked for MacOS and with MKL BLAS.
> Do you think that leaving that new github action for MKL on MacOS make sense? If so, can it look as it is or change it somehow?
Yes, we need to keep this new github action as it will cover MKL related build and tests on MacOS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-938160202
@szha, @leezu, @barry-jin - can I ask you for the review of that PR?
I've added new github action pipeline to test and verify this change with oneMKL on MacOS. It revealed some things that were needed to fix for that OS i.e:
- remove using `` -Wl, --start-group/ -Wl, --end-group`` while linking static MKL libraries in FindBLAS.cmake (issue mentioned here: https://gitlab.kitware.com/cmake/cmake/-/issues/20548);
- set proper threading layer at runtime. According to the [documentation](https://software.intel.com/content/dam/develop/external/us/en/documents/onemkl-developerguide-mac.pdf) it need to be set mkl_set_threading_layer(MKL_THREADING_INTEL);
- added github action (using existing [os_x_staticbuild.yml](https://github.com/apache/incubator-mxnet/blob/master/.github/workflows/os_x_staticbuild.yml)) that is building OneMKL with static MKL libraries (to be consistent with already existing scripts) and run the same tests that are for os_x_staticbuild.yml plus MKL tests;
- fixed hangs that appeared while running those tests were the result of the numpy linking/using OpenBLAS instead of MKL BLAS and as a consequence it was linking libgomp which resulted in the hang (two OpenMP runtimes in one process). Recompiling it (done in the numpy_mkl.sh file) resolved the issue;
- excluded ``test_bf16_operator`` tests for that action pipeline as CI MacOS seems to not support avx512;
- tested locally MxNET linked with static, dynamic and SDL (Single Dynamic Library) on MacOS and all the tests (from the os_x_staticbuild.yml + MKL tests) seems to pass without any hang.
Now it seems the change seems to be tested and checked for MacOS and with MKL BLAS.
Do you think that leaving that new github action for MKL on MacOS make sense? If so, can it look as it is or change it somehow?
**Remark**: I see that windows-gpu fails, but it's rather not connected with that change but maybe with the VS 2019 version 16.11 Release? As I see that for v16.8.1 (MSVC 19.28.29333.0) it [passed](https://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/mxnet-validation/pipelines/windows-gpu/branches/PR-20474/runs/12/nodes/40/steps/84/log/?start=0) without any issues, while for v16.11.4 (MSVC 19.29.30136.0) it [fails](https://jenkins.mxnet-ci.amazon-ml.com/blue/rest/organizations/jenkins/pipelines/mxnet-validation/pipelines/windows-gpu/branches/PR-20474/runs/13/nodes/40/steps/84/log/?start=0). But I'm not 100% sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937496064
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894027305
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-891053295
@mxnet-bot run ci [all]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894185618
Jenkins CI successfully triggered : [miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-898455853
> Hi @akarbown It looks really good. Thanks! I have a question.
>
> Basically, there are two ways: either we'll use MKL_THREADING_GNU or MKL_THREADING_INTEL. Could you please tell me whether there are differences in performance results, or, there is no impact on that?
@mozga-intel, this boils down to comparison between ICX and GCC compilers. The whole point of this PR is mainly to enable using dynamic linking of MKL libraries without hang or different OpenMP symbol conflicts. Thus performance of this 2 compilers is not really an issue here. Unit tests for both compilers run more or less in the same time (on my local machine). I've observed 10 more tests failures for icx than for gcc (unittest_ubuntu_python3_cpu), but it seems to be because of the fact that it wasn't tested before and there were problem with compiling MxNET with icc at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-926111414
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725708632
##########
File path: tools/staticbuild/build_lib.sh
##########
@@ -36,13 +40,18 @@ cmake -GNinja -C $cmake_config \
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 \
..
ninja
+if [[ ! $PLATFORM == 'darwin' ]] && [[ $BLAS == 'mkl' ]]; then
+ patchelf --set-rpath "/opt/intel/oneapi/mkl/${INTEL_MKL}/lib/intel64/:\$ORIGIN" --force-rpath libmxnet.so
Review comment:
Yes, sure. However, I've just realized that when linking with MKL static libraries there is no need to ``source /opt/intel/oneapi/setvars.sh``. It's mandatory in case of linking with MKL dynamic libraries.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-924060486
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937024695
Jenkins CI successfully triggered : [unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] barry-jin commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
barry-jin commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-938204263
> Now it seems the change seems to be tested and checked for MacOS and with MKL BLAS.
> Do you think that leaving that new github action for MKL on MacOS make sense? If so, can it look as it is or change it somehow?
Yes, we need to keep this new github action as it will cover MKL related build and tests on MacOS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-941956078
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-937496064
@mxnet-bot run ci[centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-924944521
@mxnet-bot run ci[centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-942198064
Jenkins CI successfully triggered : [centos-gpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r725212081
##########
File path: cmake/upstream/FindBLAS.cmake
##########
@@ -522,12 +525,16 @@ if(BLA_VENDOR MATCHES "Intel" OR BLA_VENDOR STREQUAL "All")
get_filename_component(BLAS_mkl_MKLROOT "${BLAS_mkl_MKLROOT}" DIRECTORY)
endif()
endif()
+ # MXNET NOTE: This change comes form the newest file version
+ # https://gitlab.kitware.com/cmake/cmake/-/issues/22295
Review comment:
Do you mean you backported this change to our file? Or is our file now up-to-date with upstream again? If it's the latter, you don't need to add the note here. If it's the former, why not update to the latest upstream instead of backporting?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] barry-jin edited a comment on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
barry-jin edited a comment on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-938204263
> Now it seems the change seems to be tested and checked for MacOS and with MKL BLAS.
> Do you think that leaving that new github action for MKL on MacOS make sense? If so, can it look as it is or change it somehow?
Yes, we need to keep this new github action as it will cover MKL related build and tests on MacOS.
> Remark: I see that windows-gpu fails, but it's rather not connected with that change but maybe with the VS 2019 version 16.11 Release? As I see that for v16.8.1 (MSVC 19.28.29333.0) it passed without any issues, while for v16.11.4 (MSVC 19.29.30136.0) it fails. But I'm not 100% sure.
I have created a pull request #20647 to try to fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895438600
@mxnet-bot run ci[unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894269343
Jenkins CI successfully triggered : [miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-895918937
Jenkins CI successfully triggered : [centos-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enables dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-889823895
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r680118268
##########
File path: src/initialize.cc
##########
@@ -93,6 +95,13 @@ LibraryInitializer::LibraryInitializer()
cpu_worker_nthreads_(dmlc::GetEnv("MXNET_CPU_WORKER_NTHREADS", 1)),
mp_cv_num_threads_(dmlc::GetEnv("MXNET_MP_OPENCV_NUM_THREADS", 0)) {
dmlc::InitLogging("mxnet");
+#if MKL_USE_SINGLE_DYNAMIC_LIBRARY
+ #if defined( __INTEL_LLVM_COMPILER)
+ mkl_set_threading_layer(MKL_THREADING_INTEL);
+ #else
+ mkl_set_threading_layer(MKL_THREADING_GNU);
Review comment:
Good point! I forgot about Windows! I'll exclude it in a moment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] akarbown commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
akarbown commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-894091751
@mxnet-bot run ci[unix-gpu, miscellaneous]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#issuecomment-912274653
Jenkins CI successfully triggered : [centos-gpu, unix-cpu]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] bgawrych commented on a change in pull request #20474: [FEATURE] Enable dynamic linking with MKL and compiler based OpenMP
Posted by GitBox <gi...@apache.org>.
bgawrych commented on a change in pull request #20474:
URL: https://github.com/apache/incubator-mxnet/pull/20474#discussion_r693905429
##########
File path: cmake/ChooseBlas.cmake
##########
@@ -155,14 +164,19 @@ elseif(BLAS STREQUAL "MKL" OR BLAS STREQUAL "mkl")
endif()
endif()
# Setting up BLAS_mkl_MKLROOT for non-Ubuntu 20.04 OSes
- find_path(BLAS_mkl_MKLROOT mkl PATHS $ENV{MKLROOT} ${INTEL_HOME_ROOT} ${INTEL_OPT_ROOT})
Review comment:
Same line not changed in: https://github.com/apache/incubator-mxnet/pull/20474/files#R27
Is it done on purpose or just missed?
##########
File path: src/initialize.cc
##########
@@ -93,6 +95,24 @@ LibraryInitializer::LibraryInitializer()
cpu_worker_nthreads_(dmlc::GetEnv("MXNET_CPU_WORKER_NTHREADS", 1)),
mp_cv_num_threads_(dmlc::GetEnv("MXNET_MP_OPENCV_NUM_THREADS", 0)) {
dmlc::InitLogging("mxnet");
+
+#if !(defined(_WIN32) || defined(_WIN64) || defined(__WINDOWS__))
Review comment:
maybe it should be moved to a function like install_pthread_atfork_handlers - it probably will be more descriptive on what's happening here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org