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

use a sym to ind dict #2206

Closed
wants to merge 3 commits into from
Closed

Conversation

chriselrod
Copy link
Contributor

No description provided.

@chriselrod
Copy link
Contributor Author

PR*:

julia> @benchmark ModelingToolkit.SymbolicIndexingInterface.state_sym_to_index($seir_model, $I)  
BenchmarkTools.Trial: 8023 samples with 907 evaluations.
 Range (min  max):  125.115 ns   20.456 μs  ┊ GC (min  max): 0.00%  99.06%
 Time  (median):     130.808 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   136.253 ns ± 276.440 ns  ┊ GC (mean ± σ):  3.14% ±  1.56%

              ▅▆█▆▄▁                                             
  ▁▁▂▂▃▃▂▂▂▃▄▇██████▇▅▄▄▅▆█▇▇▇▇▆▆▅▄▄▃▃▃▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▁▁▁ ▃
  125 ns           Histogram: frequency by time          143 ns <

 Memory estimate: 16 bytes, allocs estimate: 1.

Master:

julia> @benchmark ModelingToolkit.SymbolicIndexingInterface.state_sym_to_index($seir_model, $I)  
BenchmarkTools.Trial: 10000 samples with 10 evaluations.
 Range (min  max):  1.202 μs  984.542 μs  ┊ GC (min  max):  0.00%  99.26%
 Time  (median):     1.297 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   1.758 μs ±  18.701 μs  ┊ GC (mean ± σ):  24.67% ±  2.43%

      ▂▃▅▇█▇▆▅▄▃▂▁                                             
  ▁▂▄▆█████████████▇▅▅▄▄▃▃▃▃▃▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▁▁▁▂▁▁▁▁▁▁▁▁▁▁▁▁▁ ▃
  1.2 μs          Histogram: frequency by time        1.69 μs <

 Memory estimate: 2.42 KiB, allocs estimate: 19.

*With these PRs, to speed up dict lookups:
YingboMa/Unityper.jl@85e77bf
JuliaSymbolics/Symbolics.jl#927
JuliaSymbolics/SymbolicUtils.jl#533

function ODESystem(tag, deqs, iv, dvs, ps, tspan, var_to_name, ctrls, observed, tgrad,
jac, ctrl_jac, Wfact, Wfact_t, name, systems, defaults,
torn_matching, connector_type, preface, cevents,
devents, metadata = nothing, gui_metadata = nothing,
tearing_state = nothing,
substitutions = nothing, complete = false,
discrete_subsystems = nothing, unknown_states = nothing;
discrete_subsystems = nothing, unknown_states = nothing,
sym_to_index = Dict{Num, Int}();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sym_to_index = Dict{Num, Int}();
sym_to_index = Dict{Any, Int}();

@@ -25,7 +25,7 @@ eqs = [D(x) ~ σ*(y-x),
struct ODESystem <: AbstractODESystem
"""
tag: a tag for the system. If two systems have the same tag, then they are
structurally identical.
structurally ide, sym_to_stringntical.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deliberate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No

@ChrisRackauckas
Copy link
Member

@AayushSabharwal something of the sort is done at completion? I thought some indexing stuff was added

@AayushSabharwal
Copy link
Member

Yeah indexes are cached in Dicts now.

@chriselrod chriselrod deleted the dictforsymtoind branch February 23, 2024 20:08
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.

4 participants