Skip to content
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

BLD: Fixed detection for recent MKL versions #7908

Merged
merged 1 commit into from
Aug 8, 2016
Merged

Conversation

xabellan
Copy link

@xabellan xabellan commented Aug 5, 2016

The detection of the MKL library was broken. According to Intel, the recommended way to build numpy and use MKL is to define a site.cfg with the values:

library_dirs = /opt/intel/compilers_and_libraries_2016/linux/mkl/lib/intel64
include_dirs = /opt/intel/compilers_and_libraries_2016/linux/mkl/include
mkl_libs = mkl_rt
lapack_libs = 

See https://software.intel.com/en-us/articles/numpyscipy-with-intel-mkl for more info.

The name of the libraries in system_info has been updated so now no site.cfg is needed if the MKLROOT environment variable is properly set to detect MKL.

@@ -1030,20 +1027,7 @@ def calc_info(self):


class lapack_mkl_info(mkl_info):

Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for removing this? If the class is empty, a remark explaining why would be helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Any reason to leave the class rather than remove it completely?

Copy link
Author

Choose a reason for hiding this comment

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

I left it empty for consistency with the other mkl_info subclass blas_mkl_info, which was also empty. I guess you could remove both and just use the parent mkl_info class for both blas_mkl and lapack_mkl in the cl dictionary.

@charris
Copy link
Member

charris commented Aug 7, 2016

Does this work for older MKL also?

@charris
Copy link
Member

charris commented Aug 7, 2016

lapack_libs = seems a bit odd.

@charris
Copy link
Member

charris commented Aug 7, 2016

See also #7717.

@xabellan
Copy link
Author

xabellan commented Aug 8, 2016

It should work from MKL version 13 onwards. I tested on 13, 14, 15 and 16, which are the ones I have access to.

I also changed the is_Xeon check because it was not picking up the intel core i* family as 64 bit. I replaced it by a generic check for intel and 64 bit platform.

@charris
Copy link
Member

charris commented Aug 8, 2016

Well, let's give it a shot. Thanks @xabellan .

@charris charris merged commit e3a031d into numpy:master Aug 8, 2016
@charris charris added this to the 1.11.2 release milestone Aug 8, 2016
@charris charris removed this from the 1.11.2 release milestone Aug 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants