[SuiteSparse_jll] Update to v7.4.0

[giordano]

Usual memo to self for the update:

• change version number in stdlib/SuiteSparse_jll/Project.toml
• refresh checksums in deps/checksums/suitesparse with make -f contrib/refresh_checksums.mk -j libsuitesparse. Note: this will also delete the files deps/checksums/SuiteSparse-*.tar.gz/*, but those refer to the stdlib which is the julia wrapper around the binary dependency, those files have to be restored manually, I don't know how to make the makefile behave nicely. You will also have to remove the lines SuiteSparse-*.tar.gz/md5/* added to the file deps/checksums/suitesparse, they are wrong for the same reason
• update version numbers in deps/libsuitesparse.version.

Judging by the changes in JuliaPackaging/Yggdrasil#7596 we don't need to touch the manual build system.

I have no idea whether SuiteSparse retains API/ABI compatibility within the same major version, I surely hope so because I won't have the strength to try and fix large compatibility errors.

[giordano]

There is only one relevant test failure, of course on 32-bit Windows:

Error in testset SparseArrays/cholmod:
Test Failed at C:\buildkite-agent\builds\win2k22-amdci6-4\julialang\julia-master\julia-b3081f8082\share\julia\stdlib\v1.11\SparseArrays\test\cholmod.jl:947
  Expression: (cholesky(A, perm = 1:n)).p == 1:n
   Evaluated: [12, 25, 5, 1, 2, 3, 4, 6, 7, 8  …  91, 92, 93, 94, 95, 96, 97, 98, 99, 100] == 1:100

I have no idea of what's the problem, so I'd appreciate any help here (also in the form of removing support for 32-bit Windows 🙃)

Everywhere else it's all good. But we also need to merge JuliaSparse/SparseArrays.jl#478 at the same time to clear the warnings about the version of cholmod:

      From worker 10:	┌ Warning: CHOLMOD version incompatibility
      From worker 10:	│
      From worker 10:	│ Julia was compiled with CHOLMOD version 4.0.4. It is
      From worker 10:	│ currently linked with version 5.0.0.
      From worker 10:	│ This might cause Julia to terminate when working with
      From worker 10:	│ sparse matrix factorizations, e.g. solving systems of
      From worker 10:	│ equations with \.
      From worker 10:	│
      From worker 10:	│ It is recommended that you use Julia with the same major
      From worker 10:	│ version of CHOLMOD as the one used during the build, or
      From worker 10:	│ download the generic binaries from www.julialang.org,
      From worker 10:	│ which ship with the correct versions of all dependencies.
      From worker 10:	└ @ SparseArrays.CHOLMOD /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-b3081f8082/share/julia/stdlib/v1.11/SparseArrays/src/solvers/cholmod.jl:206

[gbaraldi]

Is it adding some garbage to the start of the array?

[giordano]

No, the number of elements is correct, but 12, 25, 5 for some reasons are at the front: https://buildkite.com/julialang/julia-master/builds/31334#018c82ce-35e2-43c1-a143-eebf240e7b15/12749-13075

F.p = [12, 25, 5, 1, 2, 3, 4, 6, 7, 8, 9, 10, 11, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100]

But this is wrong already at https://github.com/JuliaSparse/SparseArrays.jl/blob/8308232f12fe6d28c689e11b2e19b48a484136f6/src/solvers/cholmod.jl#L1320, and as far as I understand (but I may be wrong) the symbolic function is all written in Julia, no call into suitesparse yet: https://github.com/JuliaSparse/SparseArrays.jl/blob/8308232f12fe6d28c689e11b2e19b48a484136f6/src/solvers/cholmod.jl#L1258-L1278

[ViralBShah]

cc @rayegun. The challenge here is of course how does one even test on 32-bit windows.

[giordano]

I'm tempted by marking the test as broken on 32-bit Windows so that we can move on 🥲

[ViralBShah]

I think it may actually be ok to do so.

[rayegun]

😮‍💨, time to find the W32 VM again

[giordano]

In case someone has a way to debug this on 32-bit Windows, I reduced the failure to

using LinearAlgebra, SparseArrays, SparseArrays.CHOLMOD

n = 100
A = sprand(n,n,5/n) |> t -> t't + I
SA = CHOLMOD.Sparse(A)
F = CHOLMOD.@cholmod_param postorder = false begin
    CHOLMOD.@cholmod_param nmethods = 1 begin
        CHOLMOD.analyze_p(SA, Int[p-1 for p in 1:n])
    end
end
F.p == 1:n # this should be true, but it's false

It can probably be reduced further, but I know absolutely nothing of the SparseArrays code, I think I reached the maximum I could spend on this.

If no one has ideas, I might just mark the test as broken only on that platforms so that we can upgrade the package.

[ViralBShah]

@DrTimothyAldenDavis Would you have any insight into what might be causing the win32 failure above?

[DrTimothyAldenDavis]

No idea.

I do try to maintain as much backward compatibility as possible between say a 4.0 and a 5.0 package. I would have to check the details to see what API/API differs. Is that the question?

I do test SuiteSparse in its CI with WIN32 on GitHub and it's passing.

Can you try 7.4.0.betaX where X is 10 I think, but is changing daily? I will release 7.4.0 stable soon.

[ViralBShah]

That would suggest that there may be an issue in our wrappers.

[DrTimothyAldenDavis]

Yes. It could be the cholmod 4 vs 5, above.

Or it could be a Christmas bug. The first two numbers are 12,25 after all 😀

[giordano]

There are tons of test failures with SuiteSparse v7.4.0, on all platforms, not just 32-bit Windows....

[giordano]

What's most worrying is that we're getting loads of

Error: no BLAS/LAPACK library loaded!

errors, which comes from libblastrampoline, but I have no clue of how this is related to upgrading to SuiteSparse 7.4.0

[mmuetzel]

The build system of SuiteSparse has been completely overhauled for 7.4.0. Many of the configuration flags have changed their names. You'd probably need to adjust your build rules for that change here:
https://github.com/JuliaPackaging/Yggdrasil/blob/master/S/SuiteSparse/SuiteSparse%407/build_tarballs.jl

Maybe, you could also consider building with the root CMakeLists.txt now.

[giordano]

Great news: after updating the wrappers using the SuiteSparse header files (PR JuliaSparse/SparseArrays.jl#480 by @rayegun), all SparseArrays tests are passing in this PR, for all platforms, including 32-bit ones (and including the permutation test). The failures reported above were likely due to some changes in SuiteSparse between v7.2.1 and v7.3.0/v7.4.0 which broke ABI compatibility on 32-bit systems. Also @imciner2 identified the change to the type of other_2 in cholmod_common as a potential problem on 32-bit platforms, so that may be the culprit, and what forced us to update the wrappers.

Tomorrow I'll update JuliaSparse/SparseArrays.jl#478 to include the changes in JuliaSparse/SparseArrays.jl#480 and keep everything in one place, but this is now looking good for us, thanks everybody for the help!

[DrTimothyAldenDavis]

It has become impossible to maintain the SuiteSparse bindings - I have to re-learn the whole build system every few months.

Sorry for the headaches. We're getting feedback from spack, PETsc, linux distros, msys, octave, ... and of course julia, and the feedback doesn't come all at once. We're trying to make the build system work for everybody, but it means that it's an iterative process.

[DrTimothyAldenDavis]

The failures reported above were likely due to some changes in SuiteSparse between v7.2.1 and v7.3.0/v7.4.0 which broke ABI compatibility on 32-bit systems.

Whenever I change a package, I do keep to the semantic versioning process: CHOLMOD has had a few breaking changes in the past, for example. SuiteSparse itself doesn't track each version change of the packages inside it. I used to bump the major SO of SuiteSparse itself when any of its component packages had a major SO bump, but that caused problems with linux distro maintainers so I don't do that anymore.

So here's how CHOLMOD has bumped in is major SO numbers, recently:

SuiteSparse     CHOLMOD
5.13.0          3.0.14      many cholmod*h include files
6.0.0           4.0.0       single cholmod.h include file, SuiteSparse_long deprecated
7.0.0           4.0.3
7.2.1           4.2.1
7.3.0           5.0.0       added single precision support (xworksize -> xworkbytes)
7.4.0           5.1.0
7.5.0           5.1.1

and you're note that the bump from CHOLMOD 4.x.x to 5.x.x did not cause SuiteSparse itself to bump from 7.2.1 to 8.0.0.

I may have broken the ABI upward compatibility by changing the size_t other_2 to double other_2, however. That was inadvertent. That happened when going from SuiteSparse 7.2.1 to 7.3.0 and CHOLMOD bumped from 4.2.1 to 5.0.0 so at least that is "safe" in the sense that a breaking change to the ABI did get reflected in the major SO number.

[DrTimothyAldenDavis]

For example, the change from ALLOW_64BIT_BLAS to SUITESPARSE_USE_64BIT_BLAS was from linux distro feedback, or spack (I can't recall exactly), so that all user-facing cmake variables are prefixed with SUITESPARSE_ or Package_ (like UMFPACK_, etc).

[imciner2]

Sorry for the headaches.

No worries @DrTimothyAldenDavis. Thanks for the work you are doing to coalesce all these various requests and update the packages, the actual build system changes recently have actually led to a simplification of the build script we use in Yggdrasil, so that is appreciated.

One thing that might be useful going forward for doing the updates to these wrappers is to have a more detailed changelog for the APIs of the various components. For instance, when I started looking at the CHOLMOD 4->5 version bump to see if that was the cause of the errors, there wasn't really a good list of what changes were made to the API, so I ended up having to read through the header file and sometimes do a comparison between the old header and the new header. Some of the changes (like changing the naming/meaning of xdtype from xtype in cholmod_allocate_sparse were more subtle, and weren't obvious at first read through. Having a changelog for the API would make the process of updating easier and flag those types of subtle changes.

[ViralBShah]

It has become impossible to maintain the SuiteSparse bindings - I have to re-learn the whole build system every few months.

Sorry for the headaches. We're getting feedback from spack, PETsc, linux distros, msys, octave, ... and of course julia, and the feedback doesn't come all at once. We're trying to make the build system work for everybody, but it means that it's an iterative process.

@DrTimothyAldenDavis Apologies - My complaint was not meant for you, but for the way we integrate SuiteSparse in Julia itself and update it in a roundabout way.

[DrTimothyAldenDavis]

One thing that might be useful going forward for doing the updates to these wrappers is to have a more detailed changelog for the APIs of the various components.

I can work on that.

[giordano]

I updated this PR to use latest changes in JuliaSparse/SparseArrays.jl#478. That PR now includes better way to update SuiteSparse_jll to regenerate our bindings, and also improved documentation to explain how to upgrade the version of the JLL package, this should address Viral's concerns about maintenance of our bindings.

If tests are green, as I hope since there should be no functional changes compared to this fully green run (excluding to may-fail jobs), we should be good to go.

[rayegun]

I am not sure off the top of my head how to regenerate the hashes, if no one does it before ~10AM tomorrow I will givve it a shot.

[ViralBShah]

The checksum refreshing script is in contrib/refresh_checksums.mk. I can probably get to it tomorrow.

[giordano]

I am not sure off the top of my head how to regenerate the hashes

Update information in stdlib/SparseArrays.version and then run the commands

git rm -rf deps/checksums/SparseArrays-*
make -C stdlib -j checksum-SparseArrays

if no one does it before ~10AM tomorrow I will givve it a shot.

I just did it. This should be good to go now!

[ViralBShah]

Does anyone know if the BLAS patch we are applying upstreamed to SuiteSparse?

[DrTimothyAldenDavis]

Does anyone know if the BLAS patch we are applying upstreamed to SuiteSparse?

I could take a look, and consider it for SuiteSparse 7.5.0. Where is it in SuiteSparse_jll?

[giordano]

It's a backport of DrTimothyAldenDavis/SuiteSparse#671, it's already in SuiteSparse.

[KristofferC]

IIRC, @KristofferC said we can never move SparseArrays.jl out of stdlib because it would be breaking.

If I did I was just reiterating what triage (in its great wisdom) decided #48161 (comment). So according to that we are indeed never allowed to remove stdlibs from Julia. We could make the stdlibs upgradable (so you can upgrade them with the package manager) and ignore the bundled ones. This situation is of course worse for users than if the stdlibs were just properly moved out but hey, it is not "breaking".