Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Yft, Ytf to power_network_matrix Ybus #95

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

rbolgaryn
Copy link

Thanks for opening a PR to PowerNetworkMatrices.jl, please take note of the following when making a PR:

Check the contributor guidelines

@rbolgaryn rbolgaryn self-assigned this Jan 14, 2025
@rbolgaryn rbolgaryn added the enhancement New feature or request label Jan 14, 2025
@rbolgaryn
Copy link
Author

Add branch admittance matrices (Yft, Ytf) to the Ybus structure so that we can efficiently calculate branch flows in PowerFlows in AC power flow calculation.

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Performance Results

Version Precompile Time
Main 2.554139919
This Branch 2.644233004
Version Execute Time
Main-Build PTDF First 6.485688918
Main-Build PTDF Second 0.090570388
Main-Build Ybus First 0.528275665
Main-Build Ybus Second 0.007346443
Main-Build LODF First 1.404140627
Main-Build LODF Second 0.209038586
This Branch-Build PTDF First 6.678554796
This Branch-Build PTDF Second 0.125437619
This Branch-Build Ybus First 0.640975128
This Branch-Build Ybus Second 0.006177156
This Branch-Build LODF First 1.502709977
This Branch-Build LODF Second 0.219641784

Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 84.74576% with 9 lines in your changes missing coverage. Please review.

Project coverage is 77.83%. Comparing base (7069dca) to head (3a48104).

Files with missing lines Patch % Lines
src/ybus_calculations.jl 84.74% 9 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #95      +/-   ##
==========================================
+ Coverage   77.61%   77.83%   +0.22%     
==========================================
  Files          14       14              
  Lines        1398     1430      +32     
==========================================
+ Hits         1085     1113      +28     
- Misses        313      317       +4     
Flag Coverage Δ
unittests 77.83% <84.74%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/ybus_calculations.jl 47.78% <84.74%> (+6.55%) ⬆️

Roman Bolgaryn and others added 2 commits January 14, 2025 16:28
formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link

@GabrielKS GabrielKS left a comment

Choose a reason for hiding this comment

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

A few minor things. I might not yet fully understand the use case. Also, this is missing tests.

src/ybus_calculations.jl Outdated Show resolved Hide resolved
src/ybus_calculations.jl Outdated Show resolved Hide resolved
src/ybus_calculations.jl Outdated Show resolved Hide resolved
Copy link
Member

@jd-lara jd-lara left a comment

Choose a reason for hiding this comment

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

These additional entries are in fact different Matrices. They should be separated into their own object and potentially store just one matrix but implement methods that allow the calculations to be done efficiently.

@rbolgaryn
Copy link
Author

This PR should be good now

Copy link
Member

@jd-lara jd-lara left a comment

Choose a reason for hiding this comment

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

Add a kwarg to the Ybus constructor to store the additional matrices and create nullable fields for the additional matrices and the branches index.

@rbolgaryn
Copy link
Author

rbolgaryn commented Jan 31, 2025

todo: 1) make also the ft, tb, sb, nullable (don"t populate those either), 2) update docstrings for Ybus struct

Copy link

@GabrielKS GabrielKS left a comment

Choose a reason for hiding this comment

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

Deferring to @jd-lara to evaluate the design on the whole

@@ -2,55 +2,85 @@
Nodal admittance matrix (Ybus) is an N x N matrix describing a power system with N buses. It represents the nodal admittance of the buses in a power system.

The Ybus Struct is indexed using the Bus Numbers, no need for them to be sequential

The fields yft and ytf are the branch admittance matrices for the from-to and to-from branch admittances respectively. They are indexed with the branch numbers and the bus numbers.
The branch numbers are sequential. The bus numbers are represented by fb, tb arraus of sequential bus indices (internal bus indices).

Choose a reason for hiding this comment

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

Suggested change
The branch numbers are sequential. The bus numbers are represented by fb, tb arraus of sequential bus indices (internal bus indices).
The branch numbers are sequential. The bus numbers are represented by fb, tb arrays of sequential bus indices (internal bus indices).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants