cgo/clapack: update codegen for LAPACKE 3.6 #75
Conversation
|
This can be left until we want to upgrade to 3.6 since it now seems that we can link against 3.6 with ours (on my machine) and an upgrade will break installations that do not have 3.6. |
|
BTW Do not trust the github diff. It is clearly wrong in places. |
|
Does this remove the functions that are no longer in Lapack? It looked like it did more from the real diff. |
|
It actually doesn't remove anything. It changes a parameter name and adds some new functions and changes some signatures. |
|
Ah, that makes sense. Could it also remove the extra functions (so I can use the new OpenBLAS)? |
|
Can you try building with this before I do that. I don't actually see any thing that says those were deleted intentionally. |
|
As is currently set up, I get the same linking error (about undefined symbols). Do I need to change anything? |
|
You shouldn't need to. @vladimir-ch can you replicate this failure? |
|
It took me a while to figure out that you are using the develop branch. Yes, I get the same linking errors as @btracey |
|
I think the cause is |
|
No, that doesn't seem to be it. |
|
|
|
|
|
OK, that makes sense. I can now replicate the failure - artifacts for those functions were left behind and linked in, so hosing the repo and re-checking out gives the failure. So this PR should also add a list of deprecated functions to exclude. |
Because change for change's sake is a good thing.
kortschak
force-pushed
the
lapacke-3.6
branch
from
596f66a to
6343a7f
Compare
|
PTAL |
|
Could you explain this a bit? It seems to delete a lot of functions "LAPACKE_zuncsd2by1" for example. Are these being determined from a master list somewhere? Is the copyright year change intentional? |
|
Code compiles correctly for me, so LGTM |
|
Just for my better understanding of the big picture (a bit too late): this PR requires OpenBLAS to be compiled from the develop branch. It is not difficult at all but still it is a slightly harsh requirement that should be at least mentioned somewhere. Also, what is the motivation or justification of this? Otherwise we are rather conservative. |
|
For whatever reason (I don't understand this myself), OpenBLAS's default branch is develop. The motivation is that it fixes the build - when we pull for travis we get develop and so our builds were breaking since the deprecated functions were not being built. |
Interesting. I'm wondering how many packages actually use the develop branch. I didn't do any rigorous comparison but building our clapack against OpenBLAS master seemed to produce the same errors as against my Fedora 22 system OpenBLAS. |
|
Other groups are at least struggling with the issue on what to do with these deprecations and the breaks they cause. scipy/scipy#5266 The relevant factor in our case is that it seems we are the first group to actually test the RowMajor version of Lapacke. There are a bunch of memory errors and other size problems. See, for example, the two bugs I discovered in #74 alone. Using older versions of OpenBLAS give you incorrect results. Current versions do too, but that's only a stronger argument to stay with develop for the time being. |
That's not nice but I guess the alternatives are limited. Out of curiosity, does the Accelerate framework on OSX have similar issues? Or has someone tested MKL? |
|
The problems are with Lapack functions. I don't know of alternate Lapack implementations except for some specific functions, so all Lapack versions would have these problems (since they all call the same C API) |
|
You mean that (y)our implementation of Lapack in native Go is not something common? All others end up calling the same Fortran code? |
|
As far as I know, yes. OpenBLAS has a few functions, ATLAS has a few It must be the case that we're the first to really test the row major
|
|
Wow. Thanks. |
@btracey Please take a look.