Skip to content

call _get_shunts once not twice#306

Closed
luke-kiernan wants to merge 1 commit into
mainfrom
lk/ybus-get-shunts
Closed

call _get_shunts once not twice#306
luke-kiernan wants to merge 1 commit into
mainfrom
lk/ybus-get-shunts

Conversation

@luke-kiernan
Copy link
Copy Markdown
Collaborator

@luke-kiernan luke-kiernan commented May 20, 2026

I ran PProf on assembling the y bus matrix for unrelated reasons and the total time inside _get_shunts was surprisingly high. Fetching the values for both "from" and "to" sides in a single call helps.

Aside: I suspect the cost is primarily due to the type instability in the getters, which will be resolved by my unit fixes.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes Y-bus branch entry computation by avoiding repeated shunt getter calls during matrix assembly, reducing time spent in shunt retrieval on profiling.

Changes:

  • Replace per-side _get_shunt(br, node) calls with a single _get_shunts(br) call returning both end shunts.
  • Cache get_g(br) / get_b(br) results within _get_shunts to avoid repeated getter overhead.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Ybus.jl
Comment on lines 447 to +449
Y12 = -Y_l
Y21 = Y12
Y22 = Y_l + _get_shunt(br, :to)
Y22 = Y_l + shunt_to
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pre-existing issue. But yeah, it is strange that we only check Y11 and Y_l, not Y22. @m-bossart is there a reason for this?

@github-actions
Copy link
Copy Markdown
Contributor

Performance Results

Precompile Time

Main This Branch Delta
2.162 s 2.107 s -2.5%

Execution Time

Test Main This Branch Delta
matpower_ACTIVSg2000_sys-Build PTDF First 1.746 s 1.743 s -0.2%
matpower_ACTIVSg2000_sys-Build PTDF Second 96.2 ms 71.3 ms -25.9%
matpower_ACTIVSg2000_sys-Build Ybus First 13.8 ms 116.2 ms +743.0%
matpower_ACTIVSg2000_sys-Build Ybus Second 13.2 ms 11.0 ms -16.6%
matpower_ACTIVSg2000_sys-Build LODF First 614.1 ms 527.4 ms -14.1%
matpower_ACTIVSg2000_sys-Build LODF Second 170.1 ms 156.3 ms -8.1%
matpower_ACTIVSg2000_sys-Build VirtualMODF First 3.78 s 4.965 s +31.3%
matpower_ACTIVSg2000_sys-Build VirtualMODF Second 201.7 ms 712.0 ms +253.1%
matpower_ACTIVSg2000_sys-VirtualMODF Query 10 rows 502.6 ms 496.1 ms -1.3%
matpower_ACTIVSg2000_sys-Radial network reduction First 470.3 ms 449.9 ms -4.3%
matpower_ACTIVSg2000_sys-Radial network reduction Second 0.7 ms 0.7 ms +5.5%
matpower_ACTIVSg2000_sys-Degree two network reduction First 1.852 s 1.74 s -6.0%
matpower_ACTIVSg2000_sys-Degree two network reduction Second 1.0 ms 0.9 ms -13.4%
Base_Eastern_Interconnect_515GW-Build Ybus First 3.707 s 3.785 s +2.1%
Base_Eastern_Interconnect_515GW-Build Ybus Second 3.539 s 3.426 s -3.2%
Base_Eastern_Interconnect_515GW-Radial network reduction First 35.6 ms 31.7 ms -10.9%
Base_Eastern_Interconnect_515GW-Radial network reduction Second 38.9 ms 32.5 ms -16.6%
Base_Eastern_Interconnect_515GW-Degree two network reduction First 378.6 ms 362.2 ms -4.3%
Base_Eastern_Interconnect_515GW-Degree two network reduction Second 42.2 ms 33.0 ms -21.7%

@luke-kiernan
Copy link
Copy Markdown
Collaborator Author

Ok the perf measurements aren't bearing this out--I took Claude's word for it--so I'm going to close this. The root problem is the type instability in the getters, which will go away after the units fixes anyway

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.

2 participants