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

Make SparseArrays a weak dependency #134

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

IanButterworth
Copy link
Contributor

@IanButterworth IanButterworth commented Dec 7, 2022

Uses the recently merged JuliaLang/julia#47695 to set up SparseArrays as a weak dependency of Statistics, via a package extension.

All code was copied across to the package extension untouched, with the addition of some imports.

This is backwards compatible. On older julia versions that don't support package extensions, SparseArrays will be loaded as per normal.

What this would look like on julia master, if SparseArrays is removed from the sysimage

julia> @time using Statistics
  0.011658 seconds (13.76 k allocations: 1.030 MiB)

julia> @time using SparseArrays
  4.618848 seconds (7.05 M allocations: 980.989 MiB, 1.46% compilation time)

cc. @KristofferC

@codecov-commenter
Copy link

codecov-commenter commented Dec 7, 2022

Codecov Report

Base: 96.94% // Head: 97.26% // Increases project coverage by +0.31% 🎉

Coverage data is based on head (8ab238a) compared to base (e9ac70b).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   96.94%   97.26%   +0.31%     
==========================================
  Files           1        1              
  Lines         426      365      -61     
==========================================
- Hits          413      355      -58     
+ Misses         13       10       -3     
Impacted Files Coverage Δ
src/Statistics.jl 97.26% <ø> (+0.31%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@IanButterworth IanButterworth force-pushed the ib/glue branch 3 times, most recently from 35c3fff to 4c3e6ac Compare December 8, 2022 00:24
src/Statistics.jl Outdated Show resolved Hide resolved
@IanButterworth
Copy link
Contributor Author

@ViralBShah good to go ahead with this, and remove SparseArrays from the sysimage once bumped on julia master? (I don't have merge rights here)

@KristofferC
Copy link
Member

good to go ahead with this

Didn't we find out that extensions didn't work right now for stdlibs? To me, it only makes sense to merge this once Statistics.jl is not an stdlib?

@vtjnash
Copy link
Contributor

vtjnash commented Aug 31, 2023

It seemed to work for me, so I think we should try this

@KristofferC
Copy link
Member

Okay, let's go with this then. Thanks for testing!

@KristofferC KristofferC merged commit 81a90af into JuliaStats:master Sep 1, 2023
@IanButterworth IanButterworth deleted the ib/glue branch September 1, 2023 10:36
@nalimilan
Copy link
Member

Unfortunately this fails if I bump Statistics to the latest commit on Julia master. And indeed even after using SparseArrays, the cov method defined in the extension isn't used for some reason.

   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.11.0-DEV.473 (2023-09-15)
 _/ |\__'_|_|_|\__'_|  |  Commit 42d98a2ed14* (0 days old master)
|__/                   |

julia> Base.runtests(["Statistics"])
Running parallel tests with:
  nworkers() = 1
  nthreads() = 1
  Sys.CPU_THREADS = 4
  Sys.total_memory() = 15.282 GiB
  Sys.free_memory() = 7.305 GiB

Test   (Worker) | Time (s) | GC (s) | GC % | Alloc (MB) | RSS (MB)
Statistics  (1) |        started at 2023-09-15T19:57:33.267
Statistics  (1) |         failed at 2023-09-15T19:58:48.347
Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:963
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:964
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:963
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:964
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:963
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:964
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:963
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:964
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 1, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 1, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 1 … 1 1; … ; 0 1 … 1 1; 0 1 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:978
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 0, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 0, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 0 … 0 0; … ; 0 0 … 1 1; 0 0 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:979
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 0, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 0, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 0 … 0 0; … ; 0 0 … 1 1; 0 0 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:978
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 0, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 0, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 0 … 0 0; … ; 0 0 … 1 1; 0 0 … 1 1]

Test Failed at /home/milan/Dev/julia/usr/share/julia/stdlib/v1.11/Statistics/test/runtests.jl:979
  Expression: isfinite.(cov_sparse) == isfinite.(cov_dense)
   Evaluated: sparse([1, 2, 3, 4, 5, 6, 7, 8, 9, 10  …  1, 2, 3, 4, 5, 6, 7, 8, 9, 10], [1, 1, 1, 1, 1, 1, 1, 1, 1, 1  …  10, 10, 10, 10, 10, 10, 10, 10, 10, 10], Bool[0, 0, 0, 0, 0, 1, 1, 0, 0, 0  …  0, 0, 1, 1, 1, 1, 1, 1, 1, 1], 10, 10) == Bool[0 0 … 0 0; 0 0 … 0 0; … ; 0 0 … 1 1; 0 0 … 1 1]



Test Summary: | Pass  Fail  Total     Time
  Overall     |  821    12    833  1m16.5s
    Statistics |  821    12    833  1m15.7s
    FAILURE

The global RNG seed was 0x7eb6b98a471ff52f1a5a23d7acd773bb.

vtjnash added a commit to JuliaLang/julia that referenced this pull request Dec 8, 2023
Some groundwork for
JuliaStats/Statistics.jl#134 which is bumped
(and thus tested) in #52431 that will be merged after this
@KristofferC
Copy link
Member

@nalimilan, I think this is fixed now with JuliaLang/julia#52428

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.

5 participants