Skip to content
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

[SuiteSparse@7] New version for v7.0.1 #6296

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

fxcoudert
Copy link
Contributor

Previous version is almost 2 years old.

@giordano
Copy link
Member

There is also #6006. CC: @Wimmerer

@fxcoudert fxcoudert changed the title Update to SuiteSparse 7.0.1 @fxcoudert [SuiteSparse] Update to v7.0.1 Feb 26, 2023
@fxcoudert fxcoudert changed the title @fxcoudert [SuiteSparse] Update to v7.0.1 [SuiteSparse] Update to v7.0.1 Feb 26, 2023
@fxcoudert
Copy link
Contributor Author

😢 Needs cmake >= 3.22 (released November 2021)

@fxcoudert
Copy link
Contributor Author

Now only mingw is failing to build, with:

[19:33:00] make[2]: *** No rule to make target '/workspace/destdir/bin/libblastrampoline.dll', needed by 'libcholmod.dll'.  Stop.
[19:33:00] make[2]: *** Waiting for unfinished jobs....

@fxcoudert
Copy link
Contributor Author

The build is ok, I've confirmed that the artefacts contain the expected libraries. Anyone has an idea how this can be tested further?

@giordano
Copy link
Member

@Wimmerer

But as I already mentioned in #6006 (review), I'd prefer to reorganise SuiteSparse to allow keeping multiple versions in parallel (we often have to do that for Julia dependencies). I personally never got around doing that.

@fxcoudert fxcoudert changed the title [SuiteSparse] Update to v7.0.1 [SuiteSparse@7] New version for v7.0.1 Feb 27, 2023
@fxcoudert
Copy link
Contributor Author

I'd prefer to reorganise SuiteSparse to allow keeping multiple versions in parallel

Wasn't sure how you wanted to move the old version. But I've updated the PR so that the new version is in a subdirectory SuiteSparse@7.

@fxcoudert
Copy link
Contributor Author

fxcoudert commented Feb 27, 2023

For x86_64-linux-gnu, here is the diff between the content of the old package (v5) and the new package (v7):

``` --- /dev/fd/11 2023-02-27 14:56:17 +++ /dev/fd/12 2023-02-27 14:56:17 @@ -9,96 +9,96 @@ include/cholmod.h include/camd.h include/ccolamd.h include/cholmod.h -include/cholmod_blas.h -include/cholmod_camd.h -include/cholmod_check.h -include/cholmod_cholesky.h -include/cholmod_complexity.h -include/cholmod_config.h -include/cholmod_core.h -include/cholmod_function.h -include/cholmod_gpu.h -include/cholmod_gpu_kernels.h -include/cholmod_io64.h -include/cholmod_matrixops.h -include/cholmod_modify.h -include/cholmod_partition.h -include/cholmod_supernodal.h -include/cholmod_template.h include/colamd.h include/klu.h +include/klu_cholmod.h include/ldl.h -include/spqr.hpp include/umfpack.h -include/umfpack_col_to_triplet.h -include/umfpack_defaults.h -include/umfpack_free_numeric.h -include/umfpack_free_symbolic.h -include/umfpack_get_determinant.h -include/umfpack_get_lunz.h -include/umfpack_get_numeric.h -include/umfpack_get_symbolic.h -include/umfpack_global.h -include/umfpack_load_numeric.h -include/umfpack_load_symbolic.h -include/umfpack_numeric.h -include/umfpack_qsymbolic.h -include/umfpack_report_control.h -include/umfpack_report_info.h -include/umfpack_report_matrix.h -include/umfpack_report_numeric.h -include/umfpack_report_perm.h -include/umfpack_report_status.h -include/umfpack_report_symbolic.h -include/umfpack_report_triplet.h -include/umfpack_report_vector.h -include/umfpack_save_numeric.h -include/umfpack_save_symbolic.h -include/umfpack_scale.h -include/umfpack_solve.h -include/umfpack_symbolic.h -include/umfpack_tictoc.h -include/umfpack_timer.h -include/umfpack_transpose.h -include/umfpack_triplet_to_col.h -include/umfpack_wsolve.h lib +lib/cmake +lib/cmake/SuiteSparse +lib/cmake/SuiteSparse/FindAMD.cmake +lib/cmake/SuiteSparse/FindBTF.cmake +lib/cmake/SuiteSparse/FindCAMD.cmake +lib/cmake/SuiteSparse/FindCCOLAMD.cmake +lib/cmake/SuiteSparse/FindCHOLMOD.cmake +lib/cmake/SuiteSparse/FindCHOLMOD_CUDA.cmake +lib/cmake/SuiteSparse/FindCOLAMD.cmake +lib/cmake/SuiteSparse/FindKLU.cmake +lib/cmake/SuiteSparse/FindKLU_CHOLMOD.cmake +lib/cmake/SuiteSparse/FindLDL.cmake +lib/cmake/SuiteSparse/FindRBio.cmake +lib/cmake/SuiteSparse/FindSPQR.cmake +lib/cmake/SuiteSparse/FindSPQR_CUDA.cmake +lib/cmake/SuiteSparse/FindSuiteSparse_config.cmake +lib/cmake/SuiteSparse/FindUMFPACK.cmake +lib/cmake/SuiteSparse/SuiteSparseBLAS.cmake +lib/cmake/SuiteSparse/SuiteSparseBLAS32.cmake +lib/cmake/SuiteSparse/SuiteSparseBLAS64.cmake +lib/cmake/SuiteSparse/SuiteSparseLAPACK.cmake +lib/cmake/SuiteSparse/SuiteSparsePolicy.cmake +lib/cmake/SuiteSparse/SuiteSparseReport.cmake +lib/cmake/SuiteSparse/SuiteSparse_ssize_t.cmake +lib/libamd.a lib/libamd.so -lib/libamd.so.2 -lib/libamd.so.2.4.6 +lib/libamd.so.3 +lib/libamd.so.3.0.3 +lib/libbtf.a lib/libbtf.so -lib/libbtf.so.1 -lib/libbtf.so.1.2.6 +lib/libbtf.so.2 +lib/libbtf.so.2.0.3 +lib/libcamd.a lib/libcamd.so -lib/libcamd.so.2 -lib/libcamd.so.2.4.6 +lib/libcamd.so.3 +lib/libcamd.so.3.0.3 +lib/libccolamd.a lib/libccolamd.so -lib/libccolamd.so.2 -lib/libccolamd.so.2.9.6 +lib/libccolamd.so.3 +lib/libccolamd.so.3.0.3 +lib/libcholmod.a lib/libcholmod.so -lib/libcholmod.so.3 -lib/libcholmod.so.3.0.14 +lib/libcholmod.so.4 +lib/libcholmod.so.4.0.3 +lib/libcholmod_cuda.a +lib/libcholmod_cuda.so +lib/libcholmod_cuda.so.4 +lib/libcholmod_cuda.so.4.0.3 +lib/libcolamd.a lib/libcolamd.so -lib/libcolamd.so.2 -lib/libcolamd.so.2.9.6 +lib/libcolamd.so.3 +lib/libcolamd.so.3.0.3 +lib/libklu.a lib/libklu.so -lib/libklu.so.1 -lib/libklu.so.1.3.8 +lib/libklu.so.2 +lib/libklu.so.2.0.3 +lib/libklu_cholmod.a +lib/libklu_cholmod.so +lib/libklu_cholmod.so.2 +lib/libklu_cholmod.so.2.0.3 +lib/libldl.a lib/libldl.so -lib/libldl.so.2 -lib/libldl.so.2.2.6 +lib/libldl.so.3 +lib/libldl.so.3.0.3 +lib/librbio.a lib/librbio.so -lib/librbio.so.2 -lib/librbio.so.2.2.6 +lib/librbio.so.3 +lib/librbio.so.3.0.3 +lib/libspqr.a lib/libspqr.so -lib/libspqr.so.2 -lib/libspqr.so.2.0.9 +lib/libspqr.so.3 +lib/libspqr.so.3.0.3 +lib/libspqr_cuda.a +lib/libspqr_cuda.so +lib/libspqr_cuda.so.3 +lib/libspqr_cuda.so.3.0.3 +lib/libsuitesparseconfig.a lib/libsuitesparseconfig.so -lib/libsuitesparseconfig.so.5 -lib/libsuitesparseconfig.so.5.10.1 +lib/libsuitesparseconfig.so.7 +lib/libsuitesparseconfig.so.7.0.1 +lib/libumfpack.a lib/libumfpack.so -lib/libumfpack.so.5 -lib/libumfpack.so.5.7.9 +lib/libumfpack.so.6 +lib/libumfpack.so.6.1.0 share share/licenses share/licenses/SuiteSparse ```

In summary:

  • library numbers are changed (as expected)
  • static libraries are now included
  • cmake config files are now provided (due to the upstream switch from make to cmake)
  • cholmod_*.h and umfpack_*.h headers are gone
  • spqr.hpp header is gone
  • klu_cholmod.h is a new file
  • new CUDA libraries: libcholmod_cuda and libspqr_cuda

Those changes seem to match what we ship in Homebrew, so it seems things are in order.

@fxcoudert
Copy link
Contributor Author

If there are no objections or review comments, could we merge this?

@giordano
Copy link
Member

giordano commented Mar 2, 2023

I was hoping for @Wimmerer to have a look, since he maintains the SuiteSparse stack in Julia 🙂

@rayegun
Copy link
Contributor

rayegun commented Mar 2, 2023

I'll take a look at it today. I momentarily stopped looking at it once it became clear we weren't going to remove SparseArrays from the stdlibs until 1.10 so it was less urgent.

I also thought about moving KLU to a separate jll. Thoughts @giordano?

@ViralBShah
Copy link
Member

@Wimmerer I am thinking we keep things as they are (no KLU split) while SparseArrays is still an stdlib. And then do bigger overhauls later (like perhaps each suitesparse component being its own library).

@fxcoudert
Copy link
Contributor Author

Using the same cmake flags I have been able to build Julia against SuiteSparse 7.0.1 from source in JuliaLang/julia#48855

@ViralBShah
Copy link
Member

ViralBShah commented Mar 8, 2023

static libraries are now included

@giordano we generally don't ship static libraries right?

@ViralBShah
Copy link
Member

ViralBShah commented Mar 8, 2023

new CUDA libraries: libcholmod_cuda and libspqr_cuda

We probably should also not include the _cuda libraries since they need all the cuda machinery to work, which we handle in the SuiteSparse_GPU package for now.

@fxcoudert
Copy link
Contributor Author

Force-pushed with CUDA disabled (passing -DENABLE_CUDA=NO)

@fxcoudert
Copy link
Contributor Author

SparseArrays bump was merged in JuliaLang/julia#48941
This is ready to go so the companion Julia PR can be properly tested: JuliaLang/julia#48855

@ViralBShah
Copy link
Member

@fxcoudert I am just checking on whether it is ok to have .a files in the generated JLL package. I think usually we only want the dynamic libraries. This package also ships in Julia, and hence want to be careful about not including things that have traditionally not been in there.

Can you remove those? I used to be familiar with the old SuiteSparse build, but not the new cmake one.

@fxcoudert
Copy link
Contributor Author

@ViralBShah static libraries removed

@ViralBShah
Copy link
Member

ViralBShah commented Mar 9, 2023

We can just delete the cuda libraries and the .a files at the end of the script for now to get them merged.

@ViralBShah
Copy link
Member

Do we need to delete the static libraries as well? Based on the SuiteSparse issue you filed above, it seems like -DNSTATIC doesn't work correctly.

@ViralBShah
Copy link
Member

It seems now there is also some problem with building libspqr.

@fxcoudert
Copy link
Contributor Author

Do we need to delete the static libraries as well?

Only the CUDA libraries are built as static, so removing all cuda files takes care of it.

It seems now there is also some problem with building libspqr

But other libraries get linked against the CUDA libraries, even when I remove them at the end:

ERROR: could not load library "/cache/build/yggy-amdci7-4/julialang/yggdrasil/S/SuiteSparse/SuiteSparse@7/SuiteSparse/build/x86_64-linux-gnu/wBEdC5sf/x86_64-linux-gnu-libgfortran3-cxx03/destdir/lib/libklu.so.2.0.3"
  | libcholmod_cuda.so.4: cannot open shared object file: No such file or directory

@fxcoudert
Copy link
Contributor Author

Wait, it seems actually building and shipping empty CUDA-named libraries when CUDA is not enabled is not a bug, but a feature: DrTimothyAldenDavis/SuiteSparse@788eee6

🤯

@ViralBShah
Copy link
Member

Ok, so maybe this is then good to go?

@rayegun
Copy link
Contributor

rayegun commented Mar 9, 2023

LGTM, so long as the empty CUDA libs aren't dlopen'd that's fine!

@ViralBShah ViralBShah merged commit e4797ef into JuliaPackaging:master Mar 9, 2023
@fxcoudert fxcoudert deleted the suitesparse branch March 9, 2023 15:11
@fxcoudert
Copy link
Contributor Author

so long as the empty CUDA libs aren't dlopen'd that's fine

They are not, there is an explicit list of libraries dlopen'ed in the stdlib code (and version numbers, actually) and the cuda ones aren't in it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants