-
-
Notifications
You must be signed in to change notification settings - Fork 45
Add analysis points #110
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
Add analysis points #110
Conversation
Codecov Report
@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 66.92% 67.45% +0.53%
==========================================
Files 24 25 +1
Lines 1052 1177 +125
==========================================
+ Hits 704 794 +90
- Misses 348 383 +35
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
34a93aa
to
f0a1f40
Compare
test/Mechanical/rotational.jl
Outdated
prob = DAEProblem(sys, D.(states(sys)) .=> 0.0, [D(D(inertia2.phi)) => 1.0; D.(states(model)) .=> 0.0], (0, 10.0)) | ||
prob = DAEProblem(sys, D.(states(sys)) .=> 0.0, | ||
[D(D(inertia2.phi)) => 1.0; D.(states(model)) .=> 0.0], (0, 10.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.
Unrelated change made by the formatter
3b721ee
to
18a2f9c
Compare
18a2f9c
to
4111d5e
Compare
2c8e891
to
de1151f
Compare
@@ -1,4 +1,5 @@ | |||
[deps] | |||
ControlSystemsBase = "aaaaaaaa-a6ca-5380-bf3e-84a91bcd477e" |
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.
Remember compat.
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.
This is in docs/Project.toml
where no packages have compat so far :P
https://github.com/SciML/ModelingToolkitStandardLibrary.jl/blob/de1151f3381b6f4c9a3205c767a076d1627308e6/docs/Project.toml
de1151f
to
4a7b354
Compare
4a7b354
to
16e349a
Compare
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.
flatten
doesn't work with connect, as mentioned in SciML/ModelingToolkit.jl#1826 (comment) . It should be deprecated and removed. Could you take a look at the generate_connection_*
functions in MTK? https://github.com/SciML/ModelingToolkit.jl/blob/644d76c73cbeb17458f606c7b299bd450edb0c44/src/systems/connectors.jl Also, it's expensive to traverse the model. It may make sense to add this in MTK. Alternatively, we should provide interfaces that makes expanding connection equations generic.
This is roughly how connection generation works in MTK. julia> sys_without_connections, connection_sets = ModelingToolkit.generate_connection_set(rc_model);
julia> connection_sets
7-element Vector{ModelingToolkit.ConnectionSet}:
<constant₊output.u(t)::inner, source₊V.u(t)::inner>
<source₊p.v(t)::inner, resistor₊p.v(t)::inner>
<source₊p.i(t)::inner, resistor₊p.i(t)::inner>
<resistor₊n.v(t)::inner, capacitor₊p.v(t)::inner>
<capacitor₊p.i(t)::inner, resistor₊n.i(t)::inner>
<capacitor₊n.v(t)::inner, source₊n.v(t)::inner, ground₊g.v(t)::inner>
<capacitor₊n.i(t)::inner, ground₊g.i(t)::inner, source₊n.i(t)::inner>
julia> connection_equations, stream_connection_sets = ModelingToolkit.generate_connection_equations_and_stream_connections(connection_sets);
julia> connection_equations
8-element Vector{Equation}:
constant₊output₊u(t) ~ source₊V₊u(t)
source₊p₊v(t) ~ resistor₊p₊v(t)
0 ~ resistor₊p₊i(t) + source₊p₊i(t)
resistor₊n₊v(t) ~ capacitor₊p₊v(t)
0 ~ capacitor₊p₊i(t) + resistor₊n₊i(t)
capacitor₊n₊v(t) ~ source₊n₊v(t)
capacitor₊n₊v(t) ~ ground₊g₊v(t)
0 ~ capacitor₊n₊i(t) + ground₊g₊i(t) + source₊n₊i(t) |
e398b49
to
c90780a
Compare
This PR adds something called an
AnalysisPoint
. These are used for linear analyses, such asAn
AnalysisPoint
can be created explicitly, but also automatically whenconnect
ing an output to an input. Analysis points are causal, and are thus defined in theBlocks
submodule. They thus expect connections of typeBlocks.RealInput
andBlocks.ReaOutput
.Notes to reviewers
The current state of the PR does not handle namespaces and subsystems properly. In particular, I'm unsure about the proper way of modifying equations and namespaces when changing the equations in a system with subsystems.Namespaces are now handled.expand_analysis_points
manually before simulating a system containing analysis points. It would be good if this would be handled automatically bystructural_simplify
. The current implementation requiresexpand_analysis_points
to run beforeexpand_connections
, but I could not find a clean way of extending this functionality from outside of ModelingToolkit. One approach would be to defineAnalysisPoint
in MTK, but keep the functions related to sensitivity functions here (they require components from MTKStdlib)namespace_expr
forConnection
ModelingToolkit.jl#1827