Skip to content

Jd/pnm pf update#1619

Merged
jd-lara merged 19 commits into
mainfrom
jd/pnm_pf_update
Jun 7, 2026
Merged

Jd/pnm pf update#1619
jd-lara merged 19 commits into
mainfrom
jd/pnm_pf_update

Conversation

@jd-lara

@jd-lara jd-lara commented May 30, 2026

Copy link
Copy Markdown
Member

This PR updates the logic and the versions for PNM and PFs

Copilot AI 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.

Pull request overview

This PR updates PowerNetworkMatrices/PowerFlows compatibility and adapts network reduction construction to the PNM 0.22 API, where irreducible buses are supplied to matrix constructors rather than reduction specs.

Changes:

  • Updates dependency bounds for PowerFlows and PowerNetworkMatrices.
  • Refactors PTDF/MODF construction and reconciliation into helper functions.
  • Threads irreducible_buses through Ybus, VirtualPTDF, and VirtualMODF constructors.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
Project.toml Bumps PowerFlows and PowerNetworkMatrices compatibility versions.
src/core/network_model.jl Updates matrix reduction construction and PTDF/MODF consistency handling for PNM 0.22.

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

@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Performance Results

Version Precompile Time
Main 4.015227968
This Branch 3.941622487
Version Build Time
Main-Build Time Precompile 128.344117189
Main-Build Time Postcompile 1.842040759
This Branch-Build Time Precompile 133.035351051
This Branch-Build Time Postcompile 1.832313978
Version Build Time
Main-Solve Time Precompile 349.382837946
Main-Solve Time Postcompile 305.900130706
This Branch-Solve Time Precompile 3623.732303072
This Branch-Solve Time Postcompile 3566.697487472

jd-lara and others added 4 commits May 31, 2026 10:08
…n model construction

DecisionModel deepcopied the caller's template, which bit-copied the raw libklu factorization Ptr held by a user-provided VirtualPTDF/VirtualMODF. The copy aliased the original's handle while the cleanup finalizer stayed attached only to the original; when GC reclaimed the original template during build!, the finalizer freed the handle out from under the copy. The next klu_solve then read freed memory (Numeric.n == 0) -> KLU_INVALID -> SIGSEGV. Linux crashed deterministically; macOS survived on heap-layout luck. Exposed by PNM 0.22, whose Virtual matrices carry the KLU factorization through a finalizer-guarded Ptr.

Add _copy_template_for_build: a deepcopy that shares PTDF_matrix/MODF_matrix by reference (by seeding the deepcopy memo) so the factorization handle is never copied, while still deep-copying the device/branch/service models and network reduction so the model can mutate them independently. Use it in DecisionModel (replacing the unsafe deepcopy) and EmulationModel (which previously mutated the caller's template in place — the same contract violation).

https://claude.ai/code/session_01KNFhS4Z85haH3HZFT8VJC4
@jd-lara jd-lara force-pushed the jd/pnm_pf_update branch from f642037 to 88b2806 Compare May 31, 2026 10:47
claude and others added 5 commits May 31, 2026 11:21
A failure on one platform (currently macOS, which runs Julia x64 under Rosetta on the arm64 runner) was cancelling the Linux and Windows jobs before they could report. Disabling fail-fast gives a clean per-platform verdict so the Linux/KLU result is visible while the macOS failure is investigated separately.

https://claude.ai/code/session_01KNFhS4Z85haH3HZFT8VJC4

Copilot AI 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.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/operation/problem_template.jl

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jd-lara jd-lara requested a review from Copilot June 2, 2026 15:12

Copilot AI 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.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.

Comment thread src/core/network_model.jl
Comment on lines +676 to 680
# Both matrices must share one reduction so the nodal-balance rows and the
# post-contingency MODF columns line up; reconcile (and fail fast) before
# outage consolidation populates the branch maps.
_ensure_ptdf_modf_consistent!(model, sys)
_consolidate_device_model_outages_with_modf!(branch_models, get_MODF_matrix(model))

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.

This is correct. I will push a different fix for this.

Comment on lines 2199 to 2201
for (device_type, devices) in contributing_devices_map
device_model = get(devices_template, Symbol(device_type), nothing)
device_model = get(devices_template, nameof(device_type), nothing)
isnothing(device_model) && continue
Comment on lines +2078 to 2081
"An interface is specified with only part of a double-circuit that has been reduced.
Branches: $(branch_names[in_interface]) are in the interface and branches: $(branch_names[.!in_interface]) are not.
Modify the data to include all of or none of the parallel segements.",
),
Comment thread src/core/network_model.jl Outdated
name: Julia ${{ matrix.julia-version }} - ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false

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.

Just confirming, is this meant to be committed or just for debugging?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I want to keep it because if it fails in one OS throws all the CI out

Comment thread src/core/network_model.jl
Comment on lines +676 to 680
# Both matrices must share one reduction so the nodal-balance rows and the
# post-contingency MODF columns line up; reconcile (and fail fast) before
# outage consolidation populates the branch maps.
_ensure_ptdf_modf_consistent!(model, sys)
_consolidate_device_model_outages_with_modf!(branch_models, get_MODF_matrix(model))

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.

This is correct. I will push a different fix for this.

@jd-lara jd-lara requested a review from m-bossart June 4, 2026 15:04

Copilot AI 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.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Comment thread src/core/network_model.jl
Comment on lines +596 to 604
function _set_subnetworks_from_ptdf!(
model::NetworkModel{<:AbstractPTDFModel},
sys::PSY.System,
)
model.subnetworks = _make_subnetworks_from_subnetwork_axes(model.PTDF_matrix)
if length(model.subnetworks) > 1
@debug "System Contains Multiple Subnetworks. Assigning buses to subnetworks."
_assign_subnetworks_to_buses(model, sys)
end

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.

I think this is right, we should only overwrite the subnetworks if the user input is empty.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

users should never provide subnetworks here. It always needs to be derived from the data

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed via 6995326

Comment thread src/core/network_model.jl
@jd-lara jd-lara merged commit 5ffb2a5 into main Jun 7, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants