JuliaLinearAlgebra / LinearMaps.jl

A Julia package for defining and working with linear maps, also known as linear transformations or linear operators acting on vectors. The only requirement for a LinearMap is that it can act on a vector (by multiplication) efficiently.
Other
303 stars 42 forks source link

Handle ChainRulesCore via Pkg extension #206

Closed oschulz closed 1 year ago

oschulz commented 1 year ago

Substantially reduces the load time of LinearMaps on Julia v1.9.

Before:

julia> @time import LinearMaps
  0.053604 seconds (73.57 k allocations: 4.871 MiB, 6.36% compilation time)
julia> @time_imports import LinearMaps, ChainRulesCore
      0.6 ms  Statistics
      0.2 ms  Compat
      0.2 ms  Compat → CompatLinearAlgebraExt
     24.7 ms  ChainRulesCore
     25.3 ms  LinearMaps

After:

julia> @time import LinearMaps
  0.037479 seconds (39.45 k allocations: 2.582 MiB, 12.74% compilation time)
julia> @time_imports import LinearMaps, ChainRulesCore
      0.5 ms  Statistics
     24.8 ms  LinearMaps
      0.2 ms  Compat
      0.1 ms  Compat → CompatLinearAlgebraExt
     30.9 ms  ChainRulesCore
      0.2 ms  LinearMaps → LinearMapsChainRulesCoreExt
dkarrasch commented 1 year ago

Thank you very much! Do you understand the complaint by Aqua.jl? Do we need to move the additions to Project.toml to a different place? The load time gain is great. Looks like almost all the load time is spent on ChainRulesCore?

oschulz commented 1 year ago

@dkarrasch The Aqua think I had hoped was handled now, but apparently not yet. It's just that Julia v1.6 formats Project.toml differently if there are weakdeps entries (which it doesn't know about) in it. So the Project.toml formatting tests has to be disabled on v1.6.

The invalidations failure seems unavoidable with Pkg extensions right now, see PainterQubits/Unitful.jl#652 .

oschulz commented 1 year ago

Aqua tests should pass now, hopefully.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +0.26 :tada:

Comparison is base (50ace9c) 99.41% compared to head (adfd37e) 99.67%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #206 +/- ## ========================================== + Coverage 99.41% 99.67% +0.26% ========================================== Files 19 19 Lines 1537 1538 +1 ========================================== + Hits 1528 1533 +5 + Misses 9 5 -4 ``` | [Impacted Files](https://app.codecov.io/gh/JuliaLinearAlgebra/LinearMaps.jl/pull/206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaLinearAlgebra) | Coverage Δ | | |---|---|---| | [ext/LinearMapsChainRulesCoreExt.jl](https://app.codecov.io/gh/JuliaLinearAlgebra/LinearMaps.jl/pull/206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaLinearAlgebra#diff-ZXh0L0xpbmVhck1hcHNDaGFpblJ1bGVzQ29yZUV4dC5qbA==) | `100.00% <ø> (ø)` | | | [src/LinearMaps.jl](https://app.codecov.io/gh/JuliaLinearAlgebra/LinearMaps.jl/pull/206?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaLinearAlgebra#diff-c3JjL0xpbmVhck1hcHMuamw=) | `100.00% <ø> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/JuliaLinearAlgebra/LinearMaps.jl/pull/206/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaLinearAlgebra)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

oschulz commented 1 year ago

@dkarrasch This should be it now - from what I understand, the invalidations failure has to be ignored for now until with Pkg extensions, until that's handled upstream.

dkarrasch commented 1 year ago

Could you please bump the patch number and then I'll release shortly after merging. Thanks again!

oschulz commented 1 year ago

Version is bumped, thanks @dkarrasch !