DP_Ph3_PiLine, EMT_Ph1_PiLine and Ph3 diakoptics functionality#436
Conversation
FGH-wirtz
requested review from
dinkelbachjan,
gnakti and
m-mirz
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #436 +/- ##
==========================================
- Coverage 65.78% 64.52% -1.26%
==========================================
Files 381 385 +4
Lines 23445 23925 +480
Branches 11698 11811 +113
==========================================
+ Hits 15423 15438 +15
- Misses 7960 8486 +526
+ Partials 62 1 -61 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
FGH-wirtz
force-pushed
the
Additional_Pi_Line_and_3phDiakoptics
branch
3 times, most recently
from
d882f5e to
30da025
Compare
FGH-wirtz
force-pushed
the
Additional_Pi_Line_and_3phDiakoptics
branch
from
30da025 to
e9d284e
Compare
|
Hi @cwirtz-fgh, sorry for the short delay.... I will ask you to rebase the PR into the current master... I think many of the shown changes are unrelated to your PR at the moment, it will surely will be easier to check that way... |
FGH-wirtz
force-pushed
the
Additional_Pi_Line_and_3phDiakoptics
branch
3 times, most recently
from
2bcbd24 to
f8cac2f
Compare
leonardocarreras
left a comment
leonardocarreras
left a comment
There was a problem hiding this comment.
Hi @cwirtz-fgh ! Thanks for the PR... Some minor comments from my side...
(and maybe one consideration without a very strong opinion, for simple components like resistor/inductor/capacitor, would final be more appropriate than override for some of the functions? I would expect no inheritance from them)
In my opinion yes, but as components like EMT_Ph1_Resistor already use override i would see this as a more fundamental decision and seperate commit / refactoring. |
FGH-wirtz
force-pushed
the
Additional_Pi_Line_and_3phDiakoptics
branch
from
f8cac2f to
302fe21
Compare
- DP_Ph3_PiLine - EMT_Ph1_PiLine Extended Examples DP_PiLine.cpp and EMT_PiLine.cpp to test new Components. To provide Diakoptics equivalent to the DP 1Ph example, minor extension of diakoptics solver to provide 3ph capability was necessary. - tearing Matrix needs to differ between 1ph and 3ph case - added mnaTearing capabilities where missing, equivalent to 1ph DP implementation Signed-off-by: cwirtz <christoph.wirtz@fgh-ma.de>
FGH-wirtz
force-pushed
the
Additional_Pi_Line_and_3phDiakoptics
branch
from
302fe21 to
75ae179
Compare
leonardocarreras
left a comment
leonardocarreras
left a comment
There was a problem hiding this comment.
Looks good, lets see if someone else has an opinion in the next few days.
georgii-tishenin
left a comment
georgii-tishenin
left a comment
There was a problem hiding this comment.
Hi @cwirtz-fgh! Thanks for reviving the work on Diakoptics. this PR looks good to me. I have a few suggestions for potential follow-up PRs that could further improve DPsim:
- Add the new components to the pybind (see EMTComponents and DPComponents.) This will allow to use them in dpsimpy.
- Add Jupyter notebook(s) with assertions using dpsimpy. While we currently do not have formal testing guidelines, Python-based Jupyter notebooks are executed during CI. This helps track code coverage through examples and automatically detect changes in simulation results.
- Consider adding content on Diakoptics to documentation (e.g., may be Diakoptics can be added to the concepts)
leonardocarreras
merged commit 15b0ca1
into
sogno-platform:master
Added missing PiLine Models:
Extended Examples DP_PiLine.cpp and EMT_PiLine.cpp to test new Components.
To provide Diakoptics equivalent to the DP 1Ph example, minor extension of diakoptics solver to provide 3ph capability was necessary.
This is loosly connected to issue #431 (adds 3ph Diakoptics functionality).