Slow jump construction likely due to substitute

[isaacsas]

On Discourse, here, a graph SIR model was reported to take an exceptionally long time to construct the JumpProblem (18.55s out of a total time to run a simulation of 18.85s).

On running the profiler it seems the overwhelming majority of the time is being taken in the substitute call within assemble_maj at

ModelingToolkit.jl/src/systems/jumps/jumpsystem.jl

Line 97 in 6afdbd2

pval = simplify(substitute(sr,parammap)).value

I'll look into whether we can detect that the rate is just a symbol and skip this call, which should be the case for this example, but more generally is there anything that can be done about this?

[ChrisRackauckas]

@shashi could you take a look at this? substitute shouldn't take that much time here.

[isaacsas]

FWIW it is just simplifying essentially p/1 for p different parameters, so nothing complicated.

[ChrisRackauckas]

Yeah, so if we need an alternative implementation for this kind of thing that's fine, but I'm curious how any library can take 18 seconds on that.

[erlebach]

You were able to duplicate my timings?

[isaacsas]

Yes. and the issue is clear. We can fix it easily, it is just a matter of trying to avoid a temp fix for a more robust solution.

@ChrisRackauckas in that same section of code a Dict is being built that is keyed by Operations. Is that a good idea? I usually use Variables as keys.

[ChrisRackauckas]

It's probably not a good idea to hit a dictionary here, though hashing may be useful when there's tons of variables? There just has to be a better way.

[isaacsas]

But is hashing on an Operation ok to do?

[ChrisRackauckas]

Maybe

[isaacsas]

I think I had originally written this to take a premade Dict mapping Variables to numeric values when it used evaluation in a Module context. That's basically what we used in DEBio with Symbols and it had good performance (note the Dict is only built once in the JumpProblem call, not constructed each time within the jump call).

[ChrisRackauckas]

We might need to revert back to that.

[isaacsas]

I'm happy to revert it back if @shashi thinks getting a more permanent fix will take a while. Just let me know.

[ChrisRackauckas]

Let's revert for now and not wait on a permanent fix. The dict construction cost makes me think that the symbolicutils direction might just be a bad idea for substitution.

[isaacsas]

@erlebach #427 gives a big improvement. I can let you know once there is a new release with this fix.

There are some more optimizations we can do so we should leave this issue open for now. I'll try to profile some more to see what is rate limiting now over the next few days or on the weekend.

[ChrisRackauckas]

Fixed by #427