Skip to content

BranchBase Class for Other Branch Types#377

Closed
lukelowry wants to merge 3 commits intodevelopfrom
lukel/branchbase-dev
Closed

BranchBase Class for Other Branch Types#377
lukelowry wants to merge 3 commits intodevelopfrom
lukel/branchbase-dev

Conversation

@lukelowry
Copy link
Copy Markdown
Collaborator

@lukelowry lukelowry commented Apr 20, 2026

Description

Makes a BranchBase component family class so we can share boilerplate and equations for network construction across different transformer and line models.

There is a related open issue that I cannot find, but in an ideal world, the branch should own the line's current states and the residual injection equations. The Bus should own the voltage states and have one continuity equation (two in phasor dynamics, for real and imag):

Bus (owns voltage states)

$$0=\sum_{k=0}^K i_{mk}$$

Branch (From and To Currents) for pi model:

$$\begin{aligned} 0 &=i_{mk} - v_{k} * y_{mk}/2 \\ 0 &=i_{km}- v_{m} * y_{mk}/2 \end{aligned}$$

That is the next step after this PR, changing the ownership of the current is a big change.

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.
  • I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

None

@lukelowry lukelowry requested review from nkoukpaizan and pelesh April 20, 2026 17:54
@PhilipFackler PhilipFackler self-requested a review April 24, 2026 19:16
Copy link
Copy Markdown
Collaborator

@PhilipFackler PhilipFackler left a comment

Choose a reason for hiding this comment

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

I think we should reduce the scope of this PR. The new base class with the factory and JSON parser should be included here. That can be done without changing the syntax for using the branch. That is, the changes relating to "bus" vs "port" and how V and I are accessed should be in a separate PR. They deserve their own discussion.

EDIT: I should add that I like the architectural changes :)

Copy link
Copy Markdown
Collaborator

@pelesh pelesh left a comment

Choose a reason for hiding this comment

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

I agree that we need to manage the scope of this PR a little better. I suggest creating an issue to discuss the proposed changes there first, followed by documentation-only PR.

I would also suggest creating a transformer model from branch using copy-paste-tweak approach. That would give us needed component for system modeling and would also provide better clues how a base class branch should look like.

Architectural changes and new models are different topics and should be handled in separate issues/PRs imho.

@lukelowry
Copy link
Copy Markdown
Collaborator Author

@pelesh @PhilipFackler thank you both for the feedback. I will close this PR I agree with the scope issue. I will start with a more practical implementation with the Transformer instead of working backward. I hope you see the vision, however!

@lukelowry lukelowry closed this Apr 24, 2026
@lukelowry
Copy link
Copy Markdown
Collaborator Author

Close

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.

3 participants