-
Notifications
You must be signed in to change notification settings - Fork 7
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
Upgrade to 9.4.2 -- Add IndexBinaryOp #136
base: main
Are you sure you want to change the base?
Conversation
Tests are passing (🎉), but wheel builds are failing. @alugowski do you have a moment to take a look? |
Tests use the library built by conda, wheels build it ourselves. Looks like there are differences. Linux error:
Seems related to this point in the release notes for upstream 9.4.2:
Does that mean the library needs to be built differently? Or cffi needs to be updated? We don't have the JIT, so if someone uses those JIT-only kernels then something appropriate needs to happen. |
On macos:
Sounds like the directories where GraphBLAS puts things changed yet again. Someone needs to track down the new include directory on macos and make sure the build system looks there. IIRC the include search path is around here:
|
Windows failure appears to also be the undefined symbols to the no-longer-included kernels. |
I didn't move the location of GraphBLAS.h. Also, the library still has GB__Adot2B__plus_firstj_int32:
So the error you're seeing is not related to any symbols I removed. I did remove the built-in int8 and uint8 types from most of the FactoryKernels, to reduce the compile time and library size. These are all that are left for uint8:
|
GraphBLAS will still compute the correct result for uint8 and int8 types. It will either use the JIT if that's available, or if not, it will use the generic kernels. Those are slower but they still work just fine. |
I don't know if this is related: When I saw you were having these errors, I thought I would try some more testing of GraphBLAS 9.4.2 by updating the main SuiteSparse repo with this version (I hadn't done that yet). The SuiteSparse repo has an extensive cross-platform CI. I encountered some errors because 10 semirings got inadvertently defined twice in GraphBLAS.h: It's a quick fix, but I'm not sure if this problem is related to yours. These extra definitions only cause problems if GraphBLAS.h is #include'd in a C++ program, since one set of the duplicate definitions was inside an extern "C" {...} block and the other wasn't. I'll release a GraphBLAS 9.4.3 shortly -- probably by tomorrow (Dec 20). |
@DrTimothyAldenDavis it looks like it might be related. The Windows build has a better error message. Linux fails at library import while Windows fails earlier at the GraphBLAS link step and lists all the missing symbols, not just one. See full list in the Actions output: https://github.com/GraphBLAS/python-suitesparse-graphblas/actions/runs/12404089168/job/34628708451?pr=136#step:9:3393 They're repeats of these, just called from different
|
No description provided.