Skip to content

Handle ChainRulesCore via Pkg extension #206

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

Merged
merged 3 commits into from
Jun 7, 2023

Conversation

oschulz
Copy link
Contributor

@oschulz oschulz commented Jun 6, 2023

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
Copy link
Member

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
Copy link
Contributor Author

oschulz commented Jun 6, 2023

@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 .

Julia v1.6 uses different Project.toml formatting
@oschulz
Copy link
Contributor Author

oschulz commented Jun 6, 2023

Aqua tests should pass now, hopefully.

@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

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

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

Additional details and impacted files
@@            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 Coverage Δ
ext/LinearMapsChainRulesCoreExt.jl 100.00% <ø> (ø)
src/LinearMaps.jl 100.00% <ø> (ø)

... and 2 files with indirect coverage changes

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

@oschulz
Copy link
Contributor Author

oschulz commented Jun 6, 2023

@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
Copy link
Member

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

@oschulz
Copy link
Contributor Author

oschulz commented Jun 6, 2023

Version is bumped, thanks @dkarrasch !

@dkarrasch dkarrasch merged commit f522a8f into JuliaLinearAlgebra:master Jun 7, 2023
@oschulz oschulz deleted the use-extensions branch June 27, 2023 20:18
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.

2 participants