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

Update SparseArrays.jl stdlib for SuiteSparse 7 #48977

Merged
merged 18 commits into from
Jul 11, 2023
Merged

Conversation

ViralBShah
Copy link
Member

@ViralBShah ViralBShah commented Mar 11, 2023

This is essentially the same as @fxcoudert's #48855, but disables building SuiteSparse and SparseArrays in the system image, making it easy to develop.

cc @fxcoudert

@ViralBShah ViralBShah marked this pull request as draft March 11, 2023 15:57
@ViralBShah ViralBShah force-pushed the vs/suitesparse7 branch 2 times, most recently from 1d4a684 to b17d6f5 Compare March 24, 2023 14:35
@ViralBShah ViralBShah changed the title [DO NOT MERGE] SuiteSparse_jll 7 update for experimentation Update SparseArrays for SuiteSparse 7 Mar 24, 2023
@ViralBShah ViralBShah marked this pull request as ready for review March 24, 2023 14:37
@ViralBShah
Copy link
Member Author

ViralBShah commented Mar 24, 2023

@gbaraldi I think you had suggested using the compile cache. I get this for my SparseArrays tests locally:

      From worker 7:	┌ Warning: The call to compilecache failed to create a usable precompiled cache file for SparseArrays [2f01184e-e22b-5df5-ae63-d93ebab69eaf]
      From worker 7:	│   exception = ArgumentError: Invalid checksum in cache file /Users/viral/.julia/compiled/v1.10/SparseArrays/P9ieR_FQ49Q.dylib.
      From worker 7:	└ @ Base loading.jl:1817

Any suggestions?

Perhaps this is #48579

@ViralBShah ViralBShah added sparse Sparse arrays stdlib Julia's standard library JLLs labels Mar 24, 2023
@ViralBShah ViralBShah changed the title Update SparseArrays for SuiteSparse 7 Update SparseArrays.jl stdlib for SuiteSparse 7 Mar 24, 2023
@ViralBShah ViralBShah force-pushed the vs/suitesparse7 branch 2 times, most recently from decd02d to b46a1f3 Compare March 25, 2023 00:40
@fxcoudert
Copy link
Contributor

fxcoudert commented Mar 25, 2023

On i686-linux:

Test threw exception
  | Expression: (sparse(I, 2, 2) * (qr(A)).Q)::SparseMatrixCSC == sparse((qr(A)).Q) == sparse(I, 2, 2)
  | SparseArrays.CHOLMOD.CHOLMODException("invalid")

On 32-bit Windows there are more failures

@ViralBShah
Copy link
Member Author

ViralBShah commented Mar 25, 2023

Yes, made some progress in JuliaSparse/SparseArrays.jl#372, but the wrappers need some more work. SuiteSparse changed a lot of internal representations for the new cmake build system. This is all good stuff, but just needs some more time to sort out.

@ViralBShah
Copy link
Member Author

SuiteSparse 7 is not built for 32-bit BLAS on 32-bit platforms in the recent PRs we merged. JuliaPackaging/Yggdrasil#6455 is attempting to fix that.

@ViralBShah
Copy link
Member Author

ViralBShah commented Apr 12, 2023

@Wimmerer I am pulling in your SparseArrays update for 32-bit SS7 PR here. Binaries for 32-bit platforms will get built in CI and can be used for testing, and if we get lucky, the issues are easy to fix by just reading the CI logs.

@jvo203
Copy link

jvo203 commented May 15, 2023

Hi, has this been fixed in the new Julia 1.9.0 yet? On Intel macOS x86_64 this problem is still occurring each time Julia is started, or the Package Manager runs. Julia installed either via "brew install julia" or from a cask "brew install --cask julia", it doesn't matter which way, the problem is still there:

(intelpython-python3.9) chris@zodiac ~ % uname -a
Darwin zodiac.***.***.**.** 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar  6 21:00:17 PST 2023; root:xnu-8796.101.5~3/RELEASE_X86_64 x86_64
(intelpython-python3.9) chris@zodiac ~ % julia
┌ Warning: CHOLMOD version incompatibility
│
│ Julia was compiled with CHOLMOD version 3.0.14. It is
│ currently linked with version 4.0.3.
│ 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 /usr/local/Cellar/julia/1.9.0/share/julia/stdlib/v1.9/SparseArrays/src/solvers/cholmod.jl:198
┌ Error: Error during initialization of module CHOLMOD
│   exception =
│    could not load symbol "SuiteSparse_config":
│    dlsym(0x7ff900931f40, SuiteSparse_config): symbol not found
│    Stacktrace:
│     [1] __init__()
│       @ SparseArrays.CHOLMOD /usr/local/Cellar/julia/1.9.0/share/julia/stdlib/v1.9/SparseArrays/src/solvers/cholmod.jl:235
└ @ SparseArrays.CHOLMOD /usr/local/Cellar/julia/1.9.0/share/julia/stdlib/v1.9/SparseArrays/src/solvers/cholmod.jl:243
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.0 (2023-05-07)
 _/ |\__'_|_|_|\__'_|  |  Built by Homebrew (v1.9.0)
|__/                   |

Julia>

@ViralBShah
Copy link
Member Author

We do not have a path to update to SS 7 yet, which doesn't have the 32 bit QR solver. I suggest using the official Julia binaries which use an older SS 5.

@jvo203
Copy link

jvo203 commented May 15, 2023

OK, I see. But I thought the cask version of Julia installs the official .dmg package from the Julia downloads website. And yet when the cask version of Julia is installed the same error appears. There is no difference between running the cask and non-cask Julia versions. Both suffer from the error.

Anyway, will uninstall the brew versions and try the official .dmg package the next time I work from the office.

@ViralBShah ViralBShah merged commit b26f3b2 into master Jul 11, 2023
@ViralBShah ViralBShah deleted the vs/suitesparse7 branch July 11, 2023 01:01
@ViralBShah ViralBShah added the backport 1.10 Change should be backported to the 1.10 release label Jul 11, 2023
@Keno
Copy link
Member

Keno commented Jul 11, 2023

Note that this is quite breaking, because there are packages that depend directly on the jll, so there needs to be some jll rebuilding of packages first. Ideally we do that before we backport.

Keno added a commit that referenced this pull request Jul 11, 2023
@ViralBShah
Copy link
Member Author

ViralBShah commented Jul 11, 2023

Do we have some examples? Or do we need to run pkgeval?

Is the list pretty much: https://juliahub.com/ui/Packages/SuiteSparse_jll/ME9At/7.2.0+0?page=2

@ViralBShah
Copy link
Member Author

@Keno I doubt Sundials.jl which depends on Sundials 5 will cleanly be able to use SuiteSparse 7 - and I don't think it is a good idea to force it. I am sure that this will hold back most of the other Yggdrasil packages.

Giving it a shot in JuliaPackaging/Yggdrasil#7030, but I am not hopeful.

@rayegun
Copy link
Member

rayegun commented Jul 11, 2023

@ViralBShah SS 6 and 7 shouldn't have broken the 64 bit interface. At least not by very much

@ViralBShah
Copy link
Member Author

@ViralBShah SS 6 and 7 shouldn't have broken the 64 bit interface. At least not by very much

Fingers crossed. But refreshing these old builds is a bit painful: JuliaPackaging/Yggdrasil#7030

vtjnash added a commit to JuliaSparse/SparseArrays.jl that referenced this pull request Jul 12, 2023
This reverts commit 2c7f4d6 (#406),
now that JuliaLang/julia#48977 is finished.
@giordano
Copy link
Contributor

This PR broke CI in the General registry, all jobs on nightly are failing with

ERROR: LoadError: ArgumentError: Package SparseArrays does not have SuiteSparse_jll in its dependencies:
- You may have a partially installed environment. Try `Pkg.instantiate()`
  to ensure all packages in the environment are installed.
- Or, if you have SparseArrays checked out for development and have
  added SuiteSparse_jll as a dependency but haven't updated your primary
  environment's manifest file, try `Pkg.resolve()`.

@ViralBShah
Copy link
Member Author

The relevant issue is here: JuliaLang/Pkg.jl#3498.

@giordano
Copy link
Contributor

Nevermind, that was fixed by updating the manifest, which previously didn't include SuiteSparse_jll as dependency of SparseArrays: JuliaRegistries/General#87439

@ViralBShah
Copy link
Member Author

I never knew that this Manifest was the CI manifest. I checked the manfiest in the Julia source. It all makes sense now.

KristofferC pushed a commit that referenced this pull request Jul 17, 2023
* Use SparseArrays.jl updated to work with SuiteSparse 7, and with Int32 indices
* SuiteSparse_jll Update to v7.2.0 (Using @Wimmerer's SuiteSparse fork for 32-bit QR)

---------

Co-authored-by: Francois-Xavier Coudert <fxcoudert@gmail.com>
Co-authored-by: Will Kimmerer <kimmerer@mit.edu>
(cherry picked from commit b26f3b2)
KristofferC added a commit that referenced this pull request Oct 2, 2023
Backported PRs:
- [x] #48625 <!-- add replace(io, str, patterns...) -->
- [x] #48387 <!-- Improve documentation of sort-related functions -->
- [x] #48363 <!-- Revise sort.md and docstrings in sort.jl -->
- [x] #48977 <!-- Update SparseArrays.jl stdlib for SuiteSparse 7 -->
- [x] #50719 <!-- fix `CyclePadding(::DataType)` -->
- [x] #50694 <!-- inference: permit recursive type traits -->
- [x] #50860 <!-- Add `Base.get_extension` to docs/API -->
- [x] #50594 <!-- Disallow non-index Integer types in isassigned -->
- [x] #50802 <!-- Makes IntrusiveLinkedListSynchronized mutable to avoid
allocation on poptask -->
- [x] #50858 <!-- Add a `threadpool` parameter to `Channel` constructor
-->
- [x] #50874 <!-- Restrict COFF to a single thread when symbol count is
high -->
- [x] #50822 <!-- Add default method for setmodifiers! -->
- [x] #50730 <!-- Fix integer overflow in `isapprox` -->
- [x] #50850 <!-- Remove weird Rational dispatch and add pi functions to
list -->
- [x] #50809 <!-- Limit type-printing in MethodError -->
- [x] #50915 <!-- Add note the `Task` about sticky bit -->
- [x] #50929 <!-- when widening tuple types in tmerge, only widen the
complex parts -->
- [x] #50928 <!-- Bump JuliaSyntax to 0.4.6 -->
- [x] #50959 <!-- Update libssh2 patches -->
- [x] #50823 <!-- Make ranges more robust with unsigned indexes. -->
- [x] #48542 <!-- Add docs on task-specific buffering using
multithreading -->
- [x] #50912 <!-- Separate foreign threads into a :foreign threadpool
-->
- [x] #51010 <!-- Add ORIGIN to SuiteSparse rpath on Linux/FreeBSD -->
- [x] #50753 <!-- cat: remove unused promote_eltype methods that confuse
inference -->
- [x] #51027 <!-- Implement realloc accounting correctly -->
- [x] #51019 <!-- fix a case of potentially use of undefined variable
when handling error in distributed message processing -->
- [x] #51039 <!-- Revert "Optimize findall(f, ::AbstractArray{Bool})
(#42202)" -->
- [x] #51036 <!-- add missing invoke edge for nospecialize targets -->
- [x] #51042 <!-- inference: fix return_type_tfunc modeling of concrete
functions -->
- [x] #51114 <!-- Workaround upstream FreeBSD issue #272992 -->
- [x] #50892 <!-- Add `JL_DLLIMPORT` to `small_typeof` declaration -->
- [x] #51154 <!-- broadcast: use recursion rather than ntuple to map
over a tuple -->
- [x] #51153 <!-- fix debug typo in "add missing invoke edge for
nospecialize targets (#51036)" -->
- [x] #51222 <!-- Check again if the tty is open inside the IO lock -->
- [x] #51236 <!-- Add lock around uv_unref during init -->
- [x] #51243 <!-- GMP: Gracefully handle more overflows. -->
- [x] #51254 <!-- Ryu: make sure adding zeros does not overwrite
trailing dot -->
- [x] #51175 <!-- shorten stale_age for cachefile lock -->
- [x] #51300 <!-- fix method definition error for bad vararg -->
- [x] #51307 <!-- fix force-throw ctrl-C on Windows -->
- [x] #51303 <!-- ensure revising structs is safe -->
- [x] #51393 
- [x] #51403 

Need manual backport:
- [x] #51009 <!-- fix #50562, regression in `in` of tuple of Symbols -->
- [x] #51053 <!-- Bump Statistics stdlib -->
- [x] #51013 <!-- fix #50709, type bound error due to free typevar in
sparam env -->
- [x] #51305 <!-- reduce test time for rounding and floatfuncs -->

Contains multiple commits, manual intervention needed:
- [ ] #50663 <!-- Fix Expr(:loopinfo) codegen -->
- [ ] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are
now first class -->
- [ ] #51092 <!-- inference: fix bad effects for recursion -->
- [x] #51247 <!-- Check if malloc has succeeded before incrementing gc
stats -->
- [x] #51294 <!-- use LibGit2_jll for LibGit2 library -->

Non-merged PRs with backport label:
- [ ] #51132 <!-- Handle `AbstractQ` in concatenation -->
- [x] #51029 <!-- testdefs: make sure that if a test set changes the
active project, they change it back when they're done -->
- [ ] #50919 <!-- Code loading: do the "skipping mtime check for stdlib"
check regardless of the value of `ispath(f)` -->
- [ ] #50824 <!-- Add some aliasing warnings to docstrings for mutating
functions -->
- [x] #50385 <!-- Precompile pidlocks: add to NEWS and docs -->
- [ ] #49805 <!-- Limit TimeType subtraction to AbstractDateTime -->
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JLLs sparse Sparse arrays stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants