Generalized linear two terminal State Space Nodal component models#406
Conversation
leonardocarreras
left a comment
There was a problem hiding this comment.
Hi! Thanks for your PR!
I left some comments mostly on the software. I did not check the equations.
Please consider using pre-commit automatically, so it will format it before you push.
dpsim-models/include/dpsim-models/EMT/EMT_Ph1_SSN_Full_Serial_RLC.h
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR implements generalized linear two-terminal State Space Nodal (SSN) component models to extend the existing component modeling framework. The implementation provides flexible abstractions for arbitrary linear two-terminal components that can be described using state space matrices A, B, C, and D.
Key changes:
- Introduces two new generalized SSN component types: SSNTypeV2T (voltage-type) and SSNTypeI2T (current-type)
- Adds an explicit Full_Serial_RLC implementation using traditional SSN methods
- Extends the Python bindings to make all three components accessible via pybind
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| EMT_generalizedSSN_RLC_dpsimpy.ipynb | Test notebook comparing traditional RLC, explicit SSN, and generalized SSN implementations |
| EMTComponents.cpp | Python bindings for new SSN components with parameter setters and attribute getters |
| EMT_Ph1_General2TerminalSSN.cpp | C++ test examples demonstrating usage of generalized SSN components |
| CMakeLists.txt | Build configuration updates to include new test file |
| MathUtils.cpp/h | New overloaded trapezoidal discretization method for A,B matrices only |
| SSN component implementations | Three new component classes with their headers and source files |
| Components.h | Header includes for new SSN components |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
examples/Notebooks/Circuits/EMT_generalizedSSN_RLC_dpsimpy.ipynb
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #406 +/- ##
==========================================
+ Coverage 65.22% 65.69% +0.47%
==========================================
Files 374 381 +7
Lines 22723 23418 +695
Branches 11239 11695 +456
==========================================
+ Hits 14820 15385 +565
- Misses 7889 7993 +104
- Partials 14 40 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5fc98ba to
b84ed35
Compare
|
Hi @MarvinTollnitschRWTH @georgii-tishenin, if you want, you can rebase and confirm that this will be good to merge |
891e729 to
5637217
Compare
georgii-tishenin
left a comment
There was a problem hiding this comment.
Hi @MarvinTollnitschRWTH,
thanks for addressing the comments.
I rebased the PR, resolved the addressed and not critical comments. All checks now go through as well. There are still a few active comments left, the main topics are:
- use SPDX header in cpp files as well.
- do not flush in functions that are not part of component's initialization.
- make sure there are no outputs in .ipynb files in git history (strangely, the difference shows it for one of the commits).
Please address the active comments or reply to them if you do not agree. Then, we can merge.
@georgii-tishenin both options work, i think |
5637217 to
fb89ede
Compare
+ added declaration for "static void calculateStateSpaceTrapezoidalMatrices(...)" which transforms state space matrices A and B and omits C and D to "MathUtils.h" + added corresponding definition of "static void calculateStateSpaceTrapezoidalMatrices(...)" to "MathUtils.cpp" Signed-off-by: Marvin Tollnitsch <marvin.tollnitsch@rwth-aachen.de>
+ added "EMT::Ph1::SSN::Full_Serial_RLC" class with "EMT_Ph1_SSN_Full_Serial_RLC.h" and "EMT_Ph1_SSN_Full_Serial_RLC.cpp"
+ added "EMT::Ph1::SSNTypeI2T" class with "EMT_Ph1_SSNTypeI2T.h" and "EMT_Ph1_SSNTypeI2T.cpp" for a generalized model of an arbitrary two terminal I-type SSN component
+ added "EMT::Ph1::SSNTypeV2T" class with "EMT_Ph1_SSNTypeV2T.h" and "EMT_Ph1_SSNTypeV2T.cpp" for a generalized model of an arbitrary two terminal V-type SSN component
+ added above headers ("EMT_Ph1_SSN_Full_Serial_RLC.h", "EMT_Ph1_SSNTypeI2T.h", "EMT_Ph1_SSNTypeV2T.h") to "Components.h" to include them into the project
+ added above cpp files ("EMT_Ph1_SSN_Full_Serial_RLC.cpp", "EMT_Ph1_SSNTypeI2T.cpp", "EMT_Ph1_SSNTypeV2T.cpp") to the "CMakeLists.txt" for component models
Signed-off-by: Marvin Tollnitsch <marvin.tollnitsch@rwth-aachen.de>
Signed-off-by: Marvin Tollnitsch <marvin.tollnitsch@rwth-aachen.de>
+ added "EMT_Ph1_General2TerminalSSN.cpp" and corresponding notebook "EMT_Ph1_generalizedSSN.ipynb" + added "EMT_Ph1_General2TerminalSSN.cpp" to "examples/cxx/CMakeLists.txt" as a circuit source + added "EMT_generalizedSSN_RLC_dpsimpy.ipynb" to test generalized SSN RLC implementation in a circuit via pybind Signed-off-by: Marvin Tollnitsch <marvin.tollnitsch@rwth-aachen.de>
fb89ede to
2899c09
Compare
Signed-off-by: Georgii Tishenin <georgii.tishenin@eonerc.rwth-aachen.de>
This PR generalizes the State Space Nodal (SSN) component model implementation by introducing two new linear two-terminal component types defined by state-space matrices A, B, C, and D:
SSNTypeV2T– Adds a nodal equation to the MNA system matrix.SSNTypeI2T– Adds a mesh equation to the MNA system matrix (analogous to a voltage source).Additional updates included in this PR: