-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
start trying to precompile ModelingToolkit better #1215
Conversation
The symbolic-based libraries are going to be more like the plotting libraries in terms of inference. We'll probably need to dig around for things to Though curiously, one of the issues is that the profile doesn't seem to always be based in the library. @timholy do you know why that call stack doesn't show what in ModeingToolkit/Symbolics is making the call to broadcast that is then being compiled? A lot of the examples seem "unrooted" here so it's hard to really pin down the sources right now. |
src/precompile.jl
Outdated
@variables x(t) y(t) z(t) | ||
D = Differential(t) | ||
|
||
eqs = [D(D(x)) ~ σ*(y-x) + 0.000000000000135, |
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.
Note the trick here: precompiling the runtime generated function and then calling it leads to an error because it's not in the cache, so I just chose an ODE no one would ever solve so we wouldn't hit a hash that's the same. Too hacky? 😅
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.
Oof. Can we add a lot of underscores and a more "random" number?
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.
Clever! Like all great hacks. I think it gets the job done.
Yeah, this is more like what I'm accustomed to seeing from packages with significant inference issues. It's almost a Makie-like trace 🙂.
First, apologies if I'm repeating stuff you already know. If a call is dispatched at runtime, it serves as a new root. When that happens, we've set it up so that inference grabs a That new root might live in a different module than the caller. And you may have an "ownership" problem (the red bars), where a method is acting on types that it doesn't know about (defined in another package that it doesn't load). The solution is to improve inferrability so that you effectively pick up the flames with red bases and place them atop something you do own---then there's a backedge linking them together, and the whole thing will get put in the precompile cache of a package that knows about both the methods and the types. The alternative approach is to walk your way up a red-based flame until you get to something that isn't red, and then put a suitable precompile exercise in that package. The one good thing about There's a demo you can walk through yourself at https://timholy.github.io/SnoopCompile.jl/stable/snoopi_deep_analysis/, essentially a guided-tour exercise in ameliorating inference problems. Again, this might not really have answered your question, so feel free to keep asking. |
One thing that might be an easy win: for types like
Vector{T} instead of just Vector ? Even Vector{Any} may be better than Vector :
julia> isconcretetype(Vector)
false
julia> isconcretetype(Vector{Any})
true Items taken out of the vector will not be inferrable, but at least the container itself is inferrable. |
I tried this precompile load against JuliaLang/julia#43990 but got what looks like a compiler error:
This happens even on 1.7. Is there something that needs updating, or is this a Julia bug? |
```julia using ModelingToolkit, OrdinaryDiffEq function f() @parameters t σ ρ β @variables x(t) y(t) z(t) D = Differential(t) eqs = [D(D(x)) ~ σ*(y-x), D(y) ~ x*(ρ-z)-y, D(z) ~ x*y - β*z] @nAmed sys = ODESystem(eqs) sys = ode_order_lowering(sys) u0 = [D(x) => 2.0, x => 1.0, y => 0.0, z => 0.0] p = [σ => 28.0, ρ => 10.0, β => 8/3] tspan = (0.0,100.0) prob = ODEProblem(sys,u0,tspan,p,jac=true) end using SnoopCompile tinf = @snoopi_deep f() ``` ```julia Before: InferenceTimingNode: 8.138765/20.550152 on Core.Compiler.Timings.ROOT() with 821 direct children InferenceTimingNode: 8.152606/20.643050 on Core.Compiler.Timings.ROOT() with 821 direct children After: InferenceTimingNode: 8.216759/17.715817 on Core.Compiler.Timings.ROOT() with 839 direct children InferenceTimingNode: 8.272943/17.854555 on Core.Compiler.Timings.ROOT() with 840 direct children ``` only 2 seconds for now, but it's a start.
730092d
to
3c45aea
Compare
@timholy this branch is updated now. And oof, we see some regressions haha... using ModelingToolkit, OrdinaryDiffEq
function f()
@parameters t σ ρ β
@variables x(t) y(t) z(t)
D = Differential(t)
eqs = [D(D(x)) ~ σ*(y-x),
D(y) ~ x*(ρ-z)-y,
D(z) ~ x*y - β*z]
@named sys = ODESystem(eqs)
sys = structural_simplify(sys)
u0 = [D(x) => 2.0,
x => 1.0,
y => 0.0,
z => 0.0]
p = [σ => 28.0,
ρ => 10.0,
β => 8/3]
tspan = (0.0,100.0)
prob = ODEProblem(sys,u0,tspan,p,jac=true)
end
using SnoopCompile
tinf = @snoopi_deep f()
# InferenceTimingNode: 14.376150/29.364275 on Core.Compiler.Timings.ROOT() with 1263 direct children |
Thanks! I posted some numbers in the edited OP of JuliaLang/julia#43990. On that Julia build, it's almost all non-inference time (18s out of 20s). Obviously that would be the next thing to tackle. Pretty significant increase in load time, unfortunately. I haven't dug into the specifics, it's possible some of it may be fixable. |
That PR is still net beneficial here though, so that's good! There's a lot we can do in this library. We're changing the internals to be type-stable etc. so it should see a rather drastic change anyways. With our powers combined it it should get there. |
If that PR gets merged it may be important to develop some tooling that can help optimize placement of precompiles across packages. If you have a "base" package A, and B, C, and D each use A and end up recompiling some of the same machinery, that costs you. (That was the origin of JuliaLang/julia#43990 (comment).) If possible it would be better to ensure that A does the precompilation. But aside from seeing the moment of inference itself, we can't easily figure that stuff out now. I may take a second stab at a package I started, which could be subtitled "what's in my *.ji file anyway?" That would allow us to analyze what should go where. Obviously, native code is another frontier, but that's for another day (well, month). |
I can't wait. But yeah at least that PR makes precompilation in these kinds of packages work, so we can add precompilation forcing all throughout the symbolics packages which would naturally force less here, etc. But now even before we add it to A we can get usable downstream results, which makes it easier to start getting precompiles strewn around. |
Rollback #1215 as it causes package compilation to fail
only 2 seconds for now, but it's a start.