-
Notifications
You must be signed in to change notification settings - Fork 172
Unique function argument names for no recompilation #272
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
Fixes https://discourse.julialang.org/t/function-compiles-every-time/63273/5 by giving the arguments unique names. MWE: ```julia using ModelingToolkit, OrdinaryDiffEq @variables t x(t) RHS(t) # independent and dependent variables @parameters τ # parameters D = Differential(t) # define an operator for the differentiation w.r.t. time # your first ODE, consisting of a single equation, indicated by ~ @nAmed fol_separate = ODESystem([ RHS ~ (1 - x)/τ, D(x) ~ RHS ]) sys = structural_simplify(fol_separate) @time begin prob = ODEProblem(sys, [x => 0.0], (0.0,10.0), [τ => 3.0]) sol = solve(prob, Tsit5()) end ``` ``` 0.001047 seconds (2.20 k allocations: 126.812 KiB) 0.001023 seconds (2.20 k allocations: 126.812 KiB) 0.001021 seconds (2.20 k allocations: 125.672 KiB) 0.001045 seconds (2.20 k allocations: 125.672 KiB) ``` while it used to be 99.9% compile time.
| linenumbers = true) | ||
|
|
||
| dargs = map(arg -> destructure_arg(arg, !checkbounds), [args...]) | ||
| dargs = map((x) -> destructure_arg(x[2], !checkbounds, Symbol("ˍ₋arg$(x[1])")), enumerate([args...])) |
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.
two unicode underscores are better than one?
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #272 +/- ##
==========================================
- Coverage 74.44% 74.36% -0.09%
==========================================
Files 19 19
Lines 1761 1759 -2
==========================================
- Hits 1311 1308 -3
- Misses 450 451 +1 ☔ View full report in Codecov by Sentry. |
|
|
||
| function numbered_expr(de::Equation,varnumbercache,args...;varordering = args[1], | ||
| lhsname=gensym("du"),rhsnames=[gensym("MTK") for i in 1:length(args)],offset=0) | ||
| lhsname=:du,rhsnames=[Symbol("MTK$i") for i in 1:length(args)],offset=0) |
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.
These are only used in C and MATLAB outputs iirc
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.
Might as well make the codegen stable there too.
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.
And in that case, we really don't need to gensym since it's not going to take globals from Julia 😅
| function destructure_arg(arg::Union{AbstractArray, Tuple}, inbounds, name) | ||
| if !(arg isa Arr) | ||
| DestructuredArgs(map(value, arg), inbounds=inbounds) | ||
| DestructuredArgs(map(value, arg), name, inbounds=inbounds) |
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.
nice!
|
This might not solve the root issue. MTK is still regenerating the same function again. |
|
That function generation is rather cheap though. This is orders of magnitude better than before at least. |
Fixes https://discourse.julialang.org/t/function-compiles-every-time/63273/5 by giving the arguments unique names. MWE:
while it used to be 99.9% compile time. Tested on <1 and rebased.