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

Change mkl_info.dir_env_var from MKL to MKLROOT #7723

Merged
merged 1 commit into from
Jun 11, 2016

Conversation

mingwandroid
Copy link
Contributor

@charris
Copy link
Member

charris commented Jun 11, 2016

Is this new and MKL was once valid, or has it always been MKLROOT?

@mingwandroid
Copy link
Contributor Author

I don't know how to determine that as I can only find the latest documentation on the website.

@mingwandroid
Copy link
Contributor Author

Actually, I found a reference for MKLROOT dating back to 2012:

https://software.intel.com/en-us/articles/intel-mkl-103-getting-started

@charris charris added this to the 1.11.1 release milestone Jun 11, 2016
@charris
Copy link
Member

charris commented Jun 11, 2016

Looks like MKL 10.0 came out in 2008 and your reference is 10.3, so we are probably good. Thanks @mingwandroid .

@rgommers
Copy link
Member

Does this actually make a difference? The mkl_info class already has a method get_mkl_rootdir that should look for MKLROOT and add it to the library and include paths.

@mingwandroid
Copy link
Contributor Author

It does make a difference. The values from get_mkl_root get written into site.cfg (AFAIR, I am not at my computer to check this filename) only.

The case that failed for me was building scipy using a numpy built on a different system so site.cfg contained paths that don't exist.

@rgommers
Copy link
Member

site.cfg doesn't get written, but __config__.py does get written and installed.

Scipy should not be using the paths in that file though. I can see how something would go wrong if get_info was imported from numpy.distutils instead of numpy.distutils.system_info - then you'd get the numpy build-time paths.

I have no problem with this PR, but it sounds like there's something else that needs fixing. The paths in __config__.py should never be reused for building another package.

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

3 participants