-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CMake: Fix MKL detection when not using environment modules #2443
Conversation
If MKLROOT is set (e.g., `source /opt/intel/oneapi/setvars.sh` will do that), first try FindBLAS instead of just relying on MKL_INCDIR and MKL_LIB. Fixes AMICI-dev#2441.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2443 +/- ##
========================================
Coverage 77.64% 77.64%
========================================
Files 324 324
Lines 20930 20930
Branches 1464 1464
========================================
Hits 16251 16251
Misses 4676 4676
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. |
# Was MKLROOT set by /opt/intel/oneapi/setvars.sh? then cmake will find it | ||
message(STATUS "Trying to find BLAS based on MKLROOT=$ENV{MKLROOT}") | ||
# give the user the option to override the BLA_VENDOR | ||
if(NOT DEFINED BLA_VENDOR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add message about assumed default here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default is that BLA_VENDOR is undefined and that any BLAS is accepted.
I add a message with the current value.
if(NOT DEFINED BLA_VENDOR) | ||
set(BLA_VENDOR Intel10_64lp) | ||
endif() | ||
find_package(BLAS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really only going to work if MKLROOT
is set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
find_package(BLAS)
? No, generally MKLROOT does not need to be set. But to use the Intel MKL, it is most likely required. I'd have to check the full cmake module to see which other environment variables might be considered, but with a default installation, it only works after setting certain env variables, and MKLROOT is one of them.
The find_package(BLAS)
case without MKLROOT is handled further down.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, missed the if(DEFINED ENV{MKLROOT})
if(NOT DEFINED BLA_VENDOR) | ||
set(BLA_VENDOR Intel10_64lp) | ||
endif() | ||
find_package(BLAS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, missed the if(DEFINED ENV{MKLROOT})
If MKLROOT is set (e.g.,
source /opt/intel/oneapi/setvars.sh
will do that), first try FindBLAS instead of just relying on MKL_INCDIR and MKL_LIB.Fixes #2441.