Repository navigation
Load all libraries through SuiteSparse_jll and update the generator to Clang.jl 0.19 - #55
Merged
Merged
Conversation
KLU.jl declared SuiteSparse_jll as a dependency but never loaded it, and found libklu through the bare name :libklu. The other library names in wrappers.jl weren't defined at all, so calling those wrappers threw UndefVarError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
===========================================
+ Coverage 73.17% 97.54% +24.37%
===========================================
Files 2 1 -1
Lines 630 367 -263
===========================================
- Hits 461 358 -103
+ Misses 169 9 -160 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The headers moved to include/suitesparse. Map colamd.h to libcolamd, as the amd.h pattern also matched it, ignore two macros that would call C at load time, and check that every library name is a binding of SuiteSparse_jll. No struct field changed. Four structs became immutable, and they're only read with unsafe_load. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Loads all SuiteSparse libraries through
SuiteSparse_jll, the same way SparseArrays.jl does, and updates the wrapper generator to Clang.jl 0.19.Loading libraries from the JLL
KLU.jl declared
SuiteSparse_jllas a dependency, but nothing insrc/loaded it. It setconst libklu = :libkluand left the library to be found by name.src/wrappers.jlalso refers tolibsuitesparseconfig,libbtfandlibamd, which weren't defined anywhere in the module, so calling those wrappers threwUndefVarError. For example,KLU.SuiteSparse_config_malloc_func_get()did.With
using SuiteSparse_jll, all of these names come from the JLL, andsrc/,test/anddocs/make.jlno longer contain a literal library name.gen/generator.tomlstill has to spell out the binding names for Clang.jl, so the generator now asserts that each one is a binding ofSuiteSparse_jll.On Linux with Julia 1.13,
mainalready loads the same file as Julia's own copy,lib/julia/libklu.so.2, so I don't expect a change there. What this adds is that KLU.jl no longer depends on that lookup finding the right file, which could differ on Windows, where a bare name goes through DLL search rules. I haven't shown that a wrong DLL has ever been loaded, so this isn't a known fix for #38.Generator and wrappers
include/suitesparse, so the generator failed before this.colamd_*wrappers: these now uselibcolamd. They pointed atlibamdbecausecolamd.hwasn't mapped.SUITESPARSE_TIMEandSUITESPARSE_CONFIG_TIMER, which would otherwise call C at load time or refer to an undefined name.src/wrappers.jlis regenerated from SuiteSparse 7.10.1, with these effects:klu_symbolic,klu_l_symbolic,klu_numericandklu_l_numericbecame immutable. KLU.jl only reads them throughunsafe_load.klu_version,amd_version,btf_versionandcolamd_version. These may not exist in older SuiteSparse versions, so they could fail if called on those. Nothing in KLU.jl calls them.Generating for
x86_64-w64-mingw32andx86_64-linux-gnugives byte-identical output, so one set of wrappers is correct for both. That rules out a difference in the generated bindings as a cause of the Windows reports.Codecov
Adds
codecov.yml, which excludesgenandsrc/wrappers.jl. The wrappers are generated, and most of them are never called, which held the project figure at about 52%.src/KLU.jlalone was at 95.8% when last measured, so expect the reported figure to jump. That's the change in what's counted, not new tests.Testing
Two additions are folded into the existing "KLU Wrappers" testset: all five libraries are identical to the JLL's, and a
libsuitesparseconfigwrapper is callable. The latter threwUndefVarErroronmain.The full suite passes locally on Linux with Julia 1.10, 1.11, 1.12 and 1.13 at
-t 2. Aqua and the docs build also pass. Windows has only been run through CI.🤖 Generated with Claude Code