Skip to content

Fix component initialization in pi lines, RX loads - #697

Merged
leonardocarreras merged 6 commits into
sogno-platform:masterfrom
leonardocarreras:fix/component-init-seeds
Sep 3, 2026
Merged

Fix component initialization in pi lines, RX loads#697
leonardocarreras merged 6 commits into
sogno-platform:masterfrom
leonardocarreras:fix/component-init-seeds

Conversation

@leonardocarreras

@leonardocarreras leonardocarreras commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Unrelated in relation to components, but all related to initialization.

  1. EMT::Ph1::PiLine seeded its virtual node with a peak single-phase term added onto an RMS line-line voltage, phase discarded.
  2. EMT::Ph3::RXLoad hardcoded Logger::Level::trace, so impossible to turn logging off.
  3. It computed its impedances twice, the copy leaving a 1x1 matrix that series mode read out of bounds.
  4. Its series-mode virtual node seed had the same unit error, and the sign wrong on top.
  5. DP::Ph1::RXLoad divided by an unchecked terminal voltage, and clone() marked unparameterized copies as parameterized.
  6. SystemTopology::initWithPowerflow() dereferenced a null generator lookup.

For some fixes related to RX loads, thanks to Matthias.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DPsim LLM review

Claim vs. code: matches the description.

TL;DR: One high-confidence bug remains in EMT Ph1 PiLine where initialization can divide by zero, while the transformer and RXLoad changes are mostly covered by lower-confidence test/documentation gaps and one unconfirmed DP RXLoad equation concern; the scheduler and powerflow guard changes did not surface additional confirmed correctness issues.

Found 1 high, 1 medium (2 anchored to lines below).

🔴 Critical & high

  • Division by possibly zero impedance [high · 92% confidence] in dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp:105 (details inline)

🟡 Suggestions

  • Missing snubber capacitance initialization guard in EMT Ph3 Transformer [medium · 80% confidence] in dpsim-models/src/EMT/EMT_Ph3_Transformer.cpp:175 (details inline)

Not shown: 15 tentative. All are in the workflow log.

Claim vs. implementation
  • Claimed: Fix initialization issues in EMT pi lines, EMT/DP RX loads, transformers, and powerflow generator lookup.
  • Done: Adds guards and corrected seeding for EMT Ph1 pi line and EMT Ph3 RX load initialization, fixes DP Ph1 RX load clone/current handling, skips transformer snubbers when rated power is non-positive, and null-checks generator lookup in initWithPowerflow.
  • Difference: none
How this review was produced

13 specialized finder passes raised 91 findings over the diff and the full changed sources. After de-duplication, 91 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 74 refuted as unsupported, 17 kept (15 tentative).

Refuted by verification:

  • Incorrect virtual node initialization in EMT Ph1 PiLine (dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp): The file already seeds the virtual node with initialSingleVoltage(0) + PEAK1PH_TO_RMS3PH * current * **mSeriesRes, and current is derived from the peak line-to-line voltage at line 105.
  • Incorrect impedance matrix inversion in EMT Ph3 RXLoad initialization (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): The code explicitly builds a complex 3x3 impedance from R and X and inverts it as impedance.inverse() * vInitABC.
  • Incorrect virtual node initialization in EMT Ph3 RXLoad series mode (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): The file uses PEAK1PH_TO_RMS3PH * (vInitTerm0 - mResistance * iInit) exactly as described, and the finding itself says this sign is correct.
  • Incorrect conjugate power flow sign in DP Ph1 RXLoad initialization (dpsim-models/src/DP/DP_Ph1_RXLoad.cpp): The code uses std::conj(Complex(**mActivePower, **mReactivePower) / (**mIntfVoltage)(0, 0)), which is the passive-sign-convention form for load current initialization.
  • Incorrect unit conversion for EMT Ph1 PiLine virtual node seed (dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp): The claimed mixed-unit seed is exactly the changed line, and the code intentionally converts the peak voltage drop back to RMS3PH before adding it to initialSingleVoltage(0).
  • ... and 10 more, in the workflow log.

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.

Comment thread dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp
Comment thread dpsim-models/src/EMT/EMT_Ph3_Transformer.cpp Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DPsim LLM review

Claim vs. code: matches the description.

Found 1 critical, 2 high (3 anchored to lines below).

🔴 Critical & high

  • Division by possibly zero impedance [critical · 90% confidence] in dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp:105 (details inline)
  • Use DOUBLE_EPSILON for near-zero voltage guard in DP_Ph1_RXLoad [high · 90% confidence] in dpsim-models/src/DP/DP_Ph1_RXLoad.cpp:90 (details inline)
  • Use tolerance instead of zero check for voltage division [high · 88% confidence] in dpsim-models/src/DP/DP_Ph1_RXLoad.cpp:90 (details inline)

Not shown: 40 tentative. All are in the workflow log.

Claim vs. implementation
  • Claimed: Fix initialization bugs in pi lines, RX loads, transformers, and powerflow generator lookup.
  • Done: Adds guards and corrected seeding/initialization for EMT Ph1 pi lines, EMT Ph3 and DP Ph1 RX loads, DP/EMT transformers, and null-safe generator lookup in powerflow init.
  • Difference: none
How this review was produced

13 specialized finder passes raised 117 findings over the diff and the full changed sources. After de-duplication, 117 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 74 refuted as unsupported, 43 kept (40 tentative).

Refuted by verification:

  • Incorrect per-phase impedance scaling in EMT Ph3 RXLoad (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): The code intentionally uses per-phase values from the diagonal matrices and the diff only changed the zero-case handling, not the impedance formula.
  • Incorrect virtual-node voltage seed in EMT Ph3 RXLoad series mode (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): The series-mode seed line is present as the intended implementation, and the surrounding code already converts terminal voltage to peak units before applying the virtual-node seed.
  • Incorrect matrix dimension for zero reactance in EMT Ph3 RXLoad (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): mReactance is initialized to Matrix::Zero(3, 3), not a 1x1 matrix, so the claimed dimension mismatch is absent.
  • Incorrect sign in virtual node voltage seed for EMT Ph3 RXLoad series mode (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): The minus sign is the actual code and matches the PR change from the previous plus-sign formulation.
  • Missing snubber capacitance initialization guard for mSubSnubCapacitor1 (dpsim-models/src/EMT/EMT_Ph3_Transformer.cpp): The code already guards the capacitor setup with if (mSubSnubCapacitor2) before calling setParameters.
  • ... and 10 more, in the workflow log.

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.

Comment thread dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp
Comment thread dpsim-models/src/DP/DP_Ph1_RXLoad.cpp
Comment thread dpsim-models/src/DP/DP_Ph1_RXLoad.cpp
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.75%. Comparing base (584df01) to head (b14aaa8).

Files with missing lines Patch % Lines
dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp 14.28% 6 Missing ⚠️
dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp 0.00% 4 Missing ⚠️
dpsim-models/src/DP/DP_Ph1_RXLoad.cpp 66.66% 2 Missing ⚠️
dpsim-models/src/SystemTopology.cpp 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #697      +/-   ##
==========================================
- Coverage   64.94%   64.75%   -0.20%     
==========================================
  Files         533      533              
  Lines       37480    37485       +5     
  Branches    19933    19935       +2     
==========================================
- Hits        24342    24274      -68     
- Misses      13090    13125      +35     
- Partials       48       86      +38     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leonardocarreras leonardocarreras changed the title Fix component initialization in pi lines, RX loads and transformers Fix component initialization in pi lines, RX loads Sep 3, 2026
@leonardocarreras
leonardocarreras marked this pull request as ready for review September 3, 2026 11:29
@leonardocarreras leonardocarreras self-assigned this Sep 3, 2026
@leonardocarreras leonardocarreras added the bug Something isn't working label Sep 3, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DPsim LLM review

Claim vs. code: The diff also removes a duplicated EMT Ph3 RX-load impedance computation and changes its series-mode virtual-node seed formula/sign, which are not separately mentioned in the title and are only partially covered by the description.

TL;DR: Four real correctness issues remain: singular-matrix/zero-impedance initialization paths in EMT Ph3 RXLoad and EMT Ph1 PiLine, plus the DP Ph1 RXLoad divide-by-zero/clone behavior; the rest are lower-value documentation/test coverage gaps or duplicate requests, and the topology null-lookup fix appears unflagged here.

Found 2 high (2 anchored to lines below).

🔴 Critical & high

  • Missing guard for singular impedance matrix inversion [high · 94% confidence] in dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp:199 (details inline)
  • Guard matrix inversion for resistance calculation [high · 90% confidence] in dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp:199 (details inline)

Not shown: 9 tentative. All are in the workflow log.

Claim vs. implementation
  • Claimed: Fix initialization bugs in EMT pi lines and RX loads, plus DP RX load and powerflow generator lookup guards.
  • Done: Updates EMT Ph1 pi-line and EMT Ph3 RX-load virtual-node/current initialization, honors RX-load log level, guards DP Ph1 RX-load clone/current initialization, and skips missing generator-backed components in powerflow init.
  • Difference: The diff also removes a duplicated EMT Ph3 RX-load impedance computation and changes its series-mode virtual-node seed formula/sign, which are not separately mentioned in the title and are only partially covered by the description.
How this review was produced

13 specialized finder passes raised 75 findings over the diff and the full changed sources. After de-duplication, 75 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 61 refuted as unsupported, 14 kept (11 tentative).

Refuted by verification:

  • Incorrect complex power sign convention in DP Ph1 RXLoad initialization (dpsim-models/src/DP/DP_Ph1_RXLoad.cpp): The current formula is exactly the existing initialization code, and the file already guards zero voltage before the division.
  • Incorrect virtual-node voltage seed in EMT Ph1 PiLine (dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp): The seed uses PEAK1PH_TO_RMS3PH * current * **mSeriesRes, and the file’s own initialSingleVoltage()/voltage setup shows the initialization is intentionally converted before assignment.
  • Fix clone to respect parameterization state (dpsim-models/src/DP/DP_Ph1_RXLoad.cpp): The clone method now explicitly guards setParameters with if (mParametersSet) on the changed line.
  • Correct virtual node initialization units in EMT Ph1 PiLine (dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp): The code does not convert the current term back to RMS3PH twice; it computes current = voltage / impedance once and applies a single PEAK1PH_TO_RMS3PH factor at line 111.
  • Honour logLevel argument in EMT Ph3 RXLoad constructor (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): The constructor now forwards the provided logLevel to CompositePowerComp(..., logLevel).
  • ... and 10 more, in the workflow log.

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.

Comment thread dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp
Comment thread dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp
leonardocarreras and others added 6 commits September 3, 2026 19:58
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Signed-off-by: matthiasmees <matthias.mees@rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DPsim LLM review

Claim vs. code: matches the description.

TL;DR: Two high-severity numerical robustness issues remain in EMT::Ph1::PiLine and EMT::Ph3::RXLoad, while the rest are lower-confidence documentation/test coverage gaps around the initialization fixes; the DP RXLoad and SystemTopology changes did not surface a confirmed correctness bug here.

Found 2 high, 1 medium (1 anchored to lines below).

🔴 Critical & high

  • Potential division by zero when series impedance is zero [high · 92% confidence] in dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp:105 (details inline)
  • Potential singular matrix inversion for active power [high · 90% confidence] in dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp:130
    The code computes mResistance = pow(**mNomVoltage / sqrt(3), 2) * (**mActivePower).inverse(); after only checking that (**mActivePower)(0,0) != 0. If any other diagonal element of the 3×3 active power matrix is zero (or the matrix is otherwise singular), Matrix::inverse() will throw or produce NaNs, leading to a crash or invalid impedance values during simulation.
    Fix: Guard the inversion by checking that the entire active-power matrix is invertible (e.g. using Eigen::FullPivLU to test rank) or compute the impedance per-phase using only the diagonal entries. If the matrix is singular, set the corresponding resistance entries to zero or raise a clear error.

🟡 Suggestions

  • Near-zero power matrix check uses single element comparison [medium · 90% confidence] in dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp:130
    Impedance and resistance calculations are guarded only by checking ( **mActivePower )(0,0) != 0. If other diagonal entries are zero or the matrix is singular, the subsequent inverse() call can produce NaNs or throw, corrupting the admittance matrix.
    Fix: Validate the entire active-power matrix (e.g., check its determinant or that all diagonal entries exceed a tolerance) before calling inverse(). If the matrix is singular, set resistance to a zero matrix and log a warning.

Not shown: 9 tentative. All are in the workflow log.

Claim vs. implementation
  • Claimed: Fix initialization bugs in EMT pi lines and RX loads, plus DP RX load and powerflow generator lookup guards.
  • Done: Updates EMT Ph1 pi-line and EMT Ph3 RX-load initialization math, honors RX-load log level, fixes DP Ph1 RX-load clone/current initialization, and skips missing generator components in powerflow init.
  • Difference: none
How this review was produced

13 specialized finder passes raised 102 findings over the diff and the full changed sources. After de-duplication, 102 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 89 refuted as unsupported, 13 kept (10 tentative).

Refuted by verification:

  • Incorrect complex power sign convention in DP Ph1 RXLoad initialization (dpsim-models/src/DP/DP_Ph1_RXLoad.cpp): The file intentionally initializes current as std::conj(Complex(P,Q)/V) on line 91, matching the existing load initialization pattern rather than a missing or absent implementation.
  • Inconsistent zeroing of mReactance matrix in EMT Ph3 RXLoad (dpsim-models/src/EMT/EMT_Ph3_RXLoad.cpp): mReactance is zeroed in the else branch at line 138, so it is not left stale when reactive power is zero.
  • Incorrect scaling of initial current in DP Ph1 RXLoad (dpsim-models/src/DP/DP_Ph1_RXLoad.cpp): The expression on lines 89-93 is a direct complex-power-to-current initialization; no extra RMS scaling factor is present or implied in the code.
  • Missing null check before dereferencing generator component in initWithPowerflow (dpsim-models/src/SystemTopology.cpp): The added null check is on the looked-up component comp before comp->terminals()[0]; the cast to SynchronGenerator is already guarded by if (auto genPF = std::dynamic_pointer_cast<...>(compPF)) on lines 104-105.
  • Replace ad-hoc zero check with DOUBLE_EPSILON guard (dpsim-models/src/DP/DP_Ph1_RXLoad.cpp): The changed guard explicitly uses std::abs((**mIntfVoltage)(0, 0)) > 0 on line 90, so this is not an ad-hoc literal zero check claim against the current file.
  • ... and 10 more, in the workflow log.

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.

Comment thread dpsim-models/src/EMT/EMT_Ph1_PiLine.cpp
@leonardocarreras
leonardocarreras merged commit 8aac114 into sogno-platform:master Sep 3, 2026
50 of 52 checks passed
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants