add power-flow static VAR compensator in SP domain - #580
add power-flow static VAR compensator in SP domain#580leonardocarreras wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #580 +/- ##
==========================================
- Coverage 72.14% 72.01% -0.14%
==========================================
Files 491 493 +2
Lines 31623 31700 +77
Branches 16950 16986 +36
==========================================
+ Hits 22815 22828 +13
- Misses 8722 8871 +149
+ Partials 86 1 -85 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
cfc6f59 to
79d9ee6
Compare
There was a problem hiding this comment.
DPsim LLM review
Claim vs. code: matches the description.
TL;DR: One high-risk issue remains around the SVC interface and several numeric guards, while the rest are mostly duplicate or low-value style/design notes; the change otherwise appears consistent with a power-flow-only PQ-bus model and the build/pybind wiring was flagged as present. The main must-fix items are validating base/rated values before per-unit division and clarifying the reactive-power setter contract so the outer loop cannot silently lose or mis-handle setpoints.
Found 7 high, 13 medium, 3 low (0 anchored to lines below).
🔵 Optional / low-confidence (23)
- Incorrect reactive power injection update in SVC outer-loop interface
[high · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:94 - Missing MNA lifecycle hooks for SP Ph1 SVC
[high · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:26 - Validate ratedApparentPower and ratedVoltage in setParameters
[high · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:29 - Guard division by mBaseVoltage in calculatePerUnitParameters
[high · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:63 - Guard division by mBaseApparentPower in calculatePerUnitParameters
[high · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:64 - Validate powerInj.imag() before division in updateReactivePowerInjection
[high · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:95 - Missing override specifier on virtual method modifyPowerFlowBusType
[high · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:74 - Missing initialization of setPointReactivePower in setParameters
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:32 - Per-unit scaling of voltage setpoint lacks explicit RMS convention
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:63 - Incorrect handling of Complex power injection in updateReactivePowerInjection
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:94 - Incorrect return type for getApparentPower in SP domain
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:100 - Improve exception message formatting and consistency
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:74 - Clarify parameter roles in setParameters documentation
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h:56 - Unused parameter in setParameters
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:29 - Incomplete switch coverage in modifyPowerFlowBusType
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:74 - Missing override specifier on virtual method updateReactivePowerInjection
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:94 - Add new SP_Ph1_SVC.cpp source to dpsim-models CMakeLists
[medium · 35% confidence · unconfirmed]indpsim-models/src/CMakeLists.txt:174 - Include new SP_Ph1_SVC.h header in Components.h
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/Components.h:23 - Expose SP::Ph1::SVC to Python via pybind
[medium · 35% confidence · unconfirmed]indpsim/src/pybind/SPComponents.cpp:166 - Unused parameters in setParameters
[medium · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h:59 - Parameter documentation mismatch
[low · 35% confidence · unconfirmed]indpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h:56 - Unnecessary semicolon after closing brace
[low · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:26 - Fix inconsistent spacing in error message string
[low · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:88
Claim vs. implementation
- Claimed: Add a single-phase SP power-flow SVC model as a PQ-bus voltage-controlling shunt with per-unit reactive/voltage setpoints and Python exposure.
- Done: Introduces a new SP::Ph1::SVC component with V_set/Q_set and Q limits, PQ bus classification, per-unit conversion, apparent-power reporting, and pybind bindings.
- Difference: none
How this review was produced
13 specialized finder passes raised 28 findings over the diff and the full changed sources. After de-duplication, 28 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 5 refuted as unsupported, 23 kept (23 tentative).
Refuted by verification:
- Incorrect base class for SP Ph1 SVC (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): The class is intentionally a non-composite SimPowerComp power-flow bus model; it does not claim MNA stamping, and the header only declares power-flow bus methods plus no MNA hooks are needed here.
- Missing override specifier on virtual method modifyPowerFlowBusType (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): The method already has an override specifier: 'void modifyPowerFlowBusType(PowerflowBusType powerflowBusType) override;'.
- Missing virtual destructor in polymorphic base class (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): SimPowerComp has a virtual destructor ('virtual ~SimPowerComp() {}'), so SVC inherits a virtual destructor through its base classes.
- Parameter passing: long positional list in setParameters (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): This is a style/API critique of the new setParameters signature, not a defect in the file; the declaration is present and consistent with the surrounding DPsim parameter style.
- Expose SVC per-unit attributes to Python (dpsim/src/pybind/SPComponents.cpp): The SVC binding is present at lines 166-180, but this file does not expose per-unit attributes; the finding’s claim is about missing bindings beyond the shown code and is not a defect in the changed lines themselves.
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.
…version Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
There was a problem hiding this comment.
DPsim LLM review
Claim vs. code: matches the description.
TL;DR: One high-risk correctness issue stands out: the new SVC’s per-unit conversion may never run, which would leave its voltage and reactive-power limits at defaults and break outer-loop control; the remaining items are lower-confidence documentation/test coverage gaps and one unconfirmed noexcept suggestion, while no stamping/admittance issues surfaced.
Found 1 critical, 5 medium (0 anchored to lines below).
🔵 Optional / low-confidence (6)
- SVC per-unit conversion never executed
[critical · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:29 - Add documentation page for SP_Ph1_SVC
[medium · 35% confidence · unconfirmed]indocs/hugo/content/en/docs/Models/SP_Ph1_SVC.md - Add a self-contained pytest or notebook for SP_Ph1_SVC
[medium · 35% confidence · unconfirmed]intests/SP_Ph1_SVC.ipynb - Missing documentation for new SP::Ph1::SVC component
[medium · 35% confidence · unconfirmed]indocs/hugo/content/en/docs/Models/Concepts - No test exercising the new SVC component
[medium · 35% confidence · unconfirmed]intests/python - updateReactivePowerInjection should be noexcept
[medium · 35% confidence · unconfirmed]indpsim-models/src/SP/SP_Ph1_SVC.cpp:102
Claim vs. implementation
- Claimed: Add a single-phase power-flow SVC model as a PQ-bus voltage-controlling shunt with per-unit setpoint/limits and no active power or admittance stamp.
- Done: Introduces a new SP::Ph1::SVC component, registers it in C++/Python/build, and makes the solver assign its base voltage from the connected node before per-unit conversion.
- Difference: none
How this review was produced
13 specialized finder passes raised 64 findings over the diff and the full changed sources. After de-duplication, 64 were re-checked against the current file and the base-class / interface headers it inherits (code as truth), escalating survivors to a stronger model: 57 refuted as unsupported, 7 kept (7 tentative).
Refuted by verification:
- Incorrect reactive power extraction from Complex power injection (dpsim-models/src/SP/SP_Ph1_SVC.cpp): updateReactivePowerInjection intentionally stores the reactive component from a Complex injection, and the file contains no requirement to validate powerInj.real()
- Apparent power representation in getApparentPower omits per-unit scaling (dpsim-models/src/SP/SP_Ph1_SVC.cpp): getApparentPower returns Complex(0., **mSetPointReactivePower) exactly as the header comment says, with no per-unit contract in this file
- Missing documentation for ratedApparentPower and ratedVoltage in setParameters (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): The header already documents setParameters with prose explaining the controlled voltage and reactive-power limits.
- Per-unit parameter calculation does not validate ratedVoltage usage (dpsim-models/src/SP/SP_Ph1_SVC.cpp): calculatePerUnitParameters explicitly checks mBaseVoltage <= 0 and throws before dividing
- 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 inherits from SimPowerComp exactly as written; no ambiguity is present in the source.
- Incorrect base class qualification in constructor initializer list (dpsim-models/src/SP/SP_Ph1_SVC.cpp): the constructor initializer list uses SimPowerComp(uid, name, logLevel), which is the correct base in this namespace
- Missing mnaCompInitialize override for power-flow component (dpsim-models/src/SP/SP_Ph1_SVC.cpp): the class is a pure power-flow bus interface component and the file intentionally only implements PFSolverInterfaceBus methods, not MNA hooks
- Missing mBaseApparentPower initialization in calculatePerUnitParameters (dpsim-models/src/SP/SP_Ph1_SVC.cpp): calculatePerUnitParameters assigns mBaseApparentPower = baseApparentPower before using it and then guards mBaseApparentPower <= 0
- Missing system frequency parameter in updateReactivePowerInjection (dpsim-models/src/SP/SP_Ph1_SVC.cpp): updateReactivePowerInjection is a simple setter for the outer control loop and no frequency-dependent behavior is indicated in the file or header
- Missing documentation for calculatePerUnitParameters parameters (dpsim-models/include/dpsim-models/SP/SP_Ph1_SVC.h): The method is already declared in the header; the claim is only about missing docs, but the file contains the API declaration and surrounding power-flow section comments.
- Unnecessary mSLog->flush() after SPDLOG_LOGGER_INFO (dpsim-models/src/SP/SP_Ph1_SVC.cpp): the explicit mSLog->flush() after the constructor log is present in the file and is a deliberate choice, not an accidental omission
- Use DOUBLE_EPSILON for zero-voltage guard in per-unit conversion (dpsim-models/src/SP/SP_Ph1_SVC.cpp): the code already guards with if (mBaseVoltage <= 0) throw ..., so the claimed missing zero-voltage guard is not present
- Validate ratedApparentPower parameter in setParameters (dpsim-models/src/SP/SP_Ph1_SVC.cpp): setParameters stores the provided values into mSetPointVoltage, mReactivePowerMax, and mReactivePowerMin and logs them
- Guard division by mBaseApparentPower in updateReactivePowerInjection (dpsim-models/src/SP/SP_Ph1_SVC.cpp): updateReactivePowerInjection divides by mBaseApparentPower only after calculatePerUnitParameters has validated it positive in this class
- Use DOUBLE_EPSILON for zero-voltage guard when assigning SVC base voltage (dpsim/src/PFSolver.cpp): The file already uses the hard-coded 1e-6 guard at line 453; this is a style complaint, not a missing/absent defect in the changed 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.
Voltage-controlling shunt reactive device modelled as a PQ bus, to be driven by the SVC outer control loop. No active power, no admittance stamp; participates via bus classification and reactive setpoint. Reuses the SynchronGenerator per-unit/attribute pattern (V_set_pu, Q_max_pu, Q_min_pu).