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

Move default alg choice sooner to improve inner loop stability #1573

Merged
merged 9 commits into from
Jan 22, 2022

Conversation

ChrisRackauckas
Copy link
Member

No description provided.

@ChrisRackauckas ChrisRackauckas force-pushed the default_alg branch 2 times, most recently from bcf137d to 2f5c0a3 Compare January 19, 2022 11:02
@ChrisRackauckas
Copy link
Member Author

using OrdinaryDiffEq, ParameterizedFunctions, BenchmarkTools
using LinearSolve

f = @ode_def Orego begin
  dy1 = p1*(y2+y1*(1-p2*y1-y2))
  dy2 = (y3-(1+y1)*y2)/p1
  dy3 = p3*(y1-y3)
end p1 p2 p3

p = [77.27,8.375e-6,0.161]
prob = ODEProblem(f,[1.0,2.0,3.0],(0.0,30.0),p)
@btime solve(prob,Rosenbrock23(),abstol=1e-6,reltol=1e-6)
# Before:
# 1.405 ms (23476 allocations: 1.95 MiB)
# After:
# 1.423 ms (23478 allocations: 1.95 MiB)

@btime solve(prob,Rosenbrock23(linsolve=RFLUFactorization()),abstol=1e-6,reltol=1e-6)
# Before:
# 1.262 ms (10521 allocations: 952.25 KiB)
# After:
# 1.252 ms (10521 allocations: 952.25 KiB)

It's surprising this isn't helpful while making the choice is.

@ChrisRackauckas ChrisRackauckas force-pushed the default_alg branch 2 times, most recently from 6f97162 to 4a3f596 Compare January 20, 2022 13:25
@ChrisRackauckas
Copy link
Member Author

ChrisRackauckas commented Jan 20, 2022

Fixed:

using OrdinaryDiffEq, ParameterizedFunctions, BenchmarkTools
using LinearSolve

f = @ode_def Orego begin
  dy1 = p1*(y2+y1*(1-p2*y1-y2))
  dy2 = (y3-(1+y1)*y2)/p1
  dy3 = p3*(y1-y3)
end p1 p2 p3

p = [77.27,8.375e-6,0.161]
prob = ODEProblem(f,[1.0,2.0,3.0],(0.0,30.0),p)
solve(prob,Rosenbrock23(),abstol=1e-6,reltol=1e-6)

@btime solve(prob,Rosenbrock23(),abstol=1e-6,reltol=1e-6)
# Before:
# 1.405 ms (23476 allocations: 1.95 MiB)
# After:
# 1.317 ms (16999 allocations: 1.06 MiB)

@btime solve(prob,Rosenbrock23(linsolve=RFLUFactorization(),chunk_size = Val{3}()),abstol=1e-6,reltol=1e-6)
# Before:
# 1.262 ms (10521 allocations: 952.25 KiB)
# After:
# 1.323 ms (16997 allocations: 1.06 MiB)

But we found an inference bug.

alloc_lu

@ChrisRackauckas
Copy link
Member Author

@chriselrod @YingboMa that seems to be something in RecursiveFactorization or JuliaSIMD stuff. Don't know if there's a workaround JuliaLang/julia#35800

@chriselrod
Copy link
Contributor

@chriselrod @YingboMa that seems to be something in RecursiveFactorization or JuliaSIMD stuff. Don't know if there's a workaround JuliaLang/julia#35800

Yeah, that's why I removed the precompilation from TriangularSolve.jl.
With precompilation, performance was bad until you forced it to recompile with Revise.jl.
Without the precompilation, it was fast the first time. =/

I don't know inference or the compiler well enough to know what causes.
Jameson suggested it probably means the code is bad.
I define code quality mostly based on outcomes, so that seems true by definition and not helpful for finding the problem and fixing it.

@YingboMa
Copy link
Member

The failure looks real.

src/alg_utils.jl Outdated Show resolved Hide resolved
@ChrisRackauckas ChrisRackauckas merged commit 1cce39f into master Jan 22, 2022
@ChrisRackauckas ChrisRackauckas deleted the default_alg branch January 22, 2022 03:35
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.

3 participants