Skip to content

fix(models): default a load's power flow bus type to PQ - #643

Draft
leonardocarreras wants to merge 2 commits into
sogno-platform:masterfrom
leonardocarreras:fix/pf-load-default-bus-type
Draft

fix(models): default a load's power flow bus type to PQ#643
leonardocarreras wants to merge 2 commits into
sogno-platform:masterfrom
leonardocarreras:fix/pf-load-default-bus-type

Conversation

@leonardocarreras

@leonardocarreras leonardocarreras commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

A load could otherwise reach the power flow solver with an unset bus type, or when is together with a slack.

@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 low-confidence documentation gap remains: the new default PQ bus-type behavior for loads is not reflected in the docs, while the code change itself matches the PR intent and no other issues were reported. Overall risk is low, but the user-facing model documentation should be updated to avoid confusion about power-flow initialization.

Found 1 medium (0 anchored to lines below).

🔵 Optional / low-confidence (1)
  • Missing documentation of default PowerflowBusType for Load [medium · 35% confidence · unconfirmed] in docs/hugo/content/en/docs/Models/Concepts/Load.md
Claim vs. implementation
  • Claimed: Default a load’s power flow bus type to PQ so it cannot reach the solver unset.
  • Done: Sets the shared bus-type member default to None, assigns Load construction to PQ, and adds an example/test that checks the default and compares solver results.
  • Difference: none
How this review was produced

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

Refuted by verification:

  • Incorrect default power flow bus type assignment in Load constructor (dpsim-models/src/SP/SP_Ph1_Load.cpp): The constructor explicitly assigns mPowerflowBusType = PowerflowBusType::PQ on line 30, and the base default of None is overridden by that assignment.
  • Incorrect initialization of mPowerflowBusType in constructor (dpsim-models/src/SP/SP_Ph1_Load.cpp): Line 30 sets the bus type to PQ in the constructor body, so it is not left at the base-class default None.
  • Use DOUBLE_EPSILON for tolerance comparison in test (dpsim-models/src/SP/SP_Ph1_Load.cpp): This file is SP_Ph1_Load.cpp and contains no test comparing voltage magnitudes or any 1e-9 tolerance.
  • Use DOUBLE_EPSILON for tolerance comparison in test (dpsim/examples/cxx/Components/SP_Ph1_Load_DefaultPowerflowBusType.cpp): The check is a deliberate sanity assertion on the solved voltage magnitude, not a near-zero comparison, and the file already includes the SPDX header.
  • Ensure mPowerflowBusType is set before any task or dependency can read it (dpsim-models/src/SP/SP_Ph1_Load.cpp): The assignment to mPowerflowBusType happens in the constructor body immediately after terminal setup, and there is no external task execution during construction.
  • Initialize mPowerflowBusType in constructor initializer list (dpsim-models/src/SP/SP_Ph1_Load.cpp): The code intentionally assigns mPowerflowBusType in the constructor body; the claim about needing initializer-list placement is a style preference, not a defect.
  • Provide virtual destructor for polymorphic base class (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The class has no user-defined destructor issue here; it is a small interface with shared_ptr members and no owning raw resources, so the claimed UB is not evidenced in this file
  • Direct public access to mPowerflowBusType (dpsim/examples/cxx/Components/SP_Ph1_Load_DefaultPowerflowBusType.cpp): The test intentionally accesses the public member mPowerflowBusType directly on a freshly constructed Load, matching the DPsim pattern for public const attributes.
  • Use of PI constant without namespace qualification (dpsim/examples/cxx/Components/SP_Ph1_Load_DefaultPowerflowBusType.cpp): PI is brought into scope by 'using namespace DPsim;' and the file uses PI consistently in the log statement.
  • Add Doxygen comment for mPowerflowBusType (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The member already has a Doxygen comment immediately above it: '/// Define the type of bus the component is modelled by'
  • INFO-level log message contains embedded newlines (dpsim/examples/cxx/Components/SP_Ph1_Load_DefaultPowerflowBusType.cpp): The log call is a single formatted string split across source lines, with no embedded newline character in the message.
  • Expose mPowerflowBusType to Python for SP::Ph1::Load (dpsim/src/pybind/SPComponents.cpp): The Load binding already exposes modify_power_flow_bus_type at lines 128-129, and the claim is about a Python-exposed member rather than a missing binding in this file.
  • Ensure power flow bus type is set before any external use (dpsim-models/src/SP/SP_Ph1_Load.cpp): The constructor sets the bus type to PQ directly on line 30, so the object is not left with the base default None after construction.
  • Document the default power flow bus type semantics (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The default is explicit in code as '= PowerflowBusType::None', so the semantics are visible at the declaration
  • Remove non-portable -fPIC guard to avoid host-specific builds (dpsim/examples/cxx/CMakeLists.txt): The file already contains the non-Windows -fPIC guard and the comment explains it is needed for static libs into the .so.

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 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 medium documentation gap remains: the new load bus-type defaults and slack/load VD classification change are not documented for users, while no code correctness issues were reported in the reviewed passes. The rest of the change appears low-risk from the available findings.

Found 1 medium (0 anchored to lines below).

🔵 Optional / low-confidence (1)
  • Add documentation for SP::Ph1::Load bus type behavior [medium · 35% confidence · unconfirmed] in docs/hugo/content/en/docs/Models/SP/Load.md
Claim vs. implementation
  • Claimed: Default loads’ power flow bus type to PQ and treat a node with both slack and load as VD.
  • Done: Sets load components to initialize as PQ, adds solver logic to classify VD+PQ nodes as VD, and adds an example/test covering bus-type combinations.
  • Difference: none
How this review was produced

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

Refuted by verification:

  • Incorrect bus-type flip assertion for PV+PQ node (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The file explicitly checks that remoteBoth.remoteVoltage stays at the generator set point, so the test does not assert a PQ-to-PV flip there.
  • Incorrect bus-type rule for VD+PQ node (dpsim/src/PFSolver.cpp): The new branch explicitly classifies VD+PQ as VD bus, matching the surrounding VD-dominates rules and the added comment
  • Set default power flow bus type in constructor initializer list (dpsim-models/src/SP/SP_Ph1_Load.cpp): The constructor body explicitly sets mPowerflowBusType = PowerflowBusType::PQ; and the base member already defaults to None, so this is a deliberate assignment, not a missing initializer defect.
  • Initialize mPowerflowBusType in constructor initializer list (dpsim-models/src/SP/SP_Ph1_Load.cpp): The same line is the only changed line and the assignment is present in the constructor body; the finding critiques style, not a code error.
  • Missing numerical guard for zero nominal voltage in Load::initializeParentFromNodesAndTerminals (dpsim-models/src/SP/SP_Ph1_Load.cpp): The code warns on mNomVoltage == 0 and only computes resistance when **mActivePower != 0; the finding’s claimed unconditional division by zero is not what the file does.
  • Use of uninitialized mTerminals[0] in Load::initializeParentFromNodesAndTerminals (dpsim-models/src/SP/SP_Ph1_Load.cpp): The constructor calls setTerminalNumber(1), and initializeParentFromNodesAndTerminals accesses mTerminals[0] in that established single-terminal pattern.
  • Incorrect phasor convention in Load::initializeParentFromNodesAndTerminals for initial current (dpsim-models/src/SP/SP_Ph1_Load.cpp): The current is computed as conj(S / V), which matches the standard load convention for S = V conj(I) when using consumed P and Q.
  • Use of ad-hoc literal 1e-6 for zero voltage check in propagateAndVerifyBaseVoltage (dpsim/src/PFSolver.cpp): The file already uses mBaseVoltageLooseTolerance/mBaseVoltageStrictTolerance elsewhere, but this zero-voltage guard is a local near-zero filter and not a missing symbol
  • Use of ad-hoc literal 1e-6 for nominal voltage comparison in propagateAndVerifyBaseVoltage (dpsim/src/PFSolver.cpp): The comparison is in the existing load-sync loop and the code already has dedicated base-voltage tolerances for mismatch checks
  • Use of ad-hoc literal 1e-6 for zero base voltage check in propagateAndVerifyBaseVoltage (dpsim/src/PFSolver.cpp): This is the final zero-base-voltage warning check, and the file already defines DOUBLE_EPSILON in included ground truth but does not require it here
  • Initialize mPowerflowBusType to PowerflowBusType::None in header (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The member is explicitly default-initialized to PowerflowBusType::None on line 39
  • Set mPowerflowBusType in constructor initializer list (dpsim-models/src/SP/SP_Ph1_Load.cpp): The constructor already assigns mPowerflowBusType = PowerflowBusType::PQ; on the changed line, so this is not missing.
  • Avoid per-step dynamic memory allocation in hot-path example (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The component objects are created inside solve() once per case, not inside any per-step loop; there is no hot-path allocation loop in this file.
  • Avoid per-step dynamic memory allocation in hot-path example (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): SystemTopology is constructed once per solve() call as part of a one-shot example run, not repeatedly in a timing loop.
  • Avoid per-step dynamic memory allocation in hot-path example (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): Simulation is also constructed once per solve() call for a single run, with no repeated stepping loop in this file.

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.

@leonardocarreras
leonardocarreras marked this pull request as ready for review August 7, 2026 10:04

@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 low-severity CI coverage gap remains: the new PF_Load_BusTypeCombinations example is not added to the test source list, while the code-change itself appears aligned with the PR intent and no higher-risk correctness issues were surfaced. The documentation findings are speculative and duplicate each other, so they were dropped.

Found 1 low (1 anchored to lines below).

🟡 Suggestions

  • Add new example to TEST_SOURCES for CI coverage [low · 75% confidence] in dpsim/examples/cxx/CMakeLists.txt:27 (details inline)
Claim vs. implementation
  • Claimed: Default a load’s power flow bus type to PQ and classify a node with both a slack source and a load as VD.
  • Done: Sets load components to PQ by default, initializes the interface bus type to None, adds solver logic to treat VD+PQ nodes as VD, and adds a regression example/test.
  • Difference: none
How this review was produced

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

Refuted by verification:

  • Incorrect PV bus voltage assertion in test case (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The file explicitly asserts remoteGenerator.remoteVoltage is within voltageTolerance of nominalVoltage, which is the intended PV-bus check here.
  • Incorrect PV bus behavior assertion when load and generator coexist (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The file already checks the combined load+generator case stays at nominalVoltage; the extra reactive-power assertion requested is not required by this example test.
  • Ambiguous PQ-to-PV flip observability test (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The test intentionally compares remoteBoth.remoteVoltage against remoteLoad.remoteVoltage to detect a PQ-to-PV change, so the claimed ambiguity is the expected observable.
  • Initialize mPowerflowBusType in constructor initializer list (dpsim-models/src/SP/SP_Ph1_Load.cpp): mPowerflowBusType is already initialized in the base member declaration to PowerflowBusType::None, and the new assignment at line 30 is a deliberate post-construction set to PQ
  • Use explicit initializer for mPowerflowBusType in header (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The member is already explicitly initialized inline as PowerflowBusType::None, which is a valid and intentional DPsim pattern.
  • Use DOUBLE_EPSILON for base-voltage tolerance comparisons (dpsim/src/PFSolver.cpp): The file already uses mBaseVoltageStrictTolerance and mBaseVoltageLooseTolerance for the zone mismatch checks, not a hard-coded 1e-6
  • Use DOUBLE_EPSILON for nominal-voltage comparison (dpsim/src/PFSolver.cpp): The nominal-voltage update guard is a local 1e-6 comparison in existing code, but this line was not changed by the PR and the file already contains the surrounding load-sync logic
  • Guard division by zero when computing mResistance from active power (dpsim-models/src/SP/SP_Ph1_Load.cpp): the division is guarded by if (**mActivePower != 0) before mResistance = std::pow(mNomVoltage, 2) / **mActivePower;
  • Guard division by zero when computing mReactance from reactive power (dpsim-models/src/SP/SP_Ph1_Load.cpp): the reactive-power division is guarded by if (**mReactivePower != 0) before mReactance = std::pow(mNomVoltage, 2) / **mReactivePower;
  • Initialize member to match default in declaration (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): There is no constructor-body initialization here; the only initialization is the inline default at the declaration.
  • Attribute usage: mPowerflowBusType should be a plain member (dpsim-models/src/SP/SP_Ph1_Load.cpp): mPowerflowBusType is a public Attribute-style state in the related interface PFSolverInterfaceBus, so this file is not introducing a plain-member replacement bug
  • Use Domain::SP constant instead of hard-coded numeric frequency (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The topology is constructed with SystemTopology(50, ...) in the file, so the hard-coded frequency is already present as written.
  • Provide default constructor initialization in header (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The constructor is defaulted, but the member is already initialized in the class definition, so there is no missing constructor initialization.
  • Remove commented-out example (dpsim/examples/cxx/CMakeLists.txt): The commented-out example is already present as an intentional comment at line 34, not a build defect in the changed line
  • Add missing initializer comment for mPowerflowBusType (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The member already has a Doxygen comment immediately above it describing its purpose.

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.


# Powerflow examples
Circuits/PF_Slack_PiLine_PQLoad.cpp
Circuits/PF_Load_BusTypeCombinations.cpp

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.

Add new example to TEST_SOURCES for CI coverage
severity: low · confidence: 75%

The new example PF_Load_BusTypeCombinations.cpp is added to CIRCUIT_SOURCES but not to TEST_SOURCES. While this does not break correctness, it means the example will not be executed as part of the test suite in CI, reducing coverage of the new functionality.

Suggested fix: Add PF_Load_BusTypeCombinations to the TEST_SOURCES list so it is built and run as part of the test suite.

Checked against the source: line 27 adds PF_Load_BusTypeCombinations.cpp to CIRCUIT_SOURCES, and TEST_SOURCES lines 172-201 do not include it

stage: domain-modeling

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.55%. Comparing base (209f7bc) to head (cdebe33).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #643      +/-   ##
==========================================
- Coverage   72.86%   72.55%   -0.31%     
==========================================
  Files         512      512              
  Lines       33219    33223       +4     
  Branches    18018    18021       +3     
==========================================
- Hits        24204    24106      -98     
- Misses       8996     9116     +120     
+ Partials       19        1      -18     

☔ 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 force-pushed the fix/pf-load-default-bus-type branch 2 times, most recently from e6be0cc to cdebe33 Compare August 7, 2026 13:21

@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 low-confidence documentation gap was flagged for the new example, but no code correctness issues were reported in the reviewed passes; the change otherwise appears focused on the intended bus-type defaulting/classification behavior.

Found 1 medium (0 anchored to lines below).

🔵 Optional / low-confidence (1)
  • Add documentation for new example PF_Load_BusTypeCombinations [medium · 35% confidence · unconfirmed] in dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp:1
Claim vs. implementation
  • Claimed: Default load power-flow bus type to PQ and classify a node with both slack and load as VD.
  • Done: Loads now initialize their power-flow bus type to PQ, the solver treats VD+PQ nodes as VD, and a new example/test checks load bus-type combinations.
  • Difference: none
How this review was produced

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

Refuted by verification:

  • Incorrect bus-type classification for VD+PQ nodes (dpsim/src/PFSolver.cpp): The added branch explicitly pushes the node to mVDBuses, matching the existing VD-priority convention in this function
  • Incorrect assertion for PV+PQ node voltage (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The code explicitly expects remoteBoth.remoteVoltage to stay at nominalVoltage for a PV+PQ bus, which matches the intended PV-bus behavior in this example.
  • Initialize mPowerflowBusType to PowerflowBusType::None in header (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The file already shows the in-class initializer = PowerflowBusType::None, so the claimed missing/incorrect initializer is not a defect here.
  • Initialize mPowerflowBusType in constructor initializer list (dpsim-models/src/SP/SP_Ph1_Load.cpp): The constructor body explicitly sets mPowerflowBusType = PowerflowBusType::PQ on line 30, so it is not missing initialization.
  • Use default member initializer for mPowerflowBusType (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The alleged redundancy with derived-class initialization is not present in this file, and the member is intentionally default-initialized in-class.
  • Missing case for VD and PQ bus classification in determinePFBusType (dpsim/src/PFSolver.cpp): The branch does update the bus-type list with mVDBuses.push_back(node), so it does not remain unset
  • Use DOUBLE_EPSILON for zero-voltage tolerance in propagateAndVerifyBaseVoltage (dpsim/src/PFSolver.cpp): The file uses the 1e-6 literal in a zero-voltage skip check, but this is pre-existing code outside the changed hunk and not a new defect here
  • Use DOUBLE_EPSILON for nominal voltage update tolerance in propagateAndVerifyBaseVoltage (dpsim/src/PFSolver.cpp): The 1e-6 nominal-voltage comparison is also pre-existing code outside the changed lines
  • Verify load bus type via attribute access, not direct member (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The file intentionally reads freshLoad->mPowerflowBusType directly, and the claim is about a style guideline rather than a defect in this code.
  • Avoid dynamic memory allocation in hot-path loop (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): This is a standalone example test; constructing SystemTopology and Simulation inside solve() is the whole point of each case run, not a hot-path loop bug.
  • Simulation object not reused across solve() calls (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): A new Simulation per solve() call is visible in the example and is appropriate for isolated test cases.
  • SystemTopology not reused across solve() calls (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): A new SystemTopology per solve() call is visible in the example and is appropriate for isolated test cases.
  • Provide virtual destructor for polymorphic base class (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): This class is not a polymorphic deletion base in the shown code; it has no virtual destructor requirement evidenced here, and the finding is based only on the presence of a pure virtual method.
  • Remove redundant bus type check on fresh load (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The fresh-load PQ assertion is present in the file and is part of the test's intended coverage.
  • Add missing initializer comment for mPowerflowBusType (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The member already has a Doxygen comment immediately above it: /// Define the type of bus the component is modelled by.

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 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 medium documentation gap was flagged for the new PF_Load_BusTypeCombinations example; no correctness, scaling, or scheduling issues were reported in the provided pass, so the main risk appears limited to discoverability rather than behavior.

Found 1 medium (0 anchored to lines below).

🔵 Optional / low-confidence (1)
  • Missing documentation for new PF_Load_BusTypeCombinations example [medium · 35% confidence · unconfirmed] in docs/hugo/content/en/docs/Models/Concepts
Claim vs. implementation
  • Claimed: Default loads to PQ bus type and treat a node with both slack and load as VD in power flow classification.
  • Done: Sets load components to PQ by default, initializes bus type to None in the interface, adds solver logic to classify VD+PQ nodes as VD, and adds a regression example/test for load bus-type combinations.
  • Difference: none
How this review was produced

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

Refuted by verification:

  • Incorrect default power flow bus type initialization for Load (dpsim-models/src/SP/SP_Ph1_Load.cpp): The constructor’s explicit mPowerflowBusType = PowerflowBusType::PQ; is present on line 30, and the base default None in PFSolverInterfaceBus does not make this a defect.
  • Incorrect bus-type classification for VD+PQ node (dpsim/src/PFSolver.cpp): The new branch explicitly classifies VD+PQ as VD bus in determinePFBusType, matching the code and the existing VD+PV/VD,PV,PQ handling pattern.
  • Incorrect test assertion for VD+PQ node behavior (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The code intentionally checks that adding a generator to a load node changes the solution; the failing condition is on remoteBoth vs remoteLoad, not a claim that VD+PQ must remain VD.
  • Initialize mPowerflowBusType to PowerflowBusType::None in header (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The file already initializes mPowerflowBusType to PowerflowBusType::None inline, which is the exact pattern the finding describes as correct.
  • Provide default PowerflowBusType in PFSolverInterfaceBus (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): PowerflowBusType::None is a valid enum value in Definitions.h, and the header explicitly uses it as the default initializer.
  • Use DOUBLE_EPSILON for base voltage tolerance comparison (dpsim/src/PFSolver.cpp): The 1e-6 comparison at line 385 is the zero-voltage filter, not the tolerance comparison at line 408, which uses mBaseVoltageStrictTolerance.
  • Use DOUBLE_EPSILON for zero base voltage check (dpsim/src/PFSolver.cpp): The zero base-voltage check at line 472 uses 1e-6, but the file already defines and uses dedicated tolerances elsewhere; this finding targets a pre-existing literal outside the changed hunk.
  • Guard division by active power in Load::initializeParentFromNodesAndTerminals (dpsim-models/src/SP/SP_Ph1_Load.cpp): The code already guards the resistance calculation with if (**mActivePower != 0) before dividing on line 131.
  • Guard division by voltage in Load::initializeParentFromNodesAndTerminals (dpsim-models/src/SP/SP_Ph1_Load.cpp): The current is computed from attributeTyped<Real>("P")->get() and attributeTyped<Real>("Q")->get() divided by (**mIntfVoltage)(0, 0) exactly as shown on lines 169-171.
  • Initialize member in header to avoid default-constructed state (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The in-class initializer is present on the member declaration, so the default-constructed state is not left uninitialized.
  • Add missing bus-type classification for VD and PQ together (dpsim/src/PFSolver.cpp): The code does classify VD+PQ as VD, exactly as the finding describes as correct.
  • Use typed member access for interface voltage in initialization (dpsim-models/src/SP/SP_Ph1_Load.cpp): mIntfVoltage is accessed via the typed member pointer (**mIntfVoltage)(0, 0) on line 168, not via string lookup.
  • Declare correct attribute dependencies for parent post-step task (dpsim-models/src/SP/SP_Ph1_Load.cpp): mnaParentAddPostStepDependencies is implemented on lines 192-200, and the body matches the dependency declaration.
  • Access power injection attributes via typed members (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The code already dereferences the public injection attributes with **slack->mActivePowerInjection and **slack->mReactivePowerInjection.
  • Avoid unnecessary copies of String in function parameters (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The function already takes the String parameter by const reference: solve(const String &caseName, ...).

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.

@leonardocarreras leonardocarreras self-assigned this Aug 7, 2026
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
Suggested-by: Matthias Mees <matthias.mees@rwth-aachen.de>
Signed-off-by: Leonardo Carreras <leonardo.carreras@eonerc.rwth-aachen.de>
@leonardocarreras
leonardocarreras force-pushed the fix/pf-load-default-bus-type branch from cdebe33 to 14954d5 Compare August 7, 2026 19:58

@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 correctness issue remains around bus-type classification for nodes that combine a slack source and a load, while the load-default-to-PQ change itself looks consistent; the remaining items are lower-value validation, tolerance, logging, and documentation concerns. The equation/scheduling passes did not surface additional confirmed problems.

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

🔵 Optional / low-confidence (9)
  • Incorrect bus-type classification for VD+PQ nodes [high · 35% confidence · unconfirmed] in dpsim/src/PFSolver.cpp:238
  • Missing validation of load power parameters [high · 35% confidence · unconfirmed] in dpsim-models/src/SP/SP_Ph1_Load.cpp:33
  • Load constructor sets mPowerflowBusType to PQ [medium · 35% confidence · unconfirmed] in dpsim-models/src/SP/SP_Ph1_Load.cpp:30
  • Set PQ bus type in constructor [medium · 35% confidence · unconfirmed] in dpsim-models/src/SP/SP_Ph1_Load.cpp:30
  • Hard‑coded voltage tolerance instead of defined epsilon [medium · 35% confidence · unconfirmed] in dpsim/src/PFSolver.cpp:385
  • Hard‑coded nominal‑voltage tolerance instead of defined epsilon [medium · 35% confidence · unconfirmed] in dpsim/src/PFSolver.cpp:451
  • Missing documentation for new PF_Load_BusTypeCombinations example [medium · 35% confidence · unconfirmed] in docs/hugo/content/en/docs/Models/Concepts
  • Add documentation for new example PF_Load_BusTypeCombinations [medium · 28% confidence · unconfirmed] in dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp:1
  • Excessive INFO logging in bus type determination loop [low · 35% confidence · unconfirmed] in dpsim/src/PFSolver.cpp:166
Claim vs. implementation
  • Claimed: Default loads’ power flow bus type to PQ and treat nodes with both a slack source and a load as VD.
  • Done: Sets load components to PQ by default, initializes the interface bus type to None, adds solver logic to classify VD+PQ nodes as VD, and adds a regression example/test.
  • Difference: none
How this review was produced

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

Refuted by verification:

  • Incorrect test assertion for slack+load node (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The file explicitly asserts slackLoad.slackVoltage stays at nominalVoltage, which is the intended VD-bus check in this example.
  • Provide default initialization for mPowerflowBusType in PFSolverInterfaceBus (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The header already default-initializes mPowerflowBusType to PowerflowBusType::None on line 39, so there is no missing initializer to remove.
  • Add missing VD/PQ classification branch in determinePFBusType (dpsim/src/PFSolver.cpp): The file already contains the added VD+PQ branch at lines 238-242, so it is not missing.
  • Use default member initializer for mPowerflowBusType (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The inline default member initializer is present exactly at line 39, matching the claim.
  • Add missing VD+PQ bus-type branch in determinePFBusType (dpsim/src/PFSolver.cpp): The VD+PQ case is already handled in the existing determinePFBusType() chain at lines 238-242.
  • Use a shared tolerance constant instead of a literal (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The tolerance is already a named constant on line 134, not an ad-hoc literal in the assertion.
  • Missing null check for slack->mActivePowerInjection before dereference (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The code dereferences slack->mActivePowerInjection and mReactivePowerInjection only after using the same attributes in this file; the claim is about a null check that the file does not indicate is needed.
  • Missing null check for slack->mActivePowerInjection in combined case (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): The combined-case assertion is the same established pattern as the earlier power readout and does not show a missing null check defect in this file.
  • Initialize member in header to avoid default-constructed PowerflowBusType (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): mPowerflowBusType is explicitly initialized in the class definition at line 39.
  • Set PQ bus type in constructor initializer list (dpsim-models/src/SP/SP_Ph1_Load.cpp): The claim about the initializer list is contradicted because the code sets the enum in the constructor body, which is harmless for this plain member.
  • Incorrect voltage/current initialization using attribute lookup (dpsim-models/src/SP/SP_Ph1_Load.cpp): The file itself uses attributeTyped("P") and attributeTyped("Q") at lines 169-170, not mActivePower/mReactivePower.
  • Add missing case for VD+PQ bus type classification (dpsim/src/PFSolver.cpp): The comment and branch for VD+PQ are present exactly at lines 237-242, so the finding’s premise of an incorrect ordering comment is not a defect in the code.
  • Avoid per-step dynamic memory allocation in hot-path example (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): Logger::get is called once in main, not per simulation step or inside the solve loop.
  • Simulation time step and final time are not aligned with power-flow initialization (dpsim/examples/cxx/Circuits/PF_Load_BusTypeCombinations.cpp): This is a standalone example using initialization mode; the one-step time settings are part of the example setup and not shown to be incorrect here.
  • Provide default initialization for mPowerflowBusType in constructor (dpsim-models/include/dpsim-models/Solver/PFSolverInterfaceBus.h): The constructor is already defaulted on line 40, and the member is initialized inline on line 39, so there is no missing constructor initializer.

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.

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@leonardocarreras
leonardocarreras marked this pull request as draft August 11, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant