Skip to content

Add BusSignalVoltageOut and BusSignalVoltageIn bus models - #602

Open
pelesh wants to merge 8 commits into
developfrom
slaven/bus-signal-converters
Open

pelesh wants to merge 8 commits into
developfrom
slaven/bus-signal-converters

Conversation

@pelesh

@pelesh pelesh commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Created bus-like objects in Phasor Dynamics module that convert currents and voltages to signals. This is needed for co-simulations where co-simulated objects communicate through signal connection with the co-simulation platform or with each other.

The assumption baked in these objects is that all co-simulated parts have completed their computations and updated their local voltage and current values (and pushed them to co-simulation platform, if connected in that way).

This is still draft, would appreciate feedback from reviewers before finalizing.

Closes #354

Proposed changes

Two new classes derived from PhasorDynamics::BusBase, each in its own subdirectory under GridKit/Model/PhasorDynamics/Bus/ and compiled into the existing phasor_dynamics_bus library:

  • BusSignalVoltageOut keeps the voltage components as its two algebraic variables and publishes them on SignalOut ports vr and vi. SignalIn ports ir and ii set its residuals f_[0] and f_[1] before other injection currents are summed into those two residuals. The Enzyme build adds unit Jacobian entries in the signal variable columns.
  • BusSignalVoltageIn is the mirror image. Its Vr() and Vi() read SignalIn ports vr and vi directly by const reference, so the bus stores no voltage and never modifies it. The sums of current injections from attached components are published on SignalOut ports ir and ii. Like BusInfinite, it contributes no unknowns or equations.

Supporting changes:

  • BusData::BusType gains SIGNAL_VOLTAGE_OUT and SIGNAL_VOLTAGE_IN.
  • SignalNode::read() and SignalIn::readSignal() return a const reference instead of a value. All existing callers copy or cast the result, so behavior is unchanged.
  • Per-model READMEs following the modeling checklist, and unit tests for both classes covering construction, port linking, residual evaluation, verification of unlinked inputs, dependency-tracking derivatives and, with Enzyme enabled, the sparse Jacobian.

Out of scope: the JSON parser, BusFactory and SystemModel do not yet create these bus types, since BusData has no signal ID maps. The port structs already provide a connect() taking enum-to-signal-ID maps for that follow-up.

Checklist

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • The CHANGELOG.md has been updated to reflect the changes.

Further comments

I used minimalist approach as far as changing Bus* classes goes. Specific suggestions how to refactor these will be discussed in separate issues. See also #462.

BusSignalVoltageIn has zero size, so its residual vector is bound to an empty slice of system storage and cannot hold the current sums. They live in member variables that Ir() and Ii() expose and that the outlet ports are linked to, which is the same arrangement BusInfinite uses. The published sums are complete only after all attached components have evaluated their residuals, so consumers of those signals must be evaluated after them; this is documented in the header and README.

BusBase requires non-const Vr()/Vi() returning a mutable reference, and components read buses through non-const pointers. BusSignalVoltageIn delegates those overloads to the const versions and documents that callers must not write through them. Making the voltage accessors truly read-only would require changing BusBase and every component, which is deliberately left out of this PR.

Verified on macOS with GCC (Enzyme off) and Clang 18 (Enzyme on); all 25 PhasorDynamics unit tests pass in both configurations.

🤖 Generated with Claude Code

@pelesh pelesh added this to the DataBroker Co-Sim milestone Oct 2, 2026
@pelesh pelesh self-assigned this Oct 2, 2026
@pelesh pelesh added question Further information is requested new model labels Oct 2, 2026
@nkoukpaizan

Copy link
Copy Markdown
Collaborator

The approach looks reasonable to me. This is intended to replace BusToSignalAdapter, correct?

@pelesh

pelesh commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

The approach looks reasonable to me. This is intended to replace BusToSignalAdapter, correct?

Yes, that is correct.

@superwhiskers superwhiskers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

overall design looks good. just some minor comments

Comment thread GridKit/Model/PhasorDynamics/Bus/BusSignalVoltageIn/BusSignalVoltageIn.hpp Outdated
Comment thread GridKit/Model/PhasorDynamics/Bus/BusSignalVoltageOut/BusSignalVoltageOutImpl.hpp Outdated
Comment thread GridKit/Model/PhasorDynamics/SignalIn.hpp Outdated
@lukelowry

lukelowry commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Why not DependentVoltageSource, DependentCurrentSource, for these types of components? That would be more recognizable and physical

Also I have had more success with a basic cosim by interfacing across a Branch instead of a Bus, do we have a broader spec/plan for our cosim goals?

Co-authored-by: superwhiskers <whiskerdev@protonmail.com>
@pelesh

pelesh commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Why not DependentVoltageSource, DependentCurrentSource, for these types of components? That would be more recognizable and physical

These are not voltage/current dependant current/voltage sources. These simply convert variables to signals to communicate with other apps (GridKit-based or otherwise).

Also I have had more success with a basic cosim by interfacing across a Branch instead of a Bus, do we have a broader spec/plan for our cosim goals?

We need ports for the system to connect to other systems from external apps. A bus-like object seems more natural way to create such objects imho. See also discussion in #546.

@pelesh
pelesh marked this pull request as ready for review October 2, 2026 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new model question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement bus <--> signal node connections in phasor dynamics models

4 participants