Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR rewrites the
@componentsparsing and model construction logic. Instead of parsing the RHS of each equation in the@componentsblock, it evaluates it normally as plain julia code. Because the new code is a lot simpler, it was relatively easy to improve the flexibility of the model construction interface (see below) along the lines of #3791There's still a lot of work to do, so I'd like to get some feedback before pushing forward (... or giving up). Is that a reasonable direction to take?
The good
__syntax:@mtkcompile my_model = MyModel(sub_component__N=10)__syntax:@mtkcompile my_model = MyModel(sub_component__subsub_component=MyAlternativeComponent()). I believe this fixes Replacing subsystems #2038.resistors = [FixedResistor(), VariableResistor()].ifblocks now work inside of@componentsThe bad
Modelmetadata for components@test A.structure[:components] == [[:cc, :C]]now yields[[:cc, :unimplemented]]. (ButSystemmetadata is fine). How bad is that? Where is that information used? Maybe Dyad?Potential workaround: bring back
#master's parsing code just for metadata parsing. Maybe if I strip that code of everything except the metadata part, it would be reasonably small, and we can give up on more complex cases (that aren't supported ATM anyway)?The ugly
To support
plant__component__subcomponent => AlternativeComponent(), I usedScopedValuesto thread those parameters. I believe that it's broadly fine, but there might be edge cases I haven't thought about.TODO
If this PR looks appealing, then the remaining items would be
Let me know what you think! If this is too disruptive, I can tackle another direction. Most of these features can be implemented without getting rid of the RHS parsing code, but it's a lot more work.