network matrix consolidation by jd-lara · Pull Request #233 · Sienna-Platform/PowerOperationsModels.jl

jd-lara · 2026-08-12T18:23:54Z

This PR consolidated the use of a single network information entering the models. The purpose is to avoid having more than one factorization, using a more coherent interface for the reductions and simplifiying the exceptions code.

It will likley need refinements later

requires Sienna-Platform/InfrastructureOptimizationModels.jl#152

jd-lara

One thing to note is that I realized because Claude fucked up that there is a bug in the modeling of constant impedance loads, I'll leave that for a later PR

Introduce `AbstractNetworkSource` declarations (`NetworkReductionSpec`, `PrebuiltMatrixSource`, `PrebuiltCoreSource`) and derived network-data containers so every matrix a build uses comes from one Ybus, and therefore one reduction. Reduction exceptions move to `reduction_exceptions.jl` as one dispatched rule per pinning reason. Work in progress on the jd/network_matrix_consolidation branch, committed as the baseline for the zero-impedance reduction change that follows.

…nSpec `Ybus` applies a zero-impedance branch reduction to every network and takes its parameters through a dedicated kwarg, rejecting one passed inside `network_reductions`. POM never forwarded that kwarg, so the thresholds were not reachable from a NetworkModel and different applications could not pick the setting they need — while PowerFlows already exposed it, so the same reduction spec could yield different matrices in the two packages. Route the entry out of `NetworkReductionSpec`'s vector with PNM's shared `split_zero_impedance_reduction`, which PowerFlows now also calls. Requires PowerNetworkMatrices c251e1f on psy6. `_applied_reductions` now recovers the setting from an already-built reduction, so a prebuilt source carrying a non-default one reproduces instead of failing `_prebuilt_ybus`'s bus-map check. Tests assert the setting selects rather than merely toggling: on case11_network_reductions the default and a tolerance below the smallest branch resistance both retain all 11 buses, while 0.002 retains 7.

The source pinned a machine-local path that no longer exists, so the environment could not resolve. `main` is not a usable target either: the `AbstractNetworkSource` anchor this branch builds on is not on it. Pin the branch carrying that anchor (Sienna-Platform/InfrastructureOptimizationModels.jl#152) until it merges.

luke-kiernan

First pass. Again, didn't look at the tests or Claude files.

Why are some of these pieces here, not elsewhere? For example, we could move the definitions of both AbstractNetworkSource and NetworkReductionSpec into PNM. Then the rest of the network_sources.jl file could land there, too. So why don't we do that?

luke-kiernan · 2026-08-12T21:33:39Z

+sharing its reduction can be derived from it. A dense `PNM.PTDF` carries no core and
+is deliberately not a valid source.
+"""
+struct PrebuiltMatrixSource{M <: PNM.VirtualPTDF} <: IOM.AbstractNetworkSource


Why the type parameter here? VirtualPTDF doesn't have subtypes.

luke-kiernan · 2026-08-12T21:49:56Z

+end
+
+# The no-contingency constructors omit the field rather than storing `nothing`, so
+# `has_contingency_matrix` is a dispatched predicate on a concrete type.


Huh what....does this actually good sense? Can't tell.

From conversations with Claude:

"omit the field" is more like "omit the argument:" constructor ergonomics, not struct layout.

"so...is a dispatched predicate on a concrete type:" Nothing is a concrete type. Dispatching on ::DCPNetworkData{Nothing} would work just as well.

luke-kiernan · 2026-08-12T22:18:51Z

+PTDF and MODF wrappers are derived from it without re-factorizing the ABA matrix.
+"""
+struct PrebuiltCoreSource <: IOM.AbstractNetworkSource
+    core::PNM.VirtualFactorCore


Why not just pass around core::PNM.VirtualFactorCore directly? Why do we need the outer PrebuiltCoreSource wrapper?

I think so it can be defined here in POM.

luke-kiernan · 2026-08-12T22:22:04Z

+    return
+end
+
+# Fallback for monitored/outaged component types with no bus-pinning rule. Reached


Loooong comment.

m-bossart · 2026-08-12T18:48:31Z

+=#
+
+"""Ybus-only families: no factorization, no sensitivity matrices."""
+struct YbusNetworkData <: IOM.AbstractNetworkData


Why do we need to store both the matrix and the NetworkReductionData in these structs when the matrix always has the NetworkReductionData internally?

m-bossart · 2026-08-13T17:36:01Z

+
+"""PTDF families: a PTDF, plus an optional MODF, both wrapping one factorization core."""
+struct PTDFNetworkData{P, M} <: IOM.AbstractNetworkData
+    matrix::P


Shouldn't P be restricted here, and in the constructor below to VirtualPTDF because the PTDF does not have the factorization core

m-bossart · 2026-08-13T18:22:29Z

-    # check_network_reduction_compatibility(T)
-    #end
-    return
+    _consolidate_device_model_outages_with_modf!(branch_models, modf)


Do we still need this? It seems like it is unreachable now and can be deleted.

m-bossart · 2026-08-13T18:25:01Z

+PTDF and MODF wrappers are derived from it without re-factorizing the ABA matrix.
+"""
+struct PrebuiltCoreSource <: IOM.AbstractNetworkSource
+    core::PNM.VirtualFactorCore


I think so it can be defined here in POM.

m-bossart · 2026-08-13T18:33:17Z


 #################################################################################
-# AreaBalanceNetworkModel
+# Ybus-only families (ACP, ACR, IVR, LPACC, NFA, CopperPlate, AreaBalance, ...)


It seems like CopperPlate and AreaBalance should never apply a non-default network reduction. A reduction could in theory move an injector from one area to another based on the remapping. This might be a reason to keep a separate method for those two from the other Ybus only cases.

We do pin buses as irreducible if they have cross area boundaries in PNM, but only if there are AreaInterchanges in the system. Either way, I think we should have a separate method here and error if the user passes network reductions and tries to use copperplate. It doesn't buy anything and tells the user their inputs aren't being used.

CopperPlate and AreaBalance had dedicated `instantiate_network_model!` methods that built an unreduced Ybus. Consolidating onto the generic Ybus-only method dropped them, so a `network_source` handed to either is now applied — computed, then never consulted, because both resolve injections by area name or subnetwork reference bus rather than through the reduction's bus map. Only the explicit-source path changed; the default already applied a zero-impedance reduction on both sides of the change. Add an `honors_network_reduction` trait and reject a non-default source for the formulations that return false, rather than silently discarding the caller's input. The trait sits with the other per-formulation capability overrides; the guard runs in the shared validator, so it is a no-op wherever the trait holds. Also from review: - Parameterize `PrebuiltCoreSource{C <: PNM.VirtualFactorCore}`; the bare field was abstract, since `VirtualFactorCore` is itself parametric. - Call `PNM.get_applied_reductions` instead of reconstructing the spec here by reading PNM's fields by symbol. Requires PowerNetworkMatrices 20b488f on psy6. - Drop `_make_subnetworks_from_subnetwork_axes(::PNM.PTDF)`; no path reaches it with a dense PTDF. - Correct two comments: the no-contingency constructors omit the argument, not the field, and the `AreaInterchange` guard is reachable — `_pin_outage_buses!` iterates `get_associated_components` unfiltered. - Record that the stored reduction and the matrix's own copy diverge, since `populate_branch_maps_by_type!` only ever accumulates.

luke-kiernan · 2026-08-14T18:33:43Z

+
+Consequence: `PNM.get_network_reduction_data(matrix)` and `get_reduction(network_data)`
+are the same reduction but not the same object, and only the latter carries the branch
+maps. Read the reduction off the container, never off the matrix.


Consequence: PNM.get_network_reduction_data(matrix) and get_reduction(network_data)
are the same reduction but not the same object, and only the latter carries the branch
maps. Read the reduction off the container (2nd option), not the matrix (1st option).

Yikes that seems like a recipe for bugs and confusion. We have two objects of the same type, representing the same thing, that are different...imo that's a sign that there's something amiss with the design or usage of the object. Sounds like the network data's reduction only adds more stuff: go to 2 different structs, one containing the other?

jd-lara requested review from acostarelli, luke-kiernan, m-bossart and rodrigomha August 12, 2026 18:23

jd-lara commented Aug 12, 2026

View reviewed changes

jd-lara added 3 commits August 12, 2026 12:25

jd-lara force-pushed the jd/network_matrix_consolidation branch from f729f1e to e804b8e Compare August 12, 2026 21:47

luke-kiernan reviewed Aug 12, 2026

View reviewed changes

m-bossart reviewed Aug 13, 2026

View reviewed changes

luke-kiernan reviewed Aug 14, 2026

View reviewed changes

Uh oh!

Conversation

jd-lara commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jd-lara left a comment

Choose a reason for hiding this comment

Uh oh!

luke-kiernan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jd-lara commented Aug 12, 2026 •

edited

Loading