[BUGFIX] Fix building when USE_BLAS option is not provided #19832
Conversation
|
Hey @akarbown , Thanks for submitting the PR
CI supported jobs: [unix-gpu, centos-gpu, edge, clang, windows-gpu, miscellaneous, unix-cpu, website, sanity, centos-cpu, windows-cpu] Note: |
cmake/ChooseBlas.cmake
Outdated
| if(MKL_FOUND) | ||
| set(BLAS "MKL") | ||
| endif() | ||
| set(BLAS "MKL") |
There was a problem hiding this comment.
Wouldn't this fail if MKL is not available?
There was a problem hiding this comment.
Yes, it'll fail. I need to rethink it.
There was a problem hiding this comment.
I think that now it's difficult to recreate the previous logic and what's more the USE_MKL_IF_AVAILABLE variable seems to be unnecessary. What do you think about removing the USE_MKL_IF_AVAILABLE for the whole project and as a consequence of that rely only on the USE_BLAS command line option?
Thus, when USE_BLAS will be set to 'mkl' and there will be no MKL libraries - the build should fail.
There was a problem hiding this comment.
Or maybe I missed something?
There was a problem hiding this comment.
Line 18 above initializes set(BLAS "Open") and only if MKL can be found, it's overwritten to BLAS=MKL.
Do you agree? If so, I think the current PR isn't needed.
With respect to USE_MKL_IF_AVAILABLE, it's fine with me to remove the variable and just check for MKL by default. As long as user doesn't specify which BLAS library they want, I think it's reasonable for MXNet to check for BLAS libraries in MXNet specified order of priority (ie. 1st prio is MKL, if not available try OpenBlas and otherwise use whatever is available..)
There was a problem hiding this comment.
Line 18 above initializes set(BLAS "Open") and only if MKL can be found, it's overwritten to BLAS=MKL.
Do you agree? If so, I think the current PR isn't needed.
If there is no 'MKL' the following function: find_package(MKL) is looking for the MKL through FindMKL.cmake file and produce the following warning:
CMake Warning at cmake/ChooseBlas.cmake:26 (find_package):
By not providing "FindMKL.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "MKL", but
CMake did not find one.
Could not find a package configuration file provided by "MKL" with any of
the following names:
MKLConfig.cmake
mkl-config.cmake
Add the installation prefix of "MKL" to CMAKE_PREFIX_PATH or set "MKL_DIR"
to a directory containing one of the above files. If "MKL" provides a
separate development package or SDK, be sure it has been installed.
Call Stack (most recent call first):
CMakeLists.txt:312 (include)
I wanted to clear it out with that PR.
There was a problem hiding this comment.
Thanks for clarifying. One option is to move the code from elseif(BLAS STREQUAL "MKL" OR BLAS STREQUAL "mkl") case up here to the beginning of the file and return from file immediately if MKL is found, or proceed to the other options if MKL isn't found. Another option as you mention is to remove USE_MKL_IF_AVAILABLE. Either is fine with me
|
@mxnet-bot run ci [unix-cpu] |
|
Jenkins CI successfully triggered : [unix-cpu] |
|
@mxnet-bot run ci [unix-cpu] |
|
Jenkins CI successfully triggered : [unix-cpu] |
akarbown
force-pushed
the
remove-find-mkl
branch
from
88cb589 to
2d838b8
Compare
akarbown
force-pushed
the
remove-find-mkl
branch
from
2d838b8 to
dd79ed4
Compare
|
@mxnet-bot run ci [unix-cpu, windows-gpu] |
|
Jenkins CI successfully triggered : [unix-cpu, windows-gpu] |
akarbown
changed the title
|
@mxnet-bot run ci [windows-gpu] |
|
Jenkins CI successfully triggered : [windows-gpu] |
lanking520
added
pr-awaiting-testing
Description
Fix building when USE_BLAS option is not provided in the compilation command line.
Comments
Overlooked in #19766 .