Feat: SSN_DC domain components - #600
Conversation
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
There was a problem hiding this comment.
DPsim LLM review
Claim vs. code: The diff also includes unrelated synchronous-generator additions and docs/ignore changes, and the provided diff is truncated so the Python bindings claimed in the PR intent cannot be verified from what is shown.
TL;DR: One confirmed documentation/compliance issue and one repository hygiene issue need fixing; the remaining items are low-confidence, unverified modeling concerns around DC SSN sign/conductance behavior, and no stronger correctness problem was established from the provided sources.
Found 2 high, 5 medium (2 anchored to lines below).
🔴 Critical & high
- Add SPDX header to markdown handover document
[high · 96% confidence]indocs/development/SSN_DC_SESSION_HANDOVER.md:1(details inline)
🟡 Suggestions
- Ignore docs/development/ masks new handover documentation
[medium · 80% confidence]in.gitignore:68(details inline)
🔵 Optional / low-confidence (5)
- Remove untracked non-product directory .codex/
[high · 35% confidence · unconfirmed]in.codex/ - Confirm companion conductance sign for I‑type ports
[medium · 30% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:24 - Potential sign error in companion conductance calculation
[medium · 30% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:24 - Verify voltage sign in initial input builder
[medium · 25% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalVTypeSSNComp.h:22 - Missing documentation page for new DC component
[medium · 18% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_CurrentSource.h:12
Claim vs. implementation
- Claimed: Add scalar DC EMT support with PhaseType::DC, DC ideal sources, DC SSN resistor/capacitor/inductor/pi-line components, initialization, and Python bindings.
- Done: Adds PhaseType::DC, new DC base/component classes and EMT DC source/SSN implementations, plus related model/CMake wiring and a new synchronous-generator header.
- Difference: The diff also includes unrelated synchronous-generator additions and docs/ignore changes, and the provided diff is truncated so the Python bindings claimed in the PR intent cannot be verified from what is shown.
How this review was produced
13 specialized finder passes raised 76 findings over the diff and the full changed sources. After de-duplication, 76 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 69 refuted as unsupported, 7 kept (5 tentative).
Refuted by verification:
- Pi-line virtual-node voltage initialization inconsistent with companion model (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The initialization sets the virtual node to voltage0.real() + **mSeriesResistance * **mInitialCurrent, which is the intended series-R drop for the companion inductor/resistor chain.
- PiLine initialization omits virtual-node voltage propagation (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The virtual-node voltage is explicitly propagated via mVirtualNodes[0]->setInitialVoltage(Complex(virtualVoltage, 0.0)); in initializeParentFromNodesAndTerminals().
- Missing validation of shunt resistances in PiLine::createSubComponents (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The shunt-resistance path already guards the division with if (**mParallelConductance > 0.0) and then checks Math::isFinite(halfShuntResistance) before using it.
- Missing external tag on PostStep task for side-effecting voltage reconstruction (dpsim-models/src/EMT/EMT_DC_CurrentSource.cpp): mnaCompPostStep only calls mnaCompUpdateVoltage, and the class already marks the post-step task via dependency lists; no Scheduler::external tag exists in this code or base contract
- PreStep dependencies do not cover all reads and writes (dpsim-models/src/EMT/EMT_DC_CurrentSource.cpp): mnaCompAddPreStepDependencies already lists mCurrentRef in attributeDependencies and mIntfCurrent in modifiedAttributes, and mRightVector is intentionally in modifiedAttributes because it is written by the stamp
- PreStep task reads mCurrentRef without declaring it as a dependency (dpsim-models/src/EMT/EMT_DC_CurrentSource.cpp): mCurrentRef is explicitly added to attributeDependencies on line 108
- PostStep task reads leftVector without declaring it as a dependency (dpsim-models/src/EMT/EMT_DC_CurrentSource.cpp): mnaCompAddPostStepDependencies explicitly pushes leftVector into attributeDependencies on line 117
- Initialization does not set interface current to zero explicitly (dpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp): The file explicitly sets (**mIntfCurrent)(0, 0) = 0.0 in initializeFromNodesAndTerminals.
- Capacitor::setParameters wraps a plain parameter in an Attribute unnecessarily (dpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp): Base::DC::Capacitor defines mCapacitance as a public const Attribute::Ptr, so using an Attribute here is the established pattern.
- Capacitor initialization does not validate mParametersSet before accessing mCapacitance (dpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp): The code checks !mParametersSet before any access to **mCapacitance or other parameter-dependent state.
- Inductor::setParameters wraps plain parameters in Attributes unnecessarily (dpsim-models/src/EMT/EMT_DC_SSN_Inductor.cpp): Base::DC::Inductor defines mInductance and mInitialCurrent as Attribute::Ptr members, and setParameters stores into them and clone reads them back.
- PiLine::setParameters wraps plain parameters in Attributes unnecessarily (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): These parameters are Base::DC::PiLine Attribute members, and setParameters() correctly assigns through **mSeriesResistance, **mSeriesInductance, **mParallelCapacitance, **mParallelConductance, and **mInitialCurrent.
- Potential real-time violation in CurrentSource::mnaCompPreStep (dpsim-models/src/EMT/EMT_DC_CurrentSource.cpp): mnaCompPreStep dereferences mRightVector as the component-owned right-vector attribute, which is the established MNA pattern in the base class
- Potential real-time violation in CurrentSource::mnaCompPostStep (dpsim-models/src/EMT/EMT_DC_CurrentSource.cpp): mnaCompPostStep dereferences leftVector only to update interface voltage, matching the base-class post-step hook contract
- Missing finite-value guard in Capacitor::initializeFromNodesAndTerminals (dpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp): The initialization already validates the voltage with Math::isFinite(voltage) and rejects non-real values before assigning state.
Automated, non-blocking review. May be wrong. Models: find mistral-small-4-119b-2603, gpt-oss-120b → verify gpt-5.4-mini → final gpt-5.5.
| @@ -0,0 +1,111 @@ | |||
| # SSN DC Development Handover | |||
There was a problem hiding this comment.
Add SPDX header to markdown handover document
severity: high · confidence: 96%
The new markdown file docs/development/SSN_DC_SESSION_HANDOVER.md lacks the required SPDX license header. Markdown files must carry a # SPDX-FileCopyrightText and # SPDX-License-Identifier header in the file's own comment syntax.
Suggested fix: Add the following header at the very top of the file, after any shebang if present:
SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, EONERC, RWTH Aachen University
SPDX-License-Identifier: CC-BY-4.0
| # SSN DC Development Handover | |
| # SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, EONERC, RWTH Aachen University | |
| # SPDX-License-Identifier: CC-BY-4.0 | |
| # SSN DC Development Handover |
Checked against the source: the new markdown file starts with the title and contains no SPDX copyright or license header
stage: process-compliance
| Thumbs.db | ||
|
|
||
| # Docs | ||
| docs/development/ |
There was a problem hiding this comment.
Ignore docs/development/ masks new handover documentation
severity: medium · confidence: 80%
The .gitignore entry docs/development/ causes the newly added handover markdown file docs/development/SSN_DC_SESSION_HANDOVER.md to be ignored by Git, preventing it from being tracked in the repository. This results in loss of important development documentation and violates the expectation that documentation files are version-controlled.
Suggested fix: Remove the docs/development/ entry from .gitignore or adjust it to only ignore generated files, ensuring that handover and other documentation files are tracked.
| docs/development/ | |
| Delete or comment out line 68: `docs/development/` |
Checked against the source: line 68 ignores the entire docs/development/ directory with no exception for the added handover markdown path
stage: build-deps
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
6adbf0d to
aeee8db
Compare
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
There was a problem hiding this comment.
DPsim LLM review
Claim vs. code: The diff also adds an unrelated EMT::Ph3 synchronous-generator header/source wiring and a development handover doc, while the Python bindings and some claimed DC component files are not visible in the provided diff excerpt.
Found 9 high, 8 medium (5 anchored to lines below).
🔴 Critical & high
- Add SPDX license header to markdown documentation file
[high · 95% confidence]indocs/development/SSN_DC_SESSION_HANDOVER.md:1(details inline)
🟡 Suggestions
- Validation executable lacks automated regression coverage
[medium · 90% confidence]indocs/development/SSN_DC_SESSION_HANDOVER.md:89(details inline) - Register EMT_DC_SSN_Validation as an automated CTest
[medium · 88% confidence]indocs/development/SSN_DC_SESSION_HANDOVER.md:97(details inline) - Remove development handover document from tracked docs
[medium · 78% confidence]indocs/development/SSN_DC_SESSION_HANDOVER.md:1(details inline) - Use DOUBLE_EPSILON for imaginary-part tolerance check
[medium · 75% confidence]indpsim-models/src/EMT/EMT_DC_CurrentSource.cpp:50(details inline)
🔵 Optional / low-confidence (12)
- Missing Python bindings for new EMT DC components and PhaseType::DC
[high · 35% confidence · unconfirmed]indpsim/src/pybind/EMTComponents.cpp:25 - Remove non-product directory .codex/ from repository
[high · 35% confidence · unconfirmed]in.codex/ - Remove untracked AGENTS.md from repository
[high · 35% confidence · unconfirmed]inAGENTS.md - Missing Python bindings for new DC components and PhaseType::DC
[high · 35% confidence · unconfirmed]indpsim/src/pybind/EMTComponents.cpp:25 - New DC components are not bound to Python
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Components.h:114 - Missing Python bindings for newly added DC components
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Components.h:114 - New components not bound to Python
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Components.h:114 - Missing Signed-off-by DCO on a commit
[high · 35% confidence · unconfirmed]inN/A - Missing dependency declarations for PiLine composite tasks
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_PiLine.h:45 - Missing component documentation pages
[medium · 35% confidence · unconfirmed]indocs/development/SSN_DC_SESSION_HANDOVER.md:1 - Potential division by zero in companionConductance()
[medium · 30% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:24 - Potential division by zero in buildInitialInputFromNodes
[medium · 25% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalVTypeSSNComp.h:22
Claim vs. implementation
- Claimed: Add scalar DC EMT support with PhaseType::DC, DC ideal sources, DC SSN resistor/capacitor/inductor/pi-line components, initialization, and Python bindings.
- Done: Adds PhaseType::DC, new EMT::DC ideal current/voltage source classes, DC SSN resistor/capacitor/inductor/pi-line infrastructure with initialization and validation, plus related base headers, CMake/source wiring, and a new synchronous-generator header.
- Difference: The diff also adds an unrelated EMT::Ph3 synchronous-generator header/source wiring and a development handover doc, while the Python bindings and some claimed DC component files are not visible in the provided diff excerpt.
How this review was produced
13 specialized finder passes raised 87 findings over the diff and the full changed sources. After de-duplication, 86 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 69 refuted as unsupported, 17 kept (12 tentative).
Refuted by verification:
- Incomplete error message in PiLine initialization (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The throw message is complete on line 135: "DC pi-line initial node voltages must be finite real values."
- Missing imaginary-part tolerance check in PiLine initialization (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The code intentionally checks both terminal voltages for finiteness and near-zero imaginary parts before throwing.
- Capacitor state initialization inconsistent with companion form (dpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp): The file explicitly sets the capacitor state and interface voltage from the initial terminal voltage, and the base TwoTerminalITypeSSNComp/ITypeSSNComp initialization pattern handles the SSN companion state.
- Inductor state initialization inconsistent with companion form (dpsim-models/src/EMT/EMT_DC_SSN_Inductor.cpp): initializeFromNodesAndTerminals explicitly sets the SSN state and interface current from mInitialCurrent, which matches the component’s intended initial condition handling.
- PiLine validation method lacks const-correctness (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): validateDCTerminals is declared const in the file and the const_cast is used only to call node() on the current object.
- Incomplete error handling in PiLine::initializeParentFromNodesAndTerminals (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The exception string is not truncated; the full message appears on line 135.
- Missing validation of shunt capacitance and conductance in PiLine initialization (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): Initialization does not revalidate shunt attributes because setParameters already validates and stores them, and createSubComponents uses those stored values.
- PiLine virtual-node voltage initialization uses series resistor attribute directly (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The virtual-node initial voltage is computed from the stored seriesResistance and initialCurrent attributes exactly as intended for this composite.
- PiLine subcomponent pointers are not initialized in constructor (dpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_PiLine.h): The six shared_ptr members are declared in the class and, as members, are value-initialized to null; the file also shows no constructor body here to require explicit initialization.
- buildInitialInputFromNodes marked override final but not defined in base (dpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalVTypeSSNComp.h): VTypeSSNComp declares virtual MatrixComp buildInitialInputFromNodes(Real frequency) = 0, so the override exists in the base
- companionConductance marked const but not used as const in implementation (dpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h): The declaration is already
Real companionConductance() const;, so the file itself shows the const qualifier. - Use DOUBLE_EPSILON instead of hand-typed epsilon in PiLine parameter validation (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The file explicitly uses std::numeric_limits::epsilon() in setParameters; there is no DOUBLE_EPSILON usage to replace.
- Use DOUBLE_EPSILON instead of hand-typed epsilon in halfShuntResistance check (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): halfShuntResistance is checked for finiteness immediately after computation, which is a valid safeguard rather than a redundant defect.
- Incomplete throw statement in initializeParentFromNodesAndTerminals (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The throw statement is complete on the next line and does not cause a compile error.
- Use DOUBLE_EPSILON for finiteness check on interface voltage (dpsim-models/src/EMT/EMT_DC_CurrentSource.cpp): the note claims a missing tolerance guard, but
mnaCompUpdateVoltagealready checks finiteness and the issue is not present in the code.
Automated, non-blocking review. May be wrong. Models: find mistral-small-4-119b-2603, gpt-oss-120b → verify gpt-5.4-mini → final gpt-5.5.
| @@ -0,0 +1,111 @@ | |||
| # SSN DC Development Handover | |||
There was a problem hiding this comment.
Add SPDX license header to markdown documentation file
severity: high · confidence: 95%
The new markdown file docs/development/SSN_DC_SESSION_HANDOVER.md lacks the required SPDX license header. All non-source files (including documentation) must carry a valid SPDX header in the appropriate comment syntax for the file type.
Suggested fix: Add the following header at the very top of the file:
SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, EONERC, RWTH Aachen University
SPDX-License-Identifier: MPL-2.0
| # SSN DC Development Handover | |
| # SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, EONERC, RWTH Aachen University | |
| # SPDX-License-Identifier: MPL-2.0 | |
| # SSN DC Development Handover |
Checked against the source: the new markdown file starts with a title and has no SPDX copyright/license header
stage: process-compliance
| ## 9. Known problems and risks | ||
|
|
||
| - The entire implementation is uncommitted, many product files are untracked, and no complete diff review has yet been recorded; accidental omission is the highest integration risk. | ||
| - The validation is an example executable, not a registered automated unit/CTest test; regression coverage will not run automatically. |
There was a problem hiding this comment.
Validation executable lacks automated regression coverage
severity: medium · confidence: 90%
The validation executable EMT_DC_SSN_Validation is not registered as an automated unit/CTest test, so regression coverage will not run automatically. This contradicts the project's expectation that behavior-changing solver/component logic must be exercised by a registered test.
Suggested fix: Register EMT_DC_SSN_Validation as a CTest test target and ensure it is listed in the repository's test discovery mechanism. If the validation is intended to remain an example, move it to examples/Notebooks or examples/cxx and provide a separate pytest or CTest test that asserts numerical results.
Checked against the source: the file explicitly says the validation is an example executable and not a registered automated unit/CTest test
stage: tests-docs-coverage
|
|
||
| ## 10. Exact next task | ||
|
|
||
| Smallest next milestone: convert the existing scalar validation executable into an automatically registered CTest (or the repository’s directly adjacent established C++ test mechanism) without changing component equations or APIs. |
There was a problem hiding this comment.
Register EMT_DC_SSN_Validation as an automated CTest
severity: medium · confidence: 88%
The PR explicitly states that the next task is to register the existing scalar DC SSN validation executable as an automated CTest. This is not reflected in the tracked changes; the validation remains an example executable only.
Suggested fix: Create or update a CMakeLists.txt or test CMake file to register EMT_DC_SSN_Validation as a CTest test target. Ensure the test is listed in the repository's test discovery mechanism (e.g., via add_test or a test_*.yml file) so it runs under ctest.
Checked against the source: the file states the next milestone is to convert the validation executable into a registered CTest
stage: tests-docs-coverage
| @@ -0,0 +1,111 @@ | |||
| # SSN DC Development Handover | |||
There was a problem hiding this comment.
Remove development handover document from tracked docs
severity: medium · confidence: 78%
The file 'docs/development/SSN_DC_SESSION_HANDOVER.md' is a session handover document, not a product documentation page. It is not intended for the main documentation tree and should not be tracked in the repository.
Suggested fix: Move the file out of the tracked docs directory or remove it from the repository. If it is useful for internal development, consider placing it in a non-public location or under a development/ directory that is excluded from the documentation build.
Checked against the source: the new file is explicitly a session handover document rather than product documentation
stage: build-deps
| (**mIntfCurrent)(0, 0) = **mCurrentRef; | ||
| const Complex voltage = initialSingleVoltage(1) - initialSingleVoltage(0); | ||
| if (!Math::isFinite(voltage) || | ||
| std::abs(voltage.imag()) > std::numeric_limits<Real>::epsilon()) |
There was a problem hiding this comment.
Use DOUBLE_EPSILON for imaginary-part tolerance check
severity: medium · confidence: 75%
The CurrentSource::initializeFromNodesAndTerminals method compares the imaginary part of the initial voltage against std::numeric_limits::epsilon(). This should use the project-wide DOUBLE_EPSILON constant to maintain consistent numerical tolerances.
Suggested fix: Replace std::numeric_limits::epsilon() with DOUBLE_EPSILON in the imaginary-part tolerance check to align with DPsim conventions.
Checked against the source: the imaginary-part check uses std::numeric_limits::epsilon() while Definitions.h provides the project DOUBLE_EPSILON constant
stage: numerics
There was a problem hiding this comment.
DPsim LLM review
Claim vs. code: The diff does not show the claimed Python bindings, and it includes an unrelated new Ph3 synchronous generator feature not mentioned in the PR intent.
Found 15 high, 26 medium, 2 low (1 anchored to lines below).
🟡 Suggestions
- Missing Doxygen comment for public class PiLine
[medium · 88% confidence]indpsim-models/include/dpsim-models/Base/Base_DC_PiLine.h:12(details inline)
🔵 Optional / low-confidence (42)
- Incorrect companion-form state-space for DC inductor in SSN
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_SSN_Inductor.cpp:32 - Missing Scheduler::external tag for side-effecting PostStep task
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_VoltageSource.cpp:113 - Pre-step task performs vector stamping inside the hot path
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_VoltageSource.cpp:113 - Add missing virtual destructor to polymorphic base class
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Capacitor.h:14 - Add missing virtual destructor to polymorphic base class
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Inductor.h:14 - Add missing virtual destructor to polymorphic base class
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Resistor.h:14 - Add missing virtual destructor to polymorphic base class
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:17 - Missing Python bindings for new EMT DC components and PhaseType::DC
[high · 35% confidence · unconfirmed]indpsim/src/pybind/EMTComponents.cpp - Missing finiteness check on mIntfVoltage before stamp in mnaCompPreStep
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_VoltageSource.cpp:117 - Add SPDX header to docs/development directory marker
[high · 35% confidence · unconfirmed]indocs/development/:68 - Use additive RHS stamping for V‑type SSN component
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_TwoTerminalVTypeSSNComp.cpp:30 - Missing Python binding for new DC components
[high · 35% confidence · unconfirmed]indpsim/src/pybind/EMTComponents.cpp - Missing Python enum binding for PhaseType::DC
[high · 35% confidence · unconfirmed]indpsim/src/pybind/Definitions.cpp - Missing Developer Certificate of Origin sign‑off
[high · 35% confidence · unconfirmed]ingeneral - Missing per-unit normalization in SSN_SynchronousGenerator state-space matrices
[high · 28% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_Ph3_SSN_SynchronousGenerator.h:114 - Initialization of DC voltage source interface voltage without solving MNA
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_VoltageSource.cpp:50 - Missing override specifier for mnaCompApplySystemMatrixStamp
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:28 - Inconsistent companion conductance matrix construction
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_SSN_Inductor.cpp:33 - Missing finiteness check on mW(0,0) before stamping
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_TwoTerminalVTypeSSNComp.cpp:43 - Inconsistent dependency declaration in mnaCompAddPreStepDependencies
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_VoltageSource.cpp:103 - MNA hook overrides should use override specifier
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Capacitor.h:25 - MNA hook overrides should use override specifier
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Inductor.h:25 - MNA hook overrides should use override specifier
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Resistor.h:25 - MNA hook overrides should use override specifier
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:22 - Use SPDLOG_LOGGER_DEBUG for non-critical simulation-time logging
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_VoltageSource.cpp:106 - Use SPDLOG_LOGGER_DEBUG for non-critical simulation-time logging
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_TwoTerminalVTypeSSNComp.cpp:63 - Add missing SPDLOG_LOGGER_ logging to validateDCTerminals*
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_PiLine.h:63 - Add missing SPDLOG_LOGGER_ logging to validateDCTerminals*
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_TwoTerminalVTypeSSNComp.cpp:29 - Use defined DOUBLE_EPSILON for voltage imaginary part check
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_TwoTerminalVTypeSSNComp.cpp:22 - Use defined DOUBLE_EPSILON for capacitance positivity check
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp:21 - Use defined DOUBLE_EPSILON for inductance positivity check
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_SSN_Inductor.cpp:21 - Missing Doxygen comment for public class Resistor
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Resistor.h:14 - Missing Doxygen comment for public class Capacitor
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Capacitor.h:14 - Missing Doxygen comment for public class Inductor
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Inductor.h:14 - Potential division by zero in companionConductance
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:24 - Missing attribute dependency registration for PiLine parameters
[medium · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_PiLine.cpp:63 - Missing documentation for new DC components
[medium · 35% confidence · unconfirmed]indocs/hugo/content/en/docs/Models/Concepts - Missing Python bindings for new DC components
[medium · 35% confidence · unconfirmed]indpsim/src/pybind/EMTComponents.cpp - Missing test coverage for new DC components
[medium · 35% confidence · unconfirmed]intests - PR mixes unrelated large feature (synchronous generator) with DC component work
[medium · 35% confidence · unconfirmed]ingeneral - Remove editor-specific ignore pattern
[low · 35% confidence · unconfirmed]in.gitignore:52 - Unnecessary .codex entry in .gitignore
[low · 35% confidence · unconfirmed]in.gitignore:52
Claim vs. implementation
- Claimed: Add initial EMT scalar DC network support with PhaseType::DC, DC sources, DC SSN components, initialization, and Python bindings.
- Done: Adds new EMT::DC scalar DC source and SSN component classes plus PhaseType::DC and related model headers/sources, and also adds a new Ph3 synchronous generator implementation.
- Difference: The diff does not show the claimed Python bindings, and it includes an unrelated new Ph3 synchronous generator feature not mentioned in the PR intent.
How this review was produced
13 specialized finder passes raised 141 findings over the diff and the full changed sources. After de-duplication, 141 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 98 refuted as unsupported, 43 kept (42 tentative).
Refuted by verification:
- Incorrect state-vector dimension in SSN_SynchronousGenerator documentation (dpsim-models/include/dpsim-models/EMT/EMT_Ph3_SSN_SynchronousGenerator.h): The state vector comment and StateIndex enum both list 8 states, with StateCount as the 8th enumerator.
- Incorrect companion-form state-space for DC capacitor in SSN (dpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp): The file explicitly sets x = vC and passes A=0, B=1/C, C=I, D=0 to SSNComp::setParameters, matching the stated voltage-state convention rather than a missing or incorrect implementation.
- Incorrect virtual node voltage initialization for DC PiLine (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The virtual node is intentionally initialized from the series resistor only, and the class already stores the inductor initial current via mSeriesInductor->setParameters(**mSeriesInductance, **mInitialCurrent).
- Missing documentation of DC PiLine assumptions and limitations (dpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_PiLine.h): The class comment already states it is a "Scalar DC lumped pi-line" and describes the series/shunt lumped topology.
- Missing override specifier for mnaCompApplySystemMatrixStamp (dpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalVTypeSSNComp.h): The method is declared with
override finalon line 27, so the missing-override claim is contradicted by the file. - Incorrect shunt conductance calculation in PiLine (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): For a total shunt conductance G, each half branch uses R = 2.0 / G, which is the correct resistance split for two equal parallel resistors to ground.
- Incorrect shunt capacitance calculation in PiLine (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The capacitance split is already correct: halfCapacitance = **mParallelCapacitance / 2.0.
- Typo in task order constant name (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The code uses the exact enum constant MNA_SUBCOMP_TASK_ORDER::TASK_BEFORE_PARENT, not a misspelled name.
- Typo in task order constant name (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The code uses the exact enum constant MNA_SUBCOMP_TASK_ORDER::TASK_BEFORE_PARENT, not a misspelled name.
- Typo in task order constant name (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The code uses the exact enum constant MNA_SUBCOMP_TASK_ORDER::TASK_BEFORE_PARENT, not a misspelled name.
- Typo in task order constant name (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The code uses the exact enum constant MNA_SUBCOMP_TASK_ORDER::TASK_BEFORE_PARENT, not a misspelled name.
- Typo in task order constant name (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The code uses the exact enum constant MNA_SUBCOMP_TASK_ORDER::TASK_BEFORE_PARENT, not a misspelled name.
- Typo in task order constant name (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The code uses the exact enum constant MNA_SUBCOMP_TASK_ORDER::TASK_BEFORE_PARENT, not a misspelled name.
- Use DOUBLE_EPSILON instead of std::numeric_limits::epsilon() for near-zero checks (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The file does perform near-zero validation, but the claim about DOUBLE_EPSILON is a style preference; the code explicitly uses std::numeric_limits::epsilon().
- Potential division by near-zero when computing halfShuntResistance (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): The reciprocal is guarded by a finite check immediately after computation, and the conductance itself is validated as positive in setParameters().
Automated, non-blocking review. May be wrong. Models: find mistral-small-4-119b-2603, gpt-oss-120b → verify gpt-5.4-mini → final gpt-5.5.
| namespace Base { | ||
| namespace DC { | ||
|
|
||
| class PiLine { |
There was a problem hiding this comment.
Missing Doxygen comment for public class PiLine
severity: medium · confidence: 88%
The public class PiLine in the Base DC namespace does not have an explanatory Doxygen comment, contrary to DPsim documentation standards.
Suggested fix: Add a Doxygen comment explaining that this class models a scalar DC pi‑line with series R/L and shunt C/G components.
| class PiLine { | |
| /// Basic DC pi‑line model. | |
| /// Represents a scalar DC line with series resistance/inductance and optional shunt capacitance/conductance. | |
| class PiLine { |
Checked against the source: public class PiLine is introduced without any Doxygen comment
stage: naming-docs
There was a problem hiding this comment.
DPsim LLM review
Claim vs. code: The diff does not show the claimed Python bindings or notebook example additions, and it also includes an unrelated new three-phase synchronous generator implementation not mentioned in the DC-focused summary.
Found 1 critical, 21 high, 12 medium, 2 low (4 anchored to lines below).
🔴 Critical & high
- Use DOUBLE_EPSILON in companionConductance() instead of std::numeric_limits::epsilon()
[high · 78% confidence]indpsim-models/src/EMT/EMT_DC_TwoTerminalITypeSSNComp.cpp:27(details inline) - Guard division by companionConductance() in mnaCompApplyRightSideVectorStamp
[high · 65% confidence]indpsim-models/src/EMT/EMT_DC_TwoTerminalITypeSSNComp.cpp:53(details inline) - Guard division by companionConductance() in mnaCompUpdateCurrent
[high · 55% confidence]indpsim-models/src/EMT/EMT_DC_TwoTerminalITypeSSNComp.cpp:61(details inline)
🟡 Suggestions
- Missing blank line before EMT_DC sources list
[medium · 65% confidence]indpsim-models/src/CMakeLists.txt:92
CMake style in this file uses a blank line to separate logical blocks of sources. The newly added EMT_DC sources (lines 92–99) are not preceded by a blank line, breaking the visual grouping.
Fix: Insert a blank line before line 92 to restore the block separation style used throughout the file. - Repeated const_cast in terminal validation
[low · 70% confidence]indpsim-models/src/EMT/EMT_DC_TwoTerminalITypeSSNComp.cpp:18(details inline)
🔵 Optional / low-confidence (31)
- PiLine shunt capacitors will trigger singular companion impedance error
[critical · 28% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_PiLine.h:60 - Incorrect companion conductance calculation in ITypeSSN DC component
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_TwoTerminalITypeSSNComp.cpp:30 - Incorrect state-derivative evaluation for sixth-order synchronous machine
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_Ph3_SSN_SynchronousGenerator.cpp:114 - Incorrect output evaluation for sixth-order synchronous machine
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_Ph3_SSN_SynchronousGenerator.cpp:116 - Missing mnaCompApplySystemMatrixStamp implementation
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_CurrentSource.h:35 - Missing Scheduler::external tag for side-effecting PreStep task
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_CurrentSource.cpp:109 - PostStep dereferences leftVector without validation
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_VoltageSource.cpp:118 - SSN_SynchronousGenerator uses Eigen::PartialPivLU without pivot checks
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_Ph3_SSN_SynchronousGenerator.cpp:200 - Add missing override specifier to MNASimPowerComp base class
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_CurrentSource.h:17 - Add missing override specifier to base classes
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Inductor.h:14 - Add missing override specifier to base classes
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Resistor.h:14 - Add missing override specifier to base class
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalVTypeSSNComp.h:17 - Add Python bindings for new EMT DC components and PhaseType::DC
[high · 35% confidence · unconfirmed]indpsim/src/pybind/EMTComponents.cpp:25 - Missing Scheduler::external tag for side-effecting DC current-source task
[high · 35% confidence · unconfirmed]indpsim-models/src/EMT/EMT_DC_CurrentSource.cpp:109 - PhaseType::DC not handled in node indexing and MNA utilities
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Definitions.h:113 - Missing handling of new PhaseType::DC in existing switch statements
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Definitions.h:113 - Missing Python binding for new DC components
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Components.h:113 - Missing Python enum binding for PhaseType::DC
[high · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Definitions.h:113 - Missing Developer Certificate of Origin sign‑off
[high · 35% confidence · unconfirmed]ingeneral - Add documentation for EMT::DC components
[medium · 35% confidence · unconfirmed]indocs/hugo/content/en/docs/Models/Components.md - Add numerical assertions to DC_PiLine notebook
[medium · 35% confidence · unconfirmed]inexamples/Notebooks/DC_PiLine.ipynb - Clear notebook outputs before commit
[medium · 35% confidence · unconfirmed]inexamples/Notebooks/DC_PiLine.ipynb - Potential missing factory registration for new DC components
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Components.h:114 - Add Doxygen comment for public class
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Inductor.h:14 - Add Doxygen comment for public class
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_SSN_Resistor.h:14 - Missing documentation for new DC components
[medium · 35% confidence · unconfirmed]indocs/hugo/content/en/docs/Models/Concepts - No tests for newly added DC components
[medium · 35% confidence · unconfirmed]intests - Unclear purpose of .codex ignore entry
[medium · 35% confidence · unconfirmed]in.gitignore:52 - PR scope includes unrelated synchronous‑generator component
[medium · 35% confidence · unconfirmed]ingeneral - Remove unnecessary development directory entry from gitignore
[medium · 30% confidence · unconfirmed]in.gitignore:68 - Duplicate DC terminal validation logic
[low · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/EMT/EMT_DC_TwoTerminalITypeSSNComp.h:23
Claim vs. implementation
- Claimed: Add scalar DC EMT support with PhaseType::DC, EMT::DC ideal sources, EMT::DC::SSN RLC/PiLine components, initialization support, and Python bindings.
- Done: Adds PhaseType::DC plus new EMT::DC source and SSN component classes with DC terminal validation, scalar MNA/SSN behavior, initial-condition handling, and related build/header wiring.
- Difference: The diff does not show the claimed Python bindings or notebook example additions, and it also includes an unrelated new three-phase synchronous generator implementation not mentioned in the DC-focused summary.
How this review was produced
13 specialized finder passes raised 118 findings over the diff and the full changed sources. After de-duplication, 118 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 82 refuted as unsupported, 36 kept (31 tentative).
Refuted by verification:
- Incorrect companion-form companion matrix for DC inductor (dpsim-models/src/EMT/EMT_DC_SSN_Inductor.cpp): setParameters() intentionally passes 1.0 / inductance to SSNComp::setParameters, matching the x = iL, u = v, y = iL comment and the V-type SSN pattern.
- Incorrect companion-form companion matrix for DC capacitor (dpsim-models/src/EMT/EMT_DC_SSN_Capacitor.cpp): The file explicitly sets the capacitor SSN matrices with B = 1.0 / capacitance at lines 27-30, matching the stated x = vC, u = i, y = vC model.
- Incorrect virtual-node voltage initialization for DC PiLine (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): initializeParentFromNodesAndTerminals intentionally sets the virtual node to voltage0.real() + **mSeriesResistance * **mInitialCurrent, and the file contains no shunt-capacitor initialization hook to contradict that pattern
- Missing mnaCompApplySystemMatrixStamp implementation (dpsim-models/include/dpsim-models/EMT/EMT_DC_VoltageSource.h): The class explicitly declares
mnaCompApplySystemMatrixStamp(SparseMatrixRow &systemMatrix) override final;on line 36, so the implementation is not missing from the header. - Incorrect mnaCompInitialize signature (dpsim-models/src/EMT/EMT_DC_VoltageSource.cpp): The implementation matches the declared override signature exactly:
mnaCompInitialize(Real, Real, Attribute<Matrix>::Ptr). - Incorrect mnaCompPreStep signature (dpsim-models/src/EMT/EMT_DC_VoltageSource.cpp): The implementation matches the base signature exactly:
mnaCompPreStep(Real, Int). - Incorrect mnaCompPostStep signature (dpsim-models/src/EMT/EMT_DC_VoltageSource.cpp): The implementation matches the base signature exactly:
mnaCompPostStep(Real, Int, Attribute<Matrix>::Ptr &). - Missing validation of shunt conductance sign (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): setParameters already validates parallelConductance with a finite/nonnegative check and a near-zero guard at lines 42-45
- Incorrect virtual node voltage initialization (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): the implementation uses v0 + R*I_init exactly, and the cited comment text does not exist in this file
- Use DOUBLE_EPSILON instead of std::numeric_limits::epsilon() for near-zero checks (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): using std::numeric_limits::epsilon() here is the actual code in setParameters, so the claim that it is absent is false
- Validate parallelCapacitance/2.0 against DOUBLE_EPSILON, not 0.0 (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): the check is present exactly as described in the file, and the complaint is about style/numerics rather than a missing validation
- Validate parallelConductance against DOUBLE_EPSILON, not 1e-12 (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): parallelConductance is validated against std::numeric_limits::epsilon() in the file, so the finding’s premise of a missing check is false
- Guard halfShuntResistance finiteness before use (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): halfShuntResistance is computed only inside the parallelConductance > 0.0 branch and then checked with Math::isFinite immediately after
- PiLine::mnaCompUpdateCurrent should validate subcomponent intfCurrent finiteness before assignment (dpsim-models/src/EMT/EMT_DC_SSN_PiLine.cpp): mnaCompUpdateCurrent already checks (**mIntfCurrent).allFinite() before returning
- Missing Scheduler::external tag for side-effecting PreStep task (dpsim-models/src/EMT/EMT_DC_VoltageSource.cpp): The pre-step dependency hook already adds
mRightVectorandmIntfVoltage, which is the scheduler exposure mechanism for this task.
Automated, non-blocking review. May be wrong. Models: find mistral-small-4-119b-2603, gpt-oss-120b → verify gpt-5.4-mini → final gpt-5.5.
|
|
||
| Real EMT::DC::TwoTerminalITypeSSNComp::companionConductance() const { | ||
| if (mW.rows() != 1 || mW.cols() != 1 || !Math::isFinite(mW(0, 0)) || | ||
| std::abs(mW(0, 0)) <= std::numeric_limits<Real>::epsilon()) |
There was a problem hiding this comment.
Use DOUBLE_EPSILON in companionConductance() instead of std::numeric_limits::epsilon()
severity: high · confidence: 78%
The companionConductance() method compares mW(0,0) against std::numeric_limits::epsilon(), which is inconsistent with DPsim conventions and numerically unsafe for power-system companion models. The shared constant DOUBLE_EPSILON should be used for all near-zero comparisons to ensure consistent behavior across domains and solvers.
Suggested fix: Replace std::numeric_limits::epsilon() with DOUBLE_EPSILON in the singularity check within companionConductance().
Checked against the source: companionConductance compares against std::numeric_limits::epsilon() even though DOUBLE_EPSILON is defined in Definitions.h
stage: numerics
|
|
||
| void EMT::DC::TwoTerminalITypeSSNComp::mnaCompApplyRightSideVectorStamp( | ||
| Matrix &rightVector) { | ||
| const Real historyCurrent = -companionConductance() * mYHist(0, 0); |
There was a problem hiding this comment.
Guard division by companionConductance() in mnaCompApplyRightSideVectorStamp
severity: high · confidence: 65%
The method mnaCompApplyRightSideVectorStamp computes historyCurrent = -companionConductance() * mYHist(0,0). If companionConductance() throws due to a singular mW, the stamp is not applied, which is correct. However, if mYHist(0,0) is inf or nan, the result can poison the right-hand side. The code should guard mYHist(0,0) for finiteness before the multiplication.
Suggested fix: Add a Math::isFinite(mYHist(0,0)) check before computing historyCurrent. If not finite, throw a runtime_error explaining the cause.
Checked against the source: mYHist(0,0) is multiplied into the RHS history current without any local finiteness check
stage: numerics
| } | ||
|
|
||
| void EMT::DC::TwoTerminalITypeSSNComp::mnaCompUpdateCurrent(const Matrix &) { | ||
| (**mIntfCurrent)(0, 0) = |
There was a problem hiding this comment.
Guard division by companionConductance() in mnaCompUpdateCurrent
severity: high · confidence: 55%
The method mnaCompUpdateCurrent computes (**mIntfCurrent)(0,0) = companionConductance() * ((**mIntfVoltage)(0,0) - mYHist(0,0)). If companionConductance() throws due to singularity, the update is skipped, which is acceptable. However, if (**mIntfVoltage)(0,0) - mYHist(0,0) is inf or nan, the result can poison the interface current. The code should guard the difference for finiteness before the multiplication.
Suggested fix: Add a Math::isFinite((**mIntfVoltage)(0,0) - mYHist(0,0)) check before the multiplication. If not finite, throw a runtime_error explaining the cause.
Checked against the source: the current expression is assigned before the later finiteness check, so the operand difference is not guarded before multiplication
stage: numerics
| void EMT::DC::TwoTerminalITypeSSNComp::validateDCTerminals() const { | ||
| for (UInt terminalIdx = 0; terminalIdx < 2; ++terminalIdx) { | ||
| const auto terminalNode = | ||
| const_cast<TwoTerminalITypeSSNComp *>(this)->node(terminalIdx); |
There was a problem hiding this comment.
Repeated const_cast in terminal validation
severity: low · confidence: 70%
validateDCTerminals() uses const_cast to obtain a non‑const node pointer from a const method, which is unnecessary because the node pointer itself does not need to be modified. This introduces a minor readability and safety concern.
Suggested fix: Change validateDCTerminals() to accept a const SimNode* and avoid const_cast.
Checked against the source: validateDCTerminals uses const_cast<TwoTerminalITypeSSNComp *>(this)->node(terminalIdx) inside a const method
stage: realtime-resources
|
|
Hi Matthias! Some small requests before checking with more detail:
And then a more general question about what difference would we have/what we gain on top of EMT::Ph1 if we do things the way you propose here Would also be good to know what does @georgii-tishenin thinks about it |



Summary
This PR introduces initial support for scalar DC networks in the DPsim EMT
domain.
The implementation allows DC nodes and basic DC components to be used directly
inside EMT simulations without representing the DC system as an artificial
single-phase AC network and without requiring a separate DC power-flow solver.
The main objective is to provide a clean foundation for future mixed AC/DC
models, including EMT::Ph3 converter models connected to an explicitly modeled
DC grid.
Main changes
Scalar DC phase type
PhaseType::DC.existing EMT node and MNA infrastructure.
dpsimpy.PhaseType.DC.DC component namespace
Added the following ideal source components under
EMT::DC:EMT::DC::VoltageSourceEMT::DC::CurrentSourceThe components use scalar DC voltages and currents and follow the passive sign
convention used by the existing DPsim component interfaces.
DC SSN components
Added the following State-Space Nodal components under
EMT::DC::SSN:ResistorCapacitorInductorPiLineThe components are integrated into the existing EMT MNA/SSN solution process.
No additional DC solver is introduced.
The DC Pi-line contains:
Initialization
The dynamic components support explicit initial conditions, including:
This allows DC systems to start from a calculated steady-state operating point
instead of producing an artificial energization transient from zero initial
conditions.
Python bindings
Added pybind bindings for: