-
-
Notifications
You must be signed in to change notification settings - Fork 65
[Do Not Merge] Preallocate more caches #470
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
Conversation
src/factorization.jl
Outdated
const PREALLOCATED_NORMALCHOLESKY = ArrayInterface.cholesky_instance( | ||
Symmetric(rand(1, 1)), NoPivot()) | ||
|
||
function init_cacheval(alg::NormalCholeskyFactorization, A::Matrix, b, u, Pl, Pr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function init_cacheval(alg::NormalCholeskyFactorization, A::Matrix, b, u, Pl, Pr, | |
function init_cacheval(alg::NormalCholeskyFactorization, A::Matrix{Float64}, b, u, Pl, Pr, |
src/default.jl
Outdated
# DefaultAlgorithmChoice.GenericLUFactorization | ||
DefaultAlgorithmChoice.RFLUFactorization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that's anything more than voodoo, since https://github.com/JuliaLinearAlgebra/RecursiveFactorization.jl/blob/master/src/lu.jl#L62-L69 . I don't think that uses more caches or anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RFLU implementation here bypasses that to directly call https://github.com/JuliaLinearAlgebra/RecursiveFactorization.jl/blob/425e43622e8bb4a9e0a00f843791ab40df2f9466/src/lu.jl#L89
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #470 +/- ##
==========================================
+ Coverage 27.90% 28.11% +0.21%
==========================================
Files 27 27
Lines 2118 2134 +16
==========================================
+ Hits 591 600 +9
- Misses 1527 1534 +7 ☔ View full report in Codecov by Sentry. |
c9cb3cd
to
6fd247c
Compare
These EnumX to Symbol mappings do have some unnecessary overhead, let me track them down |
julia> @btime solve(prob, $NewtonRaphson(; autodiff = AutoForwardDiff(; chunksize = 2)));
2.774 μs (46 allocations: 4.55 KiB)
julia> @btime solve(prob, $NewtonRaphson(; autodiff = AutoForwardDiff(; chunksize = 2), linsolve = \));
3.233 μs (49 allocations: 4.66 KiB)
julia> @btime solve(prob, $NewtonRaphson(; autodiff = AutoForwardDiff(; chunksize = 2), linsolve = GenericLUFactorization()));
2.620 μs (44 allocations: 4.23 KiB)
julia> @btime solve(prob, $NewtonRaphson(; autodiff = AutoForwardDiff(; chunksize = 2), linsolve = RFLUFactorization()));
2.607 μs (40 allocations: 3.91 KiB) |
4bb0210
to
085ba9d
Compare
I don't seem to have caused the Enzyme failures |
Testing SciML/SciMLBenchmarks.jl#858