ENH: optimize: Rewrite LBFGSB in C #21314
Conversation
scipy/optimize/__lbfgsb.h
Outdated
| #include <math.h> | ||
|
|
||
| void | ||
| setulb(const int,const int,double*,double*,double*,int*,double*,double*, |
There was a problem hiding this comment.
Can you add documentation onto what each parameter represents, and their required sizes? this info can be adapted from the Fortran.
There was a problem hiding this comment.
Yes typically I do that in the last commit to have the last shape. This is just to get the compilation mechanism to put in place.
|
IMO we would probably lose all the printing/timing info. |
|
LAPACK should be similar to gh-19970, modulo ILP64 which is not available yet anyhow. |
I have the same opinion but no idea how this is used in the wild. However, I got rid of text based branching anyways so we can immediately remove the printing if we are OK with it.
Let me try; fingers crossed. |
|
+1 for removing printing and timing all at once. No one should be relying on the timer calls, that kind of thing is just in the code we vendored way back when. |
|
@ev-br I've restructured things a bit and removed the timing and the printing stuff. Now only errors I'm getting is the BLAS and LAPACK functions not being found. Could you please take a look at the lapack importing parts and let me know how to compile? There are still bugs to clean up but I'll start doing that after I can start compiling. The ones that I need are at the top of the header file. |
|
A short partial answer for now only, sorry. In short, you're missing function prototypes. $ git diff
diff --git a/scipy/optimize/__lbfgsb.h b/scipy/optimize/__lbfgsb.h
index 3912c91a99..cd9cdc7f66 100644
--- a/scipy/optimize/__lbfgsb.h
+++ b/scipy/optimize/__lbfgsb.h
@@ -11,10 +11,17 @@ extern "C"
#include <math.h>
#define PYERR(errobj,message) {PyErr_SetString(errobj,message); return NULL;}
-#define DNRM2 BLAS_FUNC(dnrm2)
-#define DDOT BLAS_FUNC(ddot)
-#define DCOPY BLAS_FUNC(dcopy)
-#define DTRSM BLAS_FUNC(dtrsm)
+
+// these are declared in ../_build_utils/npy_cblas_base.h via npy_cblas.h
+# define DNRM2 cblas_dnrm2
+# define DCOPY cblas_dcopy
+# define DDOT cblas_ddot
+# define DTRSM cblas_dtrsm
+
+//#define DDOT BLAS_FUNC(ddot)
+//#define DCOPY BLAS_FUNC(dcopy)
+//#define DNRM2 BLAS_FUNC(dnrm2)
+//#define DTRSM BLAS_FUNC(dtrsm)but these are CBLAS symbols (whatever that means) and you're using the original Fortran symbols. Maybe you can restructure your usage to use CBLAS (and then it should be ILP64-proof?) or stick with `BLAS_FUNC and add prototypes like in https://github.com/scipy/scipy/pull/19970/files#diff-1d06e2703e983014d899d2c42544507f537a5c6eebc84ec484cf125eed03e102R83 You'll need this anyway for One other thing is that in the meson.build you're declaring the dependency on |
|
CBLAS is also fine, they have the same signature but no dice yet. I used the following # define DNRM2 cblas_dnrm2
# define DCOPY cblas_dcopy
# define DDOT cblas_ddot
# define DAXPY cblas_daxpy dependencies: [np_dep, blas, lapack, py3_dep],
.. or
dependencies: [np_dep, blas_dep, lapack_dep, py3_dep],And I get, mentioning the In file included from ../scipy/optimize/__lbfgsb.c:1:
../scipy/optimize/__lbfgsb.c: In function 'mainlb':
../scipy/optimize/__lbfgsb.h:15:16: error: implicit declaration of function 'cblas_dcopy'; did you mean 'scipy_cblas_dcopy'? [-Wimplicit-function-declaration]
15 | # define DCOPY cblas_dcopy
| ^~~~~~~~~~~
../scipy/optimize/__lbfgsb.c:324:17: note: in expansion of macro 'DCOPY'
324 | DCOPY(&n, t, &one_int, x, &one_int);
| ^~~~~
../scipy/optimize/__lbfgsb.h:14:16: error: implicit declaration of function 'cblas_dnrm2' [-Wimplicit-function-declaration]
14 | # define DNRM2 cblas_dnrm2
| ^~~~~~~~~~~
../scipy/optimize/__lbfgsb.c:555:14: note: in expansion of macro 'DNRM2'
555 | rr = pow(DNRM2(&n, r, &one_int), 2.0);
| ^~~~~
../scipy/optimize/__lbfgsb.c:563:9: error: implicit declaration of function 'dscal'; did you mean '_scalb'? [-Wimplicit-function-declaration]
563 | dscal(&n, &stp, d, &one_int);
| ^~~~~
| _scalb
../scipy/optimize/__lbfgsb.c: In function 'bmv':
../scipy/optimize/__lbfgsb.c:757:5: error: implicit declaration of function 'DTRTRS' [-Wimplicit-function-declaration]
757 | DTRTRS("U", "T", "N", &col, &one_int, wt, &m, &p[col], &col, info);
| ^~~~~~
../scipy/optimize/__lbfgsb.c: In function 'cauchy':
../scipy/optimize/__lbfgsb.h:16:15: error: implicit declaration of function 'cblas_ddot'; did you mean 'scipy_cblas_ddot'? [-Wimplicit-function-declaration]
16 | # define DDOT cblas_ddot
| ^~~~~~~~~~
../scipy/optimize/__lbfgsb.c:931:19: note: in expansion of macro 'DDOT'
931 | f2 = f2 - DDOT(&col2, v, &one_int, p, &one_int);
| ^~~~
../scipy/optimize/__lbfgsb.c:1011:13: error: implicit declaration of function 'daxpy' [-Wimplicit-function-declaration]
1011 | daxpy(&col2, &dt, p, &one_int, c, &one_int);
| ^~~~~
../scipy/optimize/__lbfgsb.c: In function 'formk':
../scipy/optimize/__lbfgsb.c:1367:5: error: implicit declaration of function 'DPOTRF' [-Wimplicit-function-declaration]
1367 | DPOTRF('U', &col, wn, &m2, info);
| ^~~~~~
../scipy/optimize/__lbfgsb.c: In function 'lnsrlb':
../scipy/optimize/__lbfgsb.c:1590:14: error: implicit declaration of function 'dnrm2' [-Wimplicit-function-declaration]
1590 | *dnorm = dnrm2(&n, d, &one_int);
| ^~~~~
[28/29] Module scanner.
ninja: build stopped: subcommand failed.
Build failed!No rush though. I'll keep trying different stuff. I am almost certain that what we are doing is something fundamentally cumbersome and there must be a better way for this. I just don't know what yet. |
|
Okkkay, I see something different: So I guess you're building with |
|
I think we have no other option than what Or start using LAPACKE properly everywhere. |
|
Nah, I'm pretty sure there's a simpler solution. Will look simple in retrospect, that is :-). I'll take a look. |
|
The following diff makes it build for me locally (bog-standard ubuntu linux, conda env + conda compilers, openblas from conda-forge: diff --git a/scipy/optimize/__lbfgsb.c b/scipy/optimize/__lbfgsb.c
index 17af196bd6..1ea0cc507d 100644
--- a/scipy/optimize/__lbfgsb.c
+++ b/scipy/optimize/__lbfgsb.c
@@ -1179,10 +1179,10 @@ formk(const int n, const int nsub, const int* ind, const int nenter, const int i
{
js = m + jy;
temp_int = m - jy + 1;
- DCOPY(&temp_int, wn1[(jy + 1) + 2*m*(jy + 1)], &one_int, wn1[jy + 2*m*jy], &one_int);
- DCOPY(&temp_int, wn1[(js + 1) + 2*m*(js + 1)], &one_int, wn1[js + 2*m*js], &one_int);
+ DCOPY(&temp_int, &wn1[(jy + 1) + 2*m*(jy + 1)], &one_int, &wn1[jy + 2*m*jy], &one_int);
+ DCOPY(&temp_int, &wn1[(js + 1) + 2*m*(js + 1)], &one_int, &wn1[js + 2*m*js], &one_int);
temp_int = m - 1;
- DCOPY(&temp_int, wn1[(m + 1) + 2*m*(jy + 1)], &one_int, wn1[m + 2*m*jy], &one_int);
+ DCOPY(&temp_int, &wn1[(m + 1) + 2*m*(jy + 1)], &one_int, &wn1[m + 2*m*jy], &one_int);
}
// 10
}
@@ -1379,7 +1379,7 @@ formk(const int n, const int nsub, const int* ind, const int nenter, const int i
{
for (js = is; js < col2; js++)
{
- wn[is + 2*m*js] = wn[is + 2*m*js] + DDOT(col, wn[2*m*is], &one_int, wn[2*m*js], &one_int);
+ wn[is + 2*m*js] = wn[is + 2*m*js] + DDOT(&col, &wn[2*m*is], &one_int, &wn[2*m*js], &one_int);
}
// 74
}
@@ -1727,9 +1727,9 @@ matupd(const int n, const int m, double* ws, double *wy, double* sy, double* ss,
{
for (j = 1; j < *col; j++)
{
- DCOPY(&j, ss[1 + m*j], &one_int, ss[m*(j-1)], &one_int);
+ DCOPY(&j, &ss[1 + m*j], &one_int, &ss[m*(j-1)], &one_int);
temp_int = *col - j;
- DCOPY(&temp_int, sy[j + m*j], &one_int, sy[(j-1) + m*(j-1)], &one_int);
+ DCOPY(&temp_int, &sy[j + m*j], &one_int, &sy[(j-1) + m*(j-1)], &one_int);
}
// 50
}
@@ -1841,7 +1841,7 @@ void subsm(const int n, const int m, const int nsub, const int* ind,
temp = 1.0 / theta;
// n, da, dx, incx
- dscal(&nsub, &temp, d, &one_int);
+ DSCAL(&nsub, &temp, d, &one_int);
// -----------------------------------------------------
// Let us try the projection, d is the Newton direction.
diff --git a/scipy/optimize/__lbfgsb.h b/scipy/optimize/__lbfgsb.h
index 3912c91a99..cffd0c008f 100644
--- a/scipy/optimize/__lbfgsb.h
+++ b/scipy/optimize/__lbfgsb.h
@@ -14,8 +14,19 @@ extern "C"
#define DNRM2 BLAS_FUNC(dnrm2)
#define DDOT BLAS_FUNC(ddot)
#define DCOPY BLAS_FUNC(dcopy)
+#define DSCAL BLAS_FUNC(dscal)
#define DTRSM BLAS_FUNC(dtrsm)
-#define DPOTRF F_FUNC(dpotrf)
+#define DPOTRF BLAS_FUNC(dpotrf)
+#define DTRTRS BLAS_FUNC(dtrtrs)
+
+void DSCAL(int *n, double *da, double *dx, int *incx);
+double DNRM2(int *n, double *x, int *incx);
+double DDOT(int *n, double *dx, int *incx, double *dy, int *incy);
+void DCOPY(int *n, double *dx, int *incx, double *dy, int *incy);
+void DTRSM(char *side, char *uplo, char *transa, char *diag, int *m, int *n, double *alpha, double *a, int *lda, double *b, int *ldb);
+void DPORTF(char *uplo, int *n, double *a, int *lda, int *info);
+void DTRTRS(char *uplo, char *trans, char *diag, int *n, int *nrhs, double *a, int *lda, double *b, int *ldb, int *info);
+
static PyObject* lbfgsb_error;
This is likely not the end of the story: some of the warnings are definitely relevant (passing char constants instead of int pointers, const-correctness etc etc). Also ran out of time today to check that it actually imports. But there is at least some progress :-). |
|
OK so we need the prototypes regardless then. The rest is just unchecked buggy code that I will go through anyways. Thanks a lot |
|
One other thing which looks a tad strange here is that the ModuleDef python glue is in the header file. I'm not claiming it's best practice, just noting that what seemed to work (== compile, link and import) elsewhere is splitting the python module into a separate |
|
That might be true. I just did it in minpack quadpack and others and it worked independently. I think this C stuff is pretty much on consensus and no rules. So I really don't mind keeping them in the same place. |
Sure. As long as it builds, links and imports, whatever works :-). I'm just noticing one difference, maybe irrelevant. |
|
I cleaned up a bit so that only LAPACK import complaints are present and C code is not complaining. With the latest commits I get [3/114] Compiling C object scipy/optimize/_lbfgsb.cp312-win_amd64.pyd.p/__lbfgsb.c.obj
FAILED: scipy/optimize/_lbfgsb.cp312-win_amd64.pyd.p/__lbfgsb.c.obj
"cc" "-Iscipy\optimize\_lbfgsb.cp312-win_amd64.pyd.p" "-Iscipy\optimize" "-I..\scipy\optimize" "-I..\..\..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\numpy\_core\include" "-IC:/Users/ilhan/AppData/Local/Programs/Python/Python312/Lib/site-packages/scipy_openblas32/include" "-IC:\Users\ilhan\AppData\Local\Programs\Python\Python312\Include" "-fvisibility=hidden" "-fdiagnostics-color=always" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-std=c17" "-O2" "-g" "-Wno-unused-but-set-variable" "-Wno-unused-function" "-Wno-conversion" "-Wno-misleading-indentation" "-mlong-double-64" "-D__USE_MINGW_ANSI_STDIO=1" "-DBLAS_SYMBOL_PREFIX=scipy_" "-DNPY_NO_DEPRECATED_API=NPY_1_9_API_VERSION" -MD -MQ scipy/optimize/_lbfgsb.cp312-win_amd64.pyd.p/__lbfgsb.c.obj -MF "scipy\optimize\_lbfgsb.cp312-win_amd64.pyd.p\__lbfgsb.c.obj.d" -o scipy/optimize/_lbfgsb.cp312-win_amd64.pyd.p/__lbfgsb.c.obj "-c" ../scipy/optimize/__lbfgsb.c
In file included from ../scipy/optimize/__lbfgsb.h:9,
from ../scipy/optimize/__lbfgsb.c:1:
../scipy/optimize/__lbfgsb.c: In function 'formk':
<command-line>: error: implicit declaration of function 'scipy_dpotrf_'; did you mean 'scipy_dtrtrs_'? [-Wimplicit-function-declaration]
../scipy/optimize/../_build_utils/src/npy_cblas.h:50:54: note: in definition of macro 'BLAS_FUNC_CONCAT'
50 | #define BLAS_FUNC_CONCAT(name,prefix,suffix,suffix2) prefix ## name ## suffix ## suffix2
| ^~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:25: note: in expansion of macro 'BLAS_FUNC_EXPAND'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:47: note: in expansion of macro 'BLAS_SYMBOL_PREFIX'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~~~
../scipy/optimize/__lbfgsb.h:19:16: note: in expansion of macro 'BLAS_FUNC'
19 | #define DPOTRF BLAS_FUNC(dpotrf)
| ^~~~~~~~~
../scipy/optimize/__lbfgsb.c:1369:5: note: in expansion of macro 'DPOTRF'
1369 | DPOTRF(uplo, &col, wn, &m2, info);
| ^~~~~~
../scipy/optimize/__lbfgsb.h: At top level:
<command-line>: warning: 'scipy_daxpy_' used but never defined
../scipy/optimize/../_build_utils/src/npy_cblas.h:50:54: note: in definition of macro 'BLAS_FUNC_CONCAT'
50 | #define BLAS_FUNC_CONCAT(name,prefix,suffix,suffix2) prefix ## name ## suffix ## suffix2
| ^~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:25: note: in expansion of macro 'BLAS_FUNC_EXPAND'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:47: note: in expansion of macro 'BLAS_SYMBOL_PREFIX'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~~~
../scipy/optimize/__lbfgsb.h:14:15: note: in expansion of macro 'BLAS_FUNC'
14 | #define DAXPY BLAS_FUNC(daxpy)
| ^~~~~~~~~
../scipy/optimize/__lbfgsb.h:23:13: note: in expansion of macro 'DAXPY'
23 | static void DAXPY(const int* n, const double* da, const double* dx,
| ^~~~~
<command-line>: warning: 'scipy_dcopy_' used but never defined
../scipy/optimize/../_build_utils/src/npy_cblas.h:50:54: note: in definition of macro 'BLAS_FUNC_CONCAT'
50 | #define BLAS_FUNC_CONCAT(name,prefix,suffix,suffix2) prefix ## name ## suffix ## suffix2
| ^~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:25: note: in expansion of macro 'BLAS_FUNC_EXPAND'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:47: note: in expansion of macro 'BLAS_SYMBOL_PREFIX'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~~~
../scipy/optimize/__lbfgsb.h:15:15: note: in expansion of macro 'BLAS_FUNC'
15 | #define DCOPY BLAS_FUNC(dcopy)
| ^~~~~~~~~
../scipy/optimize/__lbfgsb.h:25:13: note: in expansion of macro 'DCOPY'
25 | static void DCOPY(const int* n, const double* dx, const int *incx,
| ^~~~~
<command-line>: warning: 'scipy_ddot_' used but never defined
../scipy/optimize/../_build_utils/src/npy_cblas.h:50:54: note: in definition of macro 'BLAS_FUNC_CONCAT'
50 | #define BLAS_FUNC_CONCAT(name,prefix,suffix,suffix2) prefix ## name ## suffix ## suffix2
| ^~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:25: note: in expansion of macro 'BLAS_FUNC_EXPAND'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:47: note: in expansion of macro 'BLAS_SYMBOL_PREFIX'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~~~
../scipy/optimize/__lbfgsb.h:16:14: note: in expansion of macro 'BLAS_FUNC'
16 | #define DDOT BLAS_FUNC(ddot)
| ^~~~~~~~~
../scipy/optimize/__lbfgsb.h:27:15: note: in expansion of macro 'DDOT'
27 | static double DDOT(const int *n, const double *dx, const int *incx,
| ^~~~
<command-line>: warning: 'scipy_dnrm2_' used but never defined
../scipy/optimize/../_build_utils/src/npy_cblas.h:50:54: note: in definition of macro 'BLAS_FUNC_CONCAT'
50 | #define BLAS_FUNC_CONCAT(name,prefix,suffix,suffix2) prefix ## name ## suffix ## suffix2
| ^~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:25: note: in expansion of macro 'BLAS_FUNC_EXPAND'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:47: note: in expansion of macro 'BLAS_SYMBOL_PREFIX'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~~~
../scipy/optimize/__lbfgsb.h:17:15: note: in expansion of macro 'BLAS_FUNC'
17 | #define DNRM2 BLAS_FUNC(dnrm2)
| ^~~~~~~~~
../scipy/optimize/__lbfgsb.h:29:15: note: in expansion of macro 'DNRM2'
29 | static double DNRM2(const int *n, const double *x, const int *incx);
| ^~~~~
<command-line>: warning: 'scipy_dscal_' used but never defined
../scipy/optimize/../_build_utils/src/npy_cblas.h:50:54: note: in definition of macro 'BLAS_FUNC_CONCAT'
50 | #define BLAS_FUNC_CONCAT(name,prefix,suffix,suffix2) prefix ## name ## suffix ## suffix2
| ^~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:25: note: in expansion of macro 'BLAS_FUNC_EXPAND'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:47: note: in expansion of macro 'BLAS_SYMBOL_PREFIX'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~~~
../scipy/optimize/__lbfgsb.h:18:15: note: in expansion of macro 'BLAS_FUNC'
18 | #define DSCAL BLAS_FUNC(dscal)
| ^~~~~~~~~
../scipy/optimize/__lbfgsb.h:30:13: note: in expansion of macro 'DSCAL'
30 | static void DSCAL(const int *n, const double *da, double *dx,
| ^~~~~
<command-line>: warning: 'scipy_dtrtrs_' used but never defined
../scipy/optimize/../_build_utils/src/npy_cblas.h:50:54: note: in definition of macro 'BLAS_FUNC_CONCAT'
50 | #define BLAS_FUNC_CONCAT(name,prefix,suffix,suffix2) prefix ## name ## suffix ## suffix2
| ^~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:25: note: in expansion of macro 'BLAS_FUNC_EXPAND'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~
../scipy/optimize/../_build_utils/src/npy_cblas.h:62:47: note: in expansion of macro 'BLAS_SYMBOL_PREFIX'
62 | #define BLAS_FUNC(name) BLAS_FUNC_EXPAND(name,BLAS_SYMBOL_PREFIX,BLAS_SYMBOL_SUFFIX,BLAS_FORTRAN_SUFFIX)
| ^~~~~~~~~~~~~~~~~~
../scipy/optimize/__lbfgsb.h:20:16: note: in expansion of macro 'BLAS_FUNC'
20 | #define DTRTRS BLAS_FUNC(dtrtrs)
| ^~~~~~~~~
../scipy/optimize/__lbfgsb.h:34:13: note: in expansion of macro 'DTRTRS'
34 | static void DTRTRS(const char* uplo, const char *trans, const char *diag,
[10/114] Compiling C++ object scipy/optimize/_highs/libhighs.a.p/.._..__lib_highs_extern_filereaderlp_reader.cpp.obj
ninja: build stopped: subcommand failed.
Build failed!
C:\Users\ilhan\Documents\GitHub\scipy [rewrite_lbfgsb ≡ +0 ~2 -0 !]> |
|
By the way is it |
|
You seem to be building with scipy-openblas32 wheels. The symbols there have the EDIT: The magic incantation is not really magic, it's just a way to generate a collection of data in a format the build system understands (for one, note the |
|
The whole point of name mangling is to account for various LAPACK providers: a system openblas, scipy-openblas32, BLIS, MKL etc. |
|
Ok can you please run nm on one of the scipy lapack depending binaries? I'm curious what you and I will see in terms of symbols names. Because you are absolutely right What demangles these names I don't know. Somewhere in the codebase, there must be a header that is normalizing all these names. |
It shouldn't. The name-mangling magic for the end-user happens when picking up the scipy-openblas pkgconfig from |
|
Could you give a complete example where using |
|
In this PR, in the header file we have bunch of BLAS LAPACK prototypes and all CI jobs somehow seem to pickup and compile this. Hence our confusion, especially on windows jobs. Also I am building with scipy-openblas32 on my own machine and it goes through. |
|
What do you think folks, should we kick this one in? I am a bit anxious about the LAPACK parts, so the sooner we know the better we can work on the fix. |
|
Retried rebuilding with locally with The LAPACK linking story is mysterious:
So I've no idea what is |
|
I think |
DOC:optimize: Added license and docstrings of LBFGSB C code BUG:optimize: Fix segfaults and 1-off errors in LBFGSB code BUG:optimize: Address segfaults in subsm C func
DOC:optimize: Fix LBFGSB docstring MAINT:optimize: Use int array for task and status messages in LBFGSB
DOC:MAINT:Also removed Fortran mentions MAINT:optimize: Fix LBFGSB termination messages MAINT: optimize: Modify Python file for LBFGSB rewrite
TST:optimize: Modify tests for LBFGSB rewrite
4221fe2 to
a72b6a2
Compare
|
OK squashed and separated files for later cherry-picking. This is good to go. |
Yes, but it does not seem to be used (the scipy_openblas PREFIX is |
|
I think I don't have enough tools in my belt to understand this mechanism. It seems to me that something is not demangling but something is actually adding the prefix but how does it understand which one to add and search openblas eludes me. |
|
Hi @ilayn , thanks for the excellent work! Any plans to remove gotos in the code? |
|
Hi @adam-sim-dev I considered removing them completely but the remaining gotos are not that bad (about 20 of them left from a large number), typically jumping to the cleanly separated code block. The original authors used it for flow control, though still not fun to have them, removing them leads to unnecessary code duplication if we don't put something else in place. My hope is that now that it is a bit more readable and streamlined, someone sits down and writes this in a cleaner procedural way meaning introducing new smaller functions instead of gotos. My focus was to steer away from F77 isms so I did not dare to introduce new bugs because of that. But if you have any appetite, it would be much appreciated. |
Part of #18566
This one has a very annoying todo list, so I'll leave it here for folks who know about these issues.
__quadpack.h/__minpack.h. (Both are fine, just as a todo item)daxpy,ddot,dcopy,dscal,dnrm2,dtrtrs,dpotrf)Distinguish ILP64time.hof C)?