Update the wrappers for SuiteSparse 7 and remove `cholmod_blas.h` bindings #363

Gnimuc · 2023-03-12T02:35:22Z

~~It looks like our old API in use is stable in SuiteSparse 7.~~

codecov-commenter · 2023-03-12T03:04:24Z

Codecov Report

Merging #363 (4fba4b4) into main (e17f6da) will increase coverage by 0.01%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #363      +/-   ##
==========================================
+ Coverage   93.85%   93.87%   +0.01%     
==========================================
  Files          12       12              
  Lines        7473     7472       -1     
==========================================
  Hits         7014     7014              
+ Misses        459      458       -1

Impacted Files	Coverage Δ
src/solvers/LibSuiteSparse.jl	`100.00% <ø> (ø)`

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ViralBShah · 2023-03-12T16:28:34Z

There was a small API change that was implemented in c09f0ac

ViralBShah · 2023-03-12T16:33:44Z

Even though the Manifest has the new SuiteSparse 7 in it, the generated wrappers are not picking up the new SuiteSparse 7:

SparseArrays.jl/src/solvers/lib/aarch64-apple-darwin20.jl

Line 2171 in 6a10bda

const SUITESPARSE_MAIN_VERSION = 5

ViralBShah · 2023-03-12T17:11:24Z

I have updated generator.jl to work with this setup, but it generates wrappers that have things like int64_t and various other undefined things in them.

I am not checking in the generated files, since they do not work - but they are essentially what I had in #362.

Gnimuc · 2023-03-14T11:37:58Z

I still get

ERROR: LoadError: Cannot locate artifact 'SuiteSparse' in '/Users/gnimuc/Code/juliasrc/usr/share/julia/stdlib/v1.10/SuiteSparse_jll/src/../Artifacts.toml'
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] ensure_artifact_installed(name::String, artifacts_toml::String; platform::Base.BinaryPlatforms.Platform, pkg_uuid::Nothing, verbose::Bool, quiet_download::Bool, io::Base.TTY)
   @ Pkg.Artifacts ~/Code/juliasrc/usr/share/julia/stdlib/v1.10/Pkg/src/Artifacts.jl:354
 [3] ensure_artifact_installed(name::String, artifacts_toml::String)
   @ Pkg.Artifacts ~/Code/juliasrc/usr/share/julia/stdlib/v1.10/Pkg/src/Artifacts.jl:346
 [4] top-level scope
   @ ~/.julia/dev/SparseArrays.jl/gen/generator.jl:12
in expression starting at /Users/gnimuc/.julia/dev/SparseArrays.jl/gen/generator.jl:12

We should doc how to develop and update a stdlib.

Gnimuc · 2023-03-14T11:41:10Z

I am not checking in the generated files, since they do not work - but they are essentially what I had in #362.

It should be fine. I just checked the src of SuiteSparse:

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/1d379153cd5f1ea5bc67d27dfaf5f018eb3038ad/CHOLMOD/Include/cholmod.h#L1017

these changes are intended.

ViralBShah · 2023-03-14T12:41:44Z

Do you get that error using my PR as well? I really hope we can make SparseArrays.jl a standalone package and not an stdlib.

cc @KristofferC

ViralBShah · 2023-03-14T12:52:09Z

@Gnimuc The problem with those generated files in #362 is that int64_t and some of those other things are not defined anywhere in the generated julia code. So they come up as undefined variables.

Gnimuc · 2023-03-14T13:49:26Z

@Gnimuc The problem with those generated files in #362 is that int64_t and some of those other things are not defined anywhere in the generated julia code. So they come up as undefined variables.

Sorry. I finally understood the problem in #362 (comment) . I thought there were something wrong when Clang.jl mistakenly translated those Int64 types.

Those "constants" are used in macros, so we need to either ignore the macro identifier or manually add those definitions like what we have done here: https://github.com/JuliaSparse/SparseArrays.jl/blob/main/src/solvers/LibSuiteSparse.jl#L11

Gnimuc · 2023-03-14T13:53:45Z

To ignore a macro identifier, we can add it to this list: https://github.com/JuliaSparse/SparseArrays.jl/blob/main/gen/generator.toml#L9

For example, __STDC_VERSION__ and restrict are being used in the definition of SUITESPARSE_STDC_VERSION and SUITESPARSE_RESTRICT respectively.

const SUITESPARSE_STDC_VERSION = __STDC_VERSION__

const SUITESPARSE_RESTRICT = restrict

SUITESPARSE_STDC_VERSION and SUITESPARSE_RESTRICT should be added to the ignore list.

Gnimuc · 2023-03-14T13:57:02Z

Do you get that error using my PR as well?

Yes. This is the full version info:

julia> versioninfo()
Julia Version 1.10.0-DEV.810
Commit 841e9df121 (2023-03-11 15:48 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin22.2.0)
  CPU: 8 × Apple M1
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, westmere)
  Threads: 4 on 8 virtual cores
Environment:
  JULIA_EDITOR = code
  JULIA_NUM_THREADS = 4

I remembered that in the old days, to develop a stdlib, one simple hack is to manually edit the UUID of that package. I believe this hack can not work with the latest master?

ViralBShah · 2023-03-14T13:58:59Z

I remembered that in the old days, to develop a stdlib, one simple hack is to manually edit the UUID of that package. I believe this hack can not work with the latest master?

Yes, that's why I pinged @KristofferC here. I'm hoping that JuliaLang/julia#48979 will merge soon, which will make this a lot easier, and the next step is to remove SparseArrays from the stdlibs altogether.

fxcoudert · 2023-03-22T13:09:08Z

Is there any news?

ViralBShah · 2023-03-22T13:55:56Z

The wrappers need updating as discussed here. To ease the development of that, we now have merged JuliaLang/julia#48979, which removes SparseArrays from the Julia system image.

fxcoudert · 2023-03-22T17:27:21Z

Warning:

┌ Warning: CHOLMOD version incompatibility
│ 
│ Julia was compiled with CHOLMOD version 4.0.3. It is
│ currently linked with version 3.0.14.
│ This might cause Julia to terminate when working with
│ sparse matrix factorizations, e.g. solving systems of
│ equations with \.
│ 
│ It is recommended that you use Julia with the same major
│ version of CHOLMOD as the one used during the build, or
│ download the generic binaries from www.julialang.org,
│ which ship with the correct versions of all dependencies.
└ @ SparseArrays.CHOLMOD D:\a\SparseArrays.jl\SparseArrays.jl\src\solvers\cholmod.jl:199

and failure on 32-bit Windows.

ViralBShah · 2023-03-22T17:28:42Z

I wouldn't worry about the failures here, which are using the nightly built on julia master. If it works locally, I think we should merge it, and then bump on Julia master, with some tender love and care.

Gnimuc · 2023-03-22T22:31:41Z

I should point out that I didn't test it locally.

ViralBShah · 2023-03-23T04:14:59Z

I did run the SparseArrays tests on a build of Julia that pulls in SuiteSparse 7 and SparseArrays.jl from this PR. Everything works fine, except for a couple of QR factorization failures for ordering=6.

julia> qr(sprand(10,10,0.1), ordering=6)
ERROR: Sparse QR factorization failed
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] _qr!(ordering::Int64, tol::Float64, econ::Int64, getCTX::Int64, A::Sparse{Float64}, Bsparse::Ptr{Nothing}, Bdense::Ptr{Nothing}, Zsparse::Ptr{Nothing}, Zdense::Ptr{Nothing}, R::Base.RefValue{Ptr{SparseArrays.LibSuiteSparse.cholmod_sparse_struct}}, E::Base.RefValue{Ptr{Int64}}, H::Base.RefValue{Ptr{SparseArrays.LibSuiteSparse.cholmod_sparse_struct}}, HPinv::Base.RefValue{Ptr{Int64}}, HTau::Base.RefValue{Ptr{SparseArrays.LibSuiteSparse.cholmod_dense_struct}})
   @ SparseArrays.SPQR ~/julia/usr/share/julia/stdlib/v1.10/SparseArrays/src/solvers/spqr.jl:73
 [3] qr(A::SparseMatrixCSC{Float64, Int64}; tol::Float64, ordering::Int64)
   @ SparseArrays.SPQR ~/julia/usr/share/julia/stdlib/v1.10/SparseArrays/src/solvers/spqr.jl:198
 [4] top-level scope
   @ REPL[38]:1

cc @Wimmerer

ViralBShah · 2023-03-24T03:10:04Z

The missing ordering above is added by enabling PARTITION (METIS) support in SuiteSparse. JuliaPackaging/Yggdrasil#6446.

#369

…jl into gn/ss7wrappers

ViralBShah · 2023-03-24T22:27:53Z

@Gnimuc I think this doesn't treat the 32-bit OSes right.

https://buildkite.com/julialang/julia-master/builds/22268#018714ca-ee8c-4b4e-a2d6-9a1fa3c6c7b2

ViralBShah · 2023-03-24T22:32:17Z

The way to test is to use julia branch with the SuiteSparse 7 PR and update SparseArrays.version in stdlib to be the commit SHA of the commit in your branch/PR on this repo.

Since SparseArrays is no longer in the system image, you don't have to necessarily build Julia everytime you make a change.

I think SuiteSparse_long needs to be Int32 on 32-bit systems:

SparseArrays.jl/src/solvers/lib/i686-linux-gnu.jl

Line 2408 in ee419ae

const SuiteSparse_long = int64_t

ViralBShah · 2023-03-25T00:37:51Z

Trying in #372

jvo203 · 2023-05-15T02:46:47Z

As per the comment below, this problem does not seem to have been fixed in the new Julia 1.9.0 yet. Is there any reasonable time estimate as to when this problem might be fixed?

JuliaLang/julia#48977 (comment)

rayegun · 2023-06-21T21:01:55Z

As per the comment below, this problem does not seem to have been fixed in the new Julia 1.9.0 yet. Is there any reasonable time estimate as to when this problem might be fixed?

JuliaLang/julia#48977 (comment)

We're trying to fix for 1.10.

Gnimuc added 2 commits March 12, 2023 11:27

Update Manifest.toml

66029e1

Remove bindings for cholmod_blas.h

6a10bda

Update generator.jl

f483fd6

fix-up

8e267e3

Merge branch 'main' into gn/ss7wrappers

4fba4b4

ViralBShah mentioned this pull request Mar 24, 2023

Cholmod version is recorded somewhere in source? #359

Closed

ViralBShah added 2 commits March 24, 2023 09:40

Disable failing ordering test for qr.

65e43e8

#369

Merge branch 'gn/ss7wrappers' of github.com:JuliaSparse/SparseArrays.…

6766fd0

…jl into gn/ss7wrappers

ViralBShah merged commit 691287e into main Mar 24, 2023

ViralBShah deleted the gn/ss7wrappers branch March 24, 2023 14:05

vtjnash mentioned this pull request Apr 20, 2023

improve cat design / performance JuliaLang/julia#49322

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the wrappers for SuiteSparse 7 and remove `cholmod_blas.h` bindings #363

Update the wrappers for SuiteSparse 7 and remove `cholmod_blas.h` bindings #363

Gnimuc commented Mar 12, 2023 •

edited

Loading

codecov-commenter commented Mar 12, 2023 •

edited

Loading

ViralBShah commented Mar 12, 2023

ViralBShah commented Mar 12, 2023 •

edited

Loading

ViralBShah commented Mar 12, 2023 •

edited

Loading

Gnimuc commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

ViralBShah commented Mar 14, 2023

ViralBShah commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

ViralBShah commented Mar 14, 2023

fxcoudert commented Mar 22, 2023

ViralBShah commented Mar 22, 2023

fxcoudert commented Mar 22, 2023

ViralBShah commented Mar 22, 2023

Gnimuc commented Mar 22, 2023

ViralBShah commented Mar 23, 2023 •

edited

Loading

ViralBShah commented Mar 24, 2023 •

edited

Loading

ViralBShah commented Mar 24, 2023

ViralBShah commented Mar 24, 2023 •

edited

Loading

ViralBShah commented Mar 25, 2023

jvo203 commented May 15, 2023

rayegun commented Jun 21, 2023

Update the wrappers for SuiteSparse 7 and remove cholmod_blas.h bindings #363

Update the wrappers for SuiteSparse 7 and remove cholmod_blas.h bindings #363

Conversation

Gnimuc commented Mar 12, 2023 • edited Loading

codecov-commenter commented Mar 12, 2023 • edited Loading

Codecov Report

ViralBShah commented Mar 12, 2023

ViralBShah commented Mar 12, 2023 • edited Loading

ViralBShah commented Mar 12, 2023 • edited Loading

Gnimuc commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

ViralBShah commented Mar 14, 2023

ViralBShah commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

Gnimuc commented Mar 14, 2023

ViralBShah commented Mar 14, 2023

fxcoudert commented Mar 22, 2023

ViralBShah commented Mar 22, 2023

fxcoudert commented Mar 22, 2023

ViralBShah commented Mar 22, 2023

Gnimuc commented Mar 22, 2023

ViralBShah commented Mar 23, 2023 • edited Loading

ViralBShah commented Mar 24, 2023 • edited Loading

ViralBShah commented Mar 24, 2023

ViralBShah commented Mar 24, 2023 • edited Loading

ViralBShah commented Mar 25, 2023

jvo203 commented May 15, 2023

rayegun commented Jun 21, 2023

Update the wrappers for SuiteSparse 7 and remove `cholmod_blas.h` bindings #363

Update the wrappers for SuiteSparse 7 and remove `cholmod_blas.h` bindings #363

Gnimuc commented Mar 12, 2023 •

edited

Loading

codecov-commenter commented Mar 12, 2023 •

edited

Loading

ViralBShah commented Mar 12, 2023 •

edited

Loading

ViralBShah commented Mar 12, 2023 •

edited

Loading

ViralBShah commented Mar 23, 2023 •

edited

Loading

ViralBShah commented Mar 24, 2023 •

edited

Loading

ViralBShah commented Mar 24, 2023 •

edited

Loading