-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
BLD, ENH: Enable Accelerate Framework #18874
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
Conversation
Lint failures:
|
Does the new accelerate solve the threading issues? What LAPACK version (api) is supported? |
Also, which OS X versions have the new library? @rgommers @WarrenWeckesser @mattip @seberg Thoughts? |
From when I last looked at this, I think it just kills programs that try to use it after forking with a message to the console. So I guess in some sense that is solved (better than segfaulting, which was the previous behavior). Though probably still not desirable |
Thanks @Matthew-Badin. Here is the full story from when we dropped it: https://github.com/scipy/scipy/wiki/Dropping-support-for-Accelerate. I've had a quick look at the Accelerate docs, it doesn't say what LAPACK version is used. For NumPy that's probably not an issue; for SciPy we require >= 3.4.1 |
Hi Everyone,
Thank you! |
@Matthew-Badin - thanks very much for doing this. I would be very glad if we could continue to support Accelerate. I think I'm right in saying that one issue is that our development model means we need some channel of communication that we can rely on for timely, public feedback when problems arise or we have questions. Is there a mechanism for doing that? Do y'all monitor Github often, and can you respond here? |
Developer technical support will be monitoring this repository and will notify us if something comes up, additionally I will periodically check as well. We intend to address issues promptly and plan to continue supporting and updating our BLAS and LAPACK libraries. Additionally, filing bugs against Accelerate using the developer feedback assistant tool (https://developer.apple.com/bug-reporting/) is a great way to get our attention and helps us prioritize work. |
numpy/distutils/system_info.py
Outdated
order_env_var_name = 'NPY_LAPACK_ORDER' | ||
|
||
def _calc_info_accelerate(self): |
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.
These _calc_info_accelerate
blocks are just moved around without any changes, right? It would be nice to undo that to clean up this PR diff. Then I think this patch is good to go.
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.
Good idea, done =).
I'm a bit concerned, especially as the This could use a release note explaining the availability and version dependence. See |
How would this affect downstream projects? If numpy is installed with Accelerate, scipy rejects Accelerate because of the ancient LAPACK version and uses a different lapack library. In that case two BLAS/LAPACK libraries would be loaded. |
Good question. Mixing numpy and scipy with different versions of OpenBLAS inside has always worked, but I think that was just luck. Accelerate uses the g77 ABI, so when SciPy uses OpenBLAS (gfortran ABI) that could very well blow up. On the other hand, SciPy did ban Accelerate first when NumPy still allowed it, and I don't remember user complaints at that time. tl;dr dunno, should be tested:) |
I was fairly certain numpy relies on a LAPACK bugfix that wasn't introduced til 3.2.2 (xref #9980 and #9976 (comment)). Accelerate has been on version 3.2.1 for a long time; so I'm not sure the situation of using it now is any better than using it then. Unless of course Apple have forked LAPACK and not updated the version number to reflect that... |
Thanks @Matthew-Badin . |
Should that requirement of Lapack 3.2.2 (if it exists) affect, trip the tests? I guess if that is the case, we don't have to worry much. |
@charris Thank you =). |
This pull request is to add support for Accelerate back to NumPy and is based on the pull request that removed support (#15759). With the current version of macOS, all known blocking issues should be fixed. This was verified with the current main using “python3 runtest.py -v -m full”. If additional issues are found in the current build, the best way to reach out to us is through our developer feedback assistant tool (https://developer.apple.com/bug-reporting/). This is the easiest way for us to aggregate issues and prioritize work.