-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
0.3.22 breaks functions downstream in GNU Octave #3976
Comments
If it helps: https://octave.discourse.group/t/make-check-fails-with-openblas-0-3-22/4289/4 The problem might be in dpotrf or dpstrf due to the specific mix of Octave test failures. |
Looks like i broke a division-by-zero test in zgetf2 with a quick fix lately (#3941 ,managed to change OR into AND and nobody/no test noticed) |
That's probably off-topic to this issue. But @MehdiChinoune is probably referring to the following symbols that are no longer being exported (or exported with a different signature) by the OpenBLAS library compared to version 0.3.21:
Is that intentional? Edit: Inspecting the updated library, all of those symbols seem to be exported still. Not sure why the package grokker picked them out. Maybe, just ignore... Edit 2: That was an error in the package grokker in MSYS2's CI. All symbols are still exported correctly. Please, ignore this. |
Would you like a test in Fortran which would have captured that? As scipy downstream points out, for them the regression comes from https://github.com/scipy/scipy/blob/main/scipy/linalg/src/det.f#L94 |
test in C would be best (for integrating into "utest") but can wait til later,can run the scipy reproducer for now. (Anything better than the octave approach of "just build octave from source and you will see" but can't blame them really). Will be interesting if the other fallout will be similar blunders or just consequences of changed algorithms in LAPACK 3.11 |
bug-63384.f.gz
That looks good.
|
Hmm. I wonder if the GETF2 fixes are trading a NumPy issue ( numpy/numpy#22025 ) for an octave one (the DGECON fix for Reference-LAPACK issue 763 is carried in OpenBLAS 0.3.22) |
Why does dgetrf return info=1 (and not 0)? |
Good question. Seems temp was below DBL_MIN so singularity gets reported (original code before the fix used "is unequal ZERO" there, |
This is directly in octave.
With 0.3.22:
I attached t_rcond.m with those two lines (to avoid cut-n-paste issues). |
@angsch what if I stack the old and new conditionals - if temp1 not exactly zero then check if large enough to do the division, swap&scale as needed, else set info ? I know it`s an engineering solution to a mathematical problem, but... |
My understanding of Do we know if the issue with Edit: I take that back: Reference-LAPACK says
So subnormals should be processed and possibly introduce Infinity. So to follow Reference-LAPACK I suppose two checks are needed. One for exactly zero (then set info and exit) and one |
Ok, thanks, I'm following through with that already - will also check if this takes care of the Octave issues once I've got it built |
You do not need to build a new octave, you can use the binary provided by your distro and use LD_PRELOAD to LU also appears broken in octave (though the tests did not notice that):
with 0.3.21 (works as expected):
|
@dasergatskov Looks like something is wrong with the tiebreaker when pivoting is applied... Can you print P as well? |
I assume P as permutation matrix. it is the same is both cases, but lu is still broken.
0.3.21:
|
This is also a repost from octave https://octave.discourse.group/t/make-check-fails-with-openblas-0-3-22/4289/7 E.g. with
And test fail because br is now 1x4 instead of 1x3 |
@martin-frbg |
@angsch yes, that may have been a bit sloppy but I think it is unrelated to the (main) problem. With the checks fixed to raise info on exact zero only, the Octave testsuite appears to pass (but I have not tried the separate testcases posted above yet) |
#3980 fixes the scipy error and also the rcond , lu und spurious residue errors from octave - I still see one scipy testsuite error: |
With the latest patch, using d708951, it works again in Octave:
|
Thanks for confirming. I am currently bisecting to find the cause of the lone remaining scipy test error |
Version 0.3.22 apparently breaks Octave: OpenMathLib/OpenBLAS#3976 This reverts commit 0ef7a0e.
See: OpenMathLib/OpenBLAS#3976 Include patches from upstream: - OpenMathLib/OpenBLAS#3978 - OpenMathLib/OpenBLAS#3980 - OpenMathLib/OpenBLAS#3984
Version 0.3.22 apparently breaks Octave: OpenMathLib/OpenBLAS#3976 This reverts commit 0ef7a0e.
Version 0.3.22 apparently breaks Octave: OpenMathLib/OpenBLAS#3976 This reverts commit 0ef7a0e.
Thanks. Please consider a new release to avoid distros relying on word of mouth to backport patches or avoid the last release. |
Release preparations already in progress, only interrupted by dinner due to domestic misunderstanding... |
0.3.23 released now, Windows binaries may have to wait until Monday noon (CET) unless I convince myself that the MXE (mingw) dlltool is fully equivalent to a native lib.exe (for creating the export file and import library from the generated .def) |
I guess I am late to the party, but I am still getting higher errors in complex chol (in octave)
The original test was for
|
@dasergatskov Mine is a little bit better, doesn't fail the assertion: 0.3.21:
0.3.23:
The options passed to building 0.3.23 didn't make a difference for me; I get the same results with these two sets of parameters. This:
as well as this:
Compiler is gcc 12.2.1. |
Getting |
OpenBLAS 0.3.21 works perfectly with GNU Octave, but 0.3.22 causes several test failures in matrix inverses, eigenvector calculations, and sparse matrix operations. These include returning Inf instead of NaN and giving an answer that is wrong by several orders of magnitude. Affected Octave functions include inv(), eigs(), residue(), and det(), and any user code using those functions.
Thread: https://octave.discourse.group/t/make-check-fails-with-openblas-0-3-22/4289
How to reproduce:
make check
.Workaround: I have reverted my system OpenBLAS to 0.3.21 for now, and Octave works properly again.
EDIT: Build settings for OpenBLAS, both versions:
The text was updated successfully, but these errors were encountered: