Skip to content

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

Merged
merged 9 commits into from
Sep 26, 2022
Merged

Add analysis points #110

merged 9 commits into from
Sep 26, 2022

Conversation

baggepinnen
Copy link
Contributor

@baggepinnen baggepinnen commented Sep 19, 2022

This PR adds something called an AnalysisPoint. These are used for linear analyses, such as

  • Computing sensitivity functions.
  • Computing complementary sensitivity functions.
  • Computing loop-transfer functions.
  • Linearizing systems between predefined analysis points.
  • Open loops

An AnalysisPoint can be created explicitly, but also automatically when connecting an output to an input. Analysis points are causal, and are thus defined in the Blocks submodule. They thus expect connections of type Blocks.RealInput and Blocks.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.
  • The user must currently call expand_analysis_points manually before simulating a system containing analysis points. It would be good if this would be handled automatically by structural_simplify. The current implementation requires expand_analysis_points to run before expand_connections, but I could not find a clean way of extending this functionality from outside of ModelingToolkit. One approach would be to define AnalysisPoint in MTK, but keep the functions related to sensitivity functions here (they require components from MTKStdlib)
  • The last commit depends on the PR below
  • Add namespace_expr for Connection ModelingToolkit.jl#1827

@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #110 (c90780a) into main (a8bb688) will increase coverage by 0.53%.
The diff coverage is 70.86%.

@@            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     
Impacted Files Coverage Δ
src/Blocks/analysis_points.jl 70.86% <70.86%> (ø)
src/Blocks/utils.jl 61.53% <0.00%> (+4.39%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines 63 to 64
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))
Copy link
Contributor Author

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

@baggepinnen baggepinnen force-pushed the analysis_points branch 2 times, most recently from 2c8e891 to de1151f Compare September 19, 2022 12:08
@@ -1,4 +1,5 @@
[deps]
ControlSystemsBase = "aaaaaaaa-a6ca-5380-bf3e-84a91bcd477e"
Copy link
Member

Choose a reason for hiding this comment

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

Remember compat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@baggepinnen baggepinnen changed the title WIP: add analysis points Add analysis points Sep 23, 2022
Copy link
Member

@YingboMa YingboMa left a 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.

@YingboMa
Copy link
Member

YingboMa commented Sep 26, 2022

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)

@YingboMa YingboMa merged commit b7a9196 into SciML:main Sep 26, 2022
@baggepinnen baggepinnen deleted the analysis_points branch January 3, 2025 12:58
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.

2 participants