refactor: use IRInfo and IRStructure for full_equations#4525
refactor: use IRInfo and IRStructure for full_equations#4525AayushSabharwal wants to merge 3 commits into
IRInfo and IRStructure for full_equations#4525Conversation
AayushSabharwal
force-pushed
the
as/faster-full-eqs
branch
from
dab4b8e to
1611c95
Compare
| function full_equations(sys::AbstractSystem; simplify = false) | ||
| subsys = get_systems(sys) | ||
| # Fast path using `IRInfo` | ||
| if isempty(subsys) |
There was a problem hiding this comment.
why can we only do this if we don't have subsytems? Is that fixable?
There was a problem hiding this comment.
I'll add a comment. get_ir_info will build the IRInfo if a cached one does not exist. Doing so requires the ParameterBindingsGraph, which is only valid for flattened systems. I'll add a comment.
There was a problem hiding this comment.
how hard would it be to remove that restriction? The bifurcation here seems sub-optimal.
There was a problem hiding this comment.
Probably not very hard, but it doesn't matter much. We hit the slow path for hierarchical systems, which we can't simulate anyway. The only case where full_equations on a hierarchical system matters is if the user is directly building a system that they will complete and never mtkcompile. It's safe to assume those systems are few and far between, and not large enough for the speedup here to matter. Flattening the system will also invalidate the cached IRInfo, so we'll redo this anyway.
There was a problem hiding this comment.
fair enough. I'm just coming at this from a code simplicity perspective, but I'm fine with this as is.
AayushSabharwal
force-pushed
the
as/faster-full-eqs
branch
from
586c121 to
dfb8ad1
Compare
2 participants
This is a considerable improvement for large models.