PFSolver: SVC voltage-control outer loop

[leonardocarreras]

Adds the solver-side outer loop that holds each SVC bus at its voltage setpoint. Opt in via set_pf_solver_enforce_svc_control.

Needs #580

[codecov]

Codecov Report

❌ Patch coverage is 87.17949% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.51%. Comparing base (5780c9f) to head (a3cddc2).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
dpsim-models/src/SP/SP_Ph1_SVC.cpp	75.40%	15 Missing ⚠️
dpsim/src/Simulation.cpp	71.42%	2 Missing ⚠️
dpsim/include/dpsim/PFSolver.h	50.00%	1 Missing ⚠️
dpsim/src/PFSolverPowerPolar.cpp	98.07%	1 Missing ⚠️
dpsim/src/pybind/SPComponents.cpp	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #582      +/-   ##
==========================================
+ Coverage   72.14%   72.51%   +0.36%     
==========================================
  Files         491      493       +2     
  Lines       31623    31775     +152     
  Branches    16950    17030      +80     
==========================================
+ Hits        22815    23042     +227     
- Misses       8722     8732      +10     
+ Partials       86        1      -85     

☔ 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.

[github-actions]

DPsim LLM review

Claim vs. code: matches the description.

TL;DR: Two real robustness fixes stand out: the new SVC model can divide by an unset base voltage and can propagate non-finite reactive power into solver state; the remaining items are lower-severity convention, logging, and documentation/test-coverage gaps, and the solver loop itself did not surface additional correctness issues.

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

🔴 Critical & high

• Guard against NaN/Inf propagation in updateReactivePowerInjection [high · 80% confidence] in dpsim-models/src/SP/SP_Ph1_SVC.cpp:95 (details inline)

🟡 Suggestions

• Division by zero risk in per-unit conversion [medium · 90% confidence] in dpsim-models/src/SP/SP_Ph1_SVC.cpp:63 (details inline)
• Replace hard-coded epsilon with DOUBLE_EPSILON [medium · 82% confidence] in dpsim/src/PFSolverPowerPolar.cpp:691 (details inline)
• Replace hard-coded epsilon with DOUBLE_EPSILON [medium · 82% confidence] in dpsim/src/PFSolverPowerPolar.cpp:704 (details inline)
• Noisy INFO log for SVC voltage-control step [medium · 70% confidence] in dpsim/src/PFSolverPowerPolar.cpp:713 (details inline)
• Guard against degenerate sensitivity with a more descriptive fallback [medium · 58% confidence] in dpsim/src/PFSolverPowerPolar.cpp:695 (details inline)

🔵 Optional / low-confidence (2)

• Add documentation page for SP::Ph1::SVC [medium · 35% confidence · unconfirmed] in docs/hugo/content/en/docs/Models/SP_Ph1_SVC.md
• Notebook not integrated into automated test suite [medium · 35% confidence · unconfirmed] in examples/Notebooks/Grids/PF_SVC_VoltageControl.ipynb

Claim vs. implementation

• Claimed: Add a solver-side outer loop to hold each SVC bus at its voltage setpoint, enabled via set_pf_solver_enforce_svc_control.
• Done: Adds an SVC component/model plus PF solver support and an outer loop that adjusts SVC reactive injection to regulate bus voltage, with Python bindings and an example notebook.
• Difference: none

How this review was produced

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

Refuted by verification:

• Validate input parameters in setParameters to prevent division by zero (dpsim-models/src/SP/SP_Ph1_SVC.cpp): setParameters only stores values and logs them; no division happens there, while the per-unit divisions are in calculatePerUnitParameters at lines 63-68.
• Replace hard-coded epsilon with DOUBLE_EPSILON (dpsim/src/PFSolverPowerPolar.cpp): The file uses the literal 1e-12 here, but the finding’s claim about 1e-9 at this line is not supported by the source; the 1e-9 threshold appears separately at line 694.
• Use Math::isFinite for Q limit checks (dpsim/src/PFSolverPowerPolar.cpp): qNew is clamped only after it is computed, but the code already guards the inputs with CPS::Math::isFinite(qMaxPU)/qMinPU; the finding’s claim about missing NaN/Inf protection for qNew is speculative and not a concrete defect in this file.
• Typo in exception message (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The exception message is exactly the string in the source, and the finding is only a style complaint about spaces, not a defect.
• Flush logger after construction to ensure timely output (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The constructor explicitly calls mSLog->flush() immediately after logging, so the claimed missing/extra flush behavior is contradicted by the file.
• Clear SVC history map at the start of each solve to avoid stale state (dpsim/src/PFSolverPowerPolar.cpp): The file explicitly clears mSvcPrevQV at line 67, immediately after the comment that says the history is per-solve.
• Use absolute value of B_kk for sensitivity to avoid sign ambiguity (dpsim/src/PFSolverPowerPolar.cpp): The code already seeds dQdV from std::abs(B(idx, idx)), and the surrounding comment already states |B_kk|.
• Avoid redundant convergence check in solvePowerflow (dpsim/src/PFSolver.cpp): solvePowerflow returns immediately when neither mEnforceReactiveLimits nor mEnforceSvcControl is set, and runNewtonRaphson already resets isConverged before solving
• Remove redundant break in modifyPowerFlowBusType (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The break after throw is present but unreachable by construction, so this is not a bug.
• Clarify SVC voltage-control loop intent in comments (dpsim/src/PFSolverPowerPolar.cpp): The comment is present and the method body clearly implements the SVC outer-loop control; the suggested extra wording is stylistic, not a defect.
• Fix misleading comment in SVC constructor documentation (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): The constructor comment at line 53 says "Defines name and logging level" and the delegating constructor at line 54 passes name,name,logLevel intentionally.
• Fix misleading comment in setBaseVoltage method (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The comment 'Set base voltage' is present exactly as claimed, and comment specificity is not a code defect.
• Fix misleading comment in modifyPowerFlowBusType method (dpsim-models/src/SP/SP_Ph1_SVC.cpp): modifyPowerFlowBusType is declared override in the header and implemented here; the comment critique is stylistic only.
• Fix misleading comment in enforceSvcVoltageControl method declaration (dpsim/include/dpsim/PFSolverPowerPolar.h): PFSolver::enforceSvcVoltageControl is explicitly declared as a virtual base no-op, so the comment’s override wording matches the code pattern
• Fix misleading comment in enforceSvcVoltageControl method definition (dpsim/src/PFSolverPowerPolar.cpp): The comment already describes the lambda as selecting the SVC at a node, and the code directly defines that lambda in place.

_{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.}

[github-actions]

Guard against NaN/Inf propagation in updateReactivePowerInjection
severity: high · confidence: 80%

The method assigns powerInj.imag() to mSetPointReactivePower without checking finiteness. If upstream data (e.g., a numerically unstable outer loop) yields NaN/Inf, this will poison the admittance matrix and solver state, causing silent corruption.

Suggested fix: Before assignment, check Math::isFinite(powerInj.imag()) and throw std::invalid_argument if not finite. Also validate mBaseApparentPower > DOUBLE_EPSILON to avoid division by zero in the per-unit update.

Checked against the source: updateReactivePowerInjection assigns powerInj.imag() directly with no finite-value guard

_{stage: numerics}

[github-actions]

Division by zero risk in per-unit conversion
severity: medium · confidence: 90%

SP::Ph1::SVC::calculatePerUnitParameters divides the set-point voltage by mBaseVoltage without checking that mBaseVoltage is non-zero. If a user forgets to call setBaseVoltage before the power-flow initialization, this will produce NaN/Inf values, leading to solver divergence or crashes.

Suggested fix: Validate that mBaseVoltage is positive before performing the division and throw a clear exception or log an error if it is zero.

Suggested change

	**mSetPointVoltagePerUnit = **mSetPointVoltage / mBaseVoltage;
	if (mBaseVoltage <= 0) {
	throw std::invalid_argument("Base voltage must be set and positive before per-unit conversion in SVC");
	}
	**mSetPointVoltagePerUnit = **mSetPointVoltage / mBaseVoltage;

Checked against the source: calculatePerUnitParameters divides by mBaseVoltage without checking it is initialized and nonzero

_{stage: io-robustness}

[github-actions]

Replace hard-coded epsilon with DOUBLE_EPSILON
severity: medium · confidence: 82%

The secant step uses a hand-typed literal 1e-12 instead of the project-defined DOUBLE_EPSILON. This violates the convention to use shared constants.

Suggested fix: Replace std::abs(dV) > 1e-12 with std::abs(dV) > DOUBLE_EPSILON.

Suggested change

	if (std::abs(dV) > 1e-12 && std::abs(dQ) > 1e-15)
	if (std::abs(dV) > DOUBLE_EPSILON && std::abs(dQ) > 1e-15)

Checked against the source: line 691 uses literal 1e-12 while DOUBLE_EPSILON is defined in Definitions.h

_{stage: numerics}

[github-actions]

Replace hard-coded epsilon with DOUBLE_EPSILON
severity: medium · confidence: 82%

The convergence check uses a hand-typed literal 1e-12 instead of the project-defined DOUBLE_EPSILON. This violates the convention to use shared constants.

Suggested fix: Replace std::abs(dq) < 1e-12 with std::abs(dq) < DOUBLE_EPSILON.

Suggested change

	if (std::abs(dq) < 1e-12)
	if (std::abs(dq) < DOUBLE_EPSILON)

Checked against the source: line 704 uses literal 1e-12 in the dq threshold while DOUBLE_EPSILON exists

_{stage: numerics}

[github-actions]

Noisy INFO log for SVC voltage-control step
severity: medium · confidence: 70%

During enforceSvcVoltageControl, each SVC update is logged with SPDLOG_LOGGER_INFO. This is detailed diagnostic information and should be logged at DEBUG level.

Suggested fix: Change the log macro to SPDLOG_LOGGER_DEBUG.

Checked against the source: each SVC adjustment is logged with SPDLOG_LOGGER_INFO inside the per-bus update loop

_{stage: logging}

[github-actions]

Guard against degenerate sensitivity with a more descriptive fallback
severity: medium · confidence: 58%

When the computed sensitivity dQdV is non-finite or below a threshold, the code falls back to 1.0. While this is numerically safe, the fallback value is arbitrary and could lead to slow convergence if the system is ill-conditioned. A more robust approach would be to use a small positive value (e.g., 1e-3) or to log a warning when the fallback is triggered.

Suggested fix: Consider logging a warning when the fallback sensitivity is used and/or using a small positive value (e.g., 1e-3) instead of 1.0 to avoid large steps in ill-conditioned systems. Alternatively, use the absolute value of the local susceptance as a fallback: dQdV = std::abs(B(idx, idx)) > 1e-9 ? std::abs(B(idx, idx)) : 1e-3;

Checked against the source: degenerate or non-finite dQdV is silently replaced with the fixed value 1.0

_{stage: realtime-resources}

[github-actions]

DPsim LLM review

Claim vs. code: matches the description.

TL;DR: One real correctness issue stands out: SVC limit ordering is unchecked and can break the outer voltage-control loop; the remaining items are lower-confidence documentation/test-coverage gaps and a possible solver reclassification edge case, while the DCO note is already satisfied by the commit metadata. The change otherwise appears to implement the intended SVC outer loop without any other high-confidence defects surfaced.

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

🟡 Suggestions

• Missing validation of SVC reactive power limits ordering [medium · 90% confidence] in dpsim-models/src/SP/SP_Ph1_SVC.cpp:33 (details inline)

🔵 Optional / low-confidence (5)

• Add DCO sign-off to commit introducing the notebook [high · 20% confidence · unconfirmed] in examples/Notebooks/Grids/PF_SVC_VoltageControl.ipynb:1
• Re-run Newton-Raphson after SVC control changes [medium · 35% confidence · unconfirmed] in dpsim/src/PFSolver.cpp:647
• Add documentation page for SP::Ph1::SVC [medium · 35% confidence · unconfirmed] in docs/hugo/content/en/docs/Models/SP/SP_Ph1_SVC.md
• Missing documentation for new SVC component [medium · 35% confidence · unconfirmed] in docs/hugo/content/en/docs/Models/Concepts/
• Notebook not included in test suite [medium · 35% confidence · unconfirmed] in examples/Notebooks/Grids/PF_SVC_VoltageControl.ipynb

Claim vs. implementation

• Claimed: Add a solver-side outer loop to hold SVC buses at their voltage setpoints, enabled via set_pf_solver_enforce_svc_control.
• Done: Adds an SVC component model plus power-flow support and an optional outer loop in the polar PF solver that adjusts SVC reactive injection to regulate bus voltage, with Python bindings and an example notebook.
• Difference: none

How this review was produced

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

Refuted by verification:

• Incorrect reactive power extraction from Complex power injection (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The code intentionally sets Q_set from powerInj.imag() and immediately converts it to per-unit; no sign-convention bug is evidenced in this file.
• Parameter scaling inconsistency in example (examples/Notebooks/Grids/PF_SVC_VoltageControl.ipynb): The notebook explicitly names the argument v_set_pu and multiplies it by BASE_V to convert per-unit to volts, matching the surrounding text and the SVC setpoint API usage.
• Incorrect inheritance: SimPowerComp should be SP::SimPowerComp (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): The class is in namespace CPS::SP::Ph1, and inheriting SimPowerComp is consistent with the other SP phase-1 components in this codebase.
• Incorrect inheritance in constructor initialization list (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The constructor correctly initializes the direct base as SimPowerComp(uid, name, logLevel), which is the same type as the class declaration via namespace SP::Ph1::SVC.
• Missing per-unit scaling of setPointVoltage in setParameters (dpsim-models/src/SP/SP_Ph1_SVC.cpp): setParameters stores the voltage setpoint in volts, and calculatePerUnitParameters later converts it to per-unit at line 71.
• Incorrect handling of reactive power limits in updateReactivePowerInjection (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The method updates both Q_set and Q_set_pu consistently; limit enforcement is not part of this function and is not shown missing here.
• Potential division by zero in secant sensitivity calculation (dpsim/src/PFSolverPowerPolar.cpp): The code checks std::abs(dV) > DOUBLE_EPSILON before performing dQ / dV, so the division is guarded.
• Inconsistent comment style in calculatePerUnitParameters (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The comment is just a section header and matches the file’s existing style; no defect is present.
• Unsafe access to powerInj.imag() without finiteness check (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The code simply extracts powerInj.imag(); no finiteness check is required by any visible contract in this file.
• Use of ad-hoc epsilon literals for secant sensitivity floors (dpsim/src/PFSolverPowerPolar.cpp): These are local heuristic constants for the SVC loop, and the file already uses them intentionally as named constexpr values.
• Comparison against DOUBLE_EPSILON without Math::isFinite guard (dpsim/src/PFSolverPowerPolar.cpp): The comparison against DOUBLE_EPSILON is inside a guard that only decides whether to compute the secant update; no NaN/Inf-specific bug is demonstrated here.
• Use of DOUBLE_EPSILON for convergence check on reactive injection step (dpsim/src/PFSolverPowerPolar.cpp): DOUBLE_EPSILON is used as a near-zero threshold for dq, which is a deliberate convergence/pinning check in this loop.
• Inconsistent whitespace in error message string (dpsim-models/src/SP/SP_Ph1_SVC.cpp): The thrown string is exactly the shown message; whitespace style alone is not a functional defect.
• Missing scheduler dependency declaration for SVC reactive power updates (dpsim-models/src/SP/SP_Ph1_SVC.cpp): This component is not an MNASimPowerComp and the file contains no task-graph dependency hooks to declare.
• SVC reactive power update not tagged as external side-effect (dpsim/src/PFSolverPowerPolar.cpp): This task is already marked externally visible by the solver's SolveTask in the related source, and the side effect is intentional for scheduler visibility.

_{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.}

[github-actions]

Missing validation of SVC reactive power limits ordering
severity: medium · confidence: 90%

The setParameters() method accepts qLimMax and qLimMin but does not verify that qLimMax >= qLimMin. Supplying inverted limits could cause the outer voltage‑control loop to behave incorrectly, potentially driving the reactive injection to an unintended direction or causing the solver to oscillate.

Suggested fix: Add a check that throws std::invalid_argument if qLimMax < qLimMin, and optionally ensure the limits are finite when required.

Suggested change

	**mReactivePowerMax = qLimMax;
	if (qLimMax < qLimMin)
	throw std::invalid_argument("SVC: qLimMax must be greater than or equal to qLimMin.");

Checked against the source: setParameters assigns Q_max and Q_min without checking qLimMax >= qLimMin

_{stage: numerics}