8000 numpy and scipy: drop Accelerate, use openblas by fxcoudert · Pull Request #33207 · Homebrew/homebrew-core · GitHub
[go: up one dir, main page]
Skip to content

numpy and scipy: drop Accelerate, use openblas #33207

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

numpy and scipy: drop Accelerate, use openblas #33207

wants to merge 2 commits into from

Conversation

fxcoudert
Copy link
Member
  • numpy and scipy have issues when compiled against Apple's BLAS/LAPACK, the Accelerate framework
  • until now we've preferred to use Accelerate because it's provided by the system
  • scipy is dropping support for Accelerate in the next version: https://github.com/scipy/scipy/wiki/Dropping-support-for-Accelerate
  • numpy's official wheels for macOS are built with OpenBLAS, meaning it is upstream's preferred choice of BLAS library

Thus, it makes sense to me to switch.

Fixes #31404
Thanks to @cmarquardt for reporting a clear analysis of the current situation and upstream policy changes.

@fxcoudert fxcoudert added the python Python use is a significant feature of the PR or issue label Oct 20, 2018
@fxcoudert
Copy link
Member Author

Got stuck almost at the end of testing 😢
@BrewTestBot test this please

@fxcoudert fxcoudert closed this in ed1ca79 Oct 20, 2018
@cmarquardt
Copy link
Contributor

Many thanks!

@fxcoudert fxcoudert deleted the numscipy branch October 20, 2018 19:45
@MikeMcQuaid
Copy link
Member

Came in here to disagree but ended up agreeing with the above reasoning. Nice work all 👍

@fxcoudert
Copy link
Member Author

Thanks @MikeMcQuaid, I hesitated to tag this one with a “maintainer feedback” label because I know we try hard to stick to macOS-provided libraries… but indeed, here it became untenable.

@QuLogic QuLogic mentioned this pull request Oct 21, 2018
Closed
@sjackman
Copy link
Contributor

cc @dpo

@dpo
Copy link
Contributor
dpo commented Oct 23, 2018

The issue with Accelerate is twofold:

  1. their LAPACK library is incomplete;
  2. there are bugs when using single precision (real and complex) routines.

Back in the days of homebrew/science, issue 2 prompted us to use vecLibFort, which effectively "patches" Accelerate. I believe it still does its job. If you wanted to stick with Accelerate as much as possible, it should be safe to use vecLibFort, unless you also need access to some of the missing LAPACK routines. I filed a bug report with Apple several years ago about issue 1 and I'm told a forthcoming update to Accelerate will correct it. It's not clear to me whether or not it will also resolve issue 2.

vecLibFort's advantage is that it's a thin interface that fixes issue 2 and so still allows you to use Accelerate. In a sense, it's minimally invasive.

OpenBLAS suffers from neither issue. I've observed it to be slightly slower than Accelerate in certain cases, but it's definitely a safe alternative.

One caveat is that it's possible to run into problems when a formula has a dependency that uses Accelerate (via vecLibFort or not) and another that uses OpenBLAS because both basically package the same libraries. One can shadow the other and/or the performance may not be what the user expects.

@cmarquardt
Copy link
Contributor

I believe there are more issues with Accelerate, like integer overflows even in BLAS routines. Apparently (somewhere in the middle of https://github.com/scipy/scipy/wiki/Dropping-support-for-Accelerate), the scipy folks also considered vecLibFort but concluded that the result remains too unstable for their purposes.

@lock lock bot added the outdated PR was locked due to age label Nov 22, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Nov 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@fxcoudert fxcoudert

Labels
outdated PR was locked due to age python Python use is a significant feature of the PR or issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fxcoudert @cmarquardt @MikeMcQuaid @sjackman @dpo
0