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

BUG: Prevent crashes due to MKL bug in ?heevr #11737

Conversation

congma
Copy link
Contributor

@congma congma commented Mar 28, 2020

In the LAPACK wrapper for ?heevr, enlarge the size of the isuppz array
parameter in order to prevent out-of-bound access by the underlying
library. For both ?heevr and ?syevr, more robust checks and default
values are also put in place.

Reference issue

Closes #11709

What does this implement/fix?

Crashes related to linalg.eigh()

Additional information

The current status is mostly a workaround for MKL on macos. Further investigation is needed to access the presence or impact of similar issues on other platforms.

In the LAPACK wrapper for ?heevr, enlarge the size of the isuppz array
parameter in order to prevent out-of-bound access by the underlying
library. For both ?heevr and ?syevr, more robust checks and default
values are also put in place.

Reference: scipy#11709
@miladsade96
Copy link
Member

@congma Please use to fix #11709 or fixes #11709 in the PR description in order to auto-close related issue after merging PR.

@congma
Copy link
Contributor Author

congma commented Mar 28, 2020

@congma Please use to fix #11709 or fixes #11709 in the PR description in order to auto-close related issue after merging PR.

Thanks. This is still very much WIP.

@WarrenWeckesser
Copy link
Member

I haven't reviewed the changes in this PR, so this is just a report on the effect. I am on MacOS 10.12.6. With the master branch of scipy (SHA 1150c4c...) and using numpy from anaconda (so with MKL), I get a seg. fault when I run the linalg test suite. With this PR, the linalg test suite completes with all tests passing. 🎉

@rgommers rgommers requested a review from ilayn April 5, 2020 12:14
@rgommers rgommers added the maintenance Items related to regular maintenance tasks label Apr 5, 2020
@rgommers rgommers added this to the 1.5.0 milestone Apr 5, 2020
@rgommers
Copy link
Member

rgommers commented Apr 5, 2020

Fixes multiple crashes on Linux with MKL as well:

FAILED scipy/linalg/tests/test_decomp.py::TestSVDVals::test_empty
FAILED scipy/sparse/linalg/eigen/arpack/tests/test_arpack.py::test_symmetric_modes
FAILED scipy/sparse/linalg/eigen/arpack/tests/test_arpack.py::test_hermitian_modes
FAILED scipy/sparse/linalg/eigen/arpack/tests/test_arpack.py::test_real_nonsymmetric_modes
FAILED scipy/sparse/linalg/eigen/arpack/tests/test_arpack.py::test_svd_linop

@rgommers
Copy link
Member

rgommers commented May 6, 2020

These crashes are starting to get annoying, and I'm not sure if on macOS we're only seeing these ones or another one as well.

This PR is still WIP, but it does improve the situation. @congma, @ilayn is there any reason not to merge this?

@ilayn
Copy link
Member

ilayn commented May 6, 2020

No there isn't. I just forgot. We need to cleanup a bit a few details but essentially quite doable quickly. I'll have a look.

@ilayn ilayn changed the title [WIP] BUG: Prevent crash in LAPACK Hermitian eigendecomposition wrapper. BUG: Prevent crashes due to MKL bug in ?heevr May 6, 2020
@ilayn
Copy link
Member

ilayn commented May 6, 2020

Apart from the recent Cython issue, Travis being Travis again hence merging. Feedback from MKL users are welcome.

@ilayn ilayn merged commit d62ab82 into scipy:master May 6, 2020
@rgommers
Copy link
Member

rgommers commented May 6, 2020

Thanks @congma and @ilayn!

@WarrenWeckesser
Copy link
Member

Thanks @congma and @ilayn, and @rgommers for the nudge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.linalg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eigh() tests fail to pass, crash Python with seemingly ramdom pattern
5 participants