ENH: Add locking to umath_linalg if no lapack is detected at build time#26750
ENH: Add locking to umath_linalg if no lapack is detected at build time#26750rgommers merged 4 commits intonumpy:mainfrom
Conversation
|
This needs a release note I think since lapack_lite has been thread-unsafe since forever. Also all the work I’m doing toward improving and documenting thread safety probably needs a release note. Just commenting to remind myself to open a followup issue about a release note for free-threaded support and a release note specific to this, since this thread safety issue is a problem even with the GIL. |
seberg
left a comment
There was a problem hiding this comment.
Nice straight forward! I happy to not test every function here, that might be nice but rather repetitive anyway.
One small comment mostly, I think we should use have_lapack (even if it may not matter in practice)?
Was considering if there are other blas/lapacks that are not thread-safe. But I think renaming the lock is a bridge we can cross when that happens.
rgommers
left a comment
There was a problem hiding this comment.
LGTM modulo two quite minor comments. Thanks @ngoldbaum
ebdeb4d to
79bd9cc
Compare
rgommers
left a comment
There was a problem hiding this comment.
Thanks Nathan. One more little bug in one of the last commits, plus two tiny suggestions for the release note
rgommers
left a comment
There was a problem hiding this comment.
LGTM now, in it goes. Thanks Nathan! And thanks for the review Sebastian.
Closes #22509
This is a re-do of #26687 following the suggestion in #26687 (comment).
This adds locking macros around all the calls into the low-level LAPACK code in umath_linalg.cpp. The macros are either no-ops or calls to lock or unlock a global mutex declared statically in the
_umath_linalgmodule and initialized during that modules' initialization. The mutex is only used if we do not detect a LAPACK during the build, indicating that lapack_lite is being used.The test I added randomly fails or segfaults on
mainif you force numpy to build using lapack_lite.Happy to add more extensive thread safety testing if there's a desire for it.
I think I got all the low-level calls but please let me know if you think I missed something.