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

Fix ATLAS version detection #7840

Merged
merged 1 commit into from
Jul 19, 2016
Merged

Conversation

llasram
Copy link
Contributor

@llasram llasram commented Jul 15, 2016

ATLAS version detection still depends on the deprecated-and-removed get_output method. Restoring this function seems to be the easiest short-term fix.

In more detail: within numpy.distutils, the system_info.get_atlas_version() function is still implemented in terms of the config.config.get_output() method removed in release 1.10.0. The get_atlas_version() function is written in such a way that it swallows the unexpected AttributeError (or any other exception). When ATLAS is the selected BLAS implementation, this causes e.g. the get_info('blas_opt')['define_macros'] list to include ('NO_ATLAS_INFO', -1). It is unclear what this is intended to mean, and at least scikit-learn checks only for the presence of NO_ATLAS_INFO and not the associated value, as seen in scikit-learn/scikit-learn#5489.

Obviously longer-term numpy.distutils should switch to a more appropriate mechanism for identifying the ATLAS version, but restoring get_output() gets the existing detection function working until it can be re-implemented.

The ATLAS version check still depends on this function.
@charris
Copy link
Member

charris commented Jul 16, 2016

Could you expand on what the problem is?

@llasram
Copy link
Contributor Author

llasram commented Jul 16, 2016

@charris Sure thing! I updated the main PR description with more details.

@rgommers rgommers added this to the 1.11.2 release milestone Jul 19, 2016
@rgommers
Copy link
Member

Makes sense to me to revert the removal of get_output. May as well merge this, backport to 1.11.2, and then make a new issue for a better fix (which isn't super high prio imho).

@charris
Copy link
Member

charris commented Jul 19, 2016

@rgommers Thanks for taking a look.

@charris charris merged commit 93240e0 into numpy:master Jul 19, 2016
@charris
Copy link
Member

charris commented Jul 19, 2016

@llasram Thanks.

@charris charris removed this from the 1.11.2 release milestone Jul 19, 2016
@charris
Copy link
Member

charris commented Jul 19, 2016

Backported in #7851.

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