Skip to content

IOM-side port of PSI#1579 (SCUC branch formulation using MODF)#102

Open
acostarelli wants to merge 6 commits into
mainfrom
ac/scuc
Open

IOM-side port of PSI#1579 (SCUC branch formulation using MODF)#102
acostarelli wants to merge 6 commits into
mainfrom
ac/scuc

Conversation

@acostarelli
Copy link
Copy Markdown
Member

Bumps PNM compat to ^0.20 for VirtualMODF. Renames the NetworkModel LODF_matrix field to MODF_matrix (PNM.VirtualMODF). Adds an outages field/kwarg/getter on DeviceModel plus a _formulation_supports_outages trait that POM will specialize for AbstractSecurityConstrainedStaticBranch. Adds sparse-axis paths for post-contingency duals across dual_processing, optimization_container, decision_model_store, and add_constraint_dual. Extends Base.isempty/empty! on BranchReductionOptimizationTracker to cover constraint_map_by_type.

Bumps PNM compat to ^0.20 for VirtualMODF. Renames the NetworkModel
LODF_matrix field to MODF_matrix (PNM.VirtualMODF). Adds an outages
field/kwarg/getter on DeviceModel plus a _formulation_supports_outages
trait that POM will specialize for AbstractSecurityConstrainedStaticBranch.
Adds sparse-axis paths for post-contingency duals across dual_processing,
optimization_container, decision_model_store, and add_constraint_dual.
Extends Base.isempty/empty! on BranchReductionOptimizationTracker to
cover constraint_map_by_type.

PSI's power-specific changes (devices_models, network_models,
services_models, contingency_model, template_validation, etc.) remain
out of scope here and will follow in POM.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

Performance Results
Main


This branch


Adds four MODF helpers that PSI placed in core/network_model.jl but are
PSY-free, so they belong in IOM rather than POM:

- _build_network_reductions: pure PNM, drop-in.
- _validate_provided_modf_reduction!: pure PNM, drop-in.
- _template_has_outage_aware_branch: recast from PSI's
  _template_uses_security_constrained_branch to dispatch on the
  _formulation_supports_outages trait (already in IOM) rather than the
  POM-resident AbstractSecurityConstrainedStaticBranch type.
- _consolidate_device_model_outages_with_modf!: same recast; name kept.

The POM port will call these via IOM.* directly. The remaining new
helpers in PSI's network_model.jl (irreducible-bus discovery for
monitored components, instantiate_network_model! updates) stay POM-bound
because they need PSY.System / PSY.Branch / PSY.has_time_series / etc.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Member Author

@acostarelli acostarelli left a comment

Choose a reason for hiding this comment

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

See the original PR: Sienna-Platform/PowerSimulations.jl#1579 .

There were more changes here that were left out of this port. Please bring in the rest that are relevant to IOM.

Comment thread src/common_models/add_constraint_dual.jl Outdated
Comment thread src/core/device_model.jl
Comment thread src/core/device_model.jl Outdated
Comment thread src/core/optimization_container.jl Outdated
Comment thread src/core/network_model.jl
Comment thread src/core/device_model.jl Outdated
Anthony Costarelli and others added 2 commits May 25, 2026 12:29
Review comments:
- Rename `_formulation_supports_outages` → `supports_outages` and export it,
  matching the sibling `supports_branch_filtering` / `requires_all_branch_models`
  pattern. The leading underscore was misleading for a downstream extension point.
- Document why `DeviceModel.outages` is keyed by UUID (mirrors PNM's
  `get_registered_contingencies` so the consolidation set-diff is direct).
- Replace `existing isa SparseAxisArray` in `assign_dual_variable!` with dispatch
  on new `_assign_dual_from_existing!` methods.
- Collapse the `isa SparseAxisArray` branch in `_calculate_dual_variable_value!` —
  both `_copy_dual_values!` methods (Dense and Sparse) already exist in
  dual_processing.jl, so dispatch handles it.
- Add a docstring callout pointing to where `BranchModelContainer` is defined.

Missing port chunks:
- `should_write_resulting_value(::Type{PostContingencyBranchFlow}) = true` so
  sparse post-contingency flows actually persist to the store.
- Style sweep: `=== nothing` → `isnothing(...)` across 6 files (23 lines) matching
  PSI#1579.

The `if !supports_outages(B)` branch in `_add_device_model_outages` is kept as-is
(reviewer asked for full dispatch, but with the rename + export the trait now
reads as a documented extension point rather than an "extra helper"). Will reply
on the thread.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@acostarelli acostarelli requested a review from Copilot May 25, 2026 20:21
@acostarelli acostarelli changed the title [Do not review yet] IOM-side port of PSI#1579 (SCUC branch formulation using MODF) IOM-side port of PSI#1579 (SCUC branch formulation using MODF) May 25, 2026
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

IOM-side scaffolding port for the PSI#1579 SCUC branch formulation that uses PNM's new VirtualMODF. Bumps PowerNetworkMatrices to ^0.20, renames the NetworkModel LODF_matrix field/kwarg/getter to MODF_matrix, threads a new outages field/kwarg/getter through DeviceModel (plus a supports_outages trait specialized downstream by POM), and adds sparse-axis-array handling paths so post-contingency duals/expressions flow through dual_processing, optimization_container, decision_model_store, and add_constraint_dual. The bulk of consumers (POM's AbstractSecurityConstrainedStaticBranch) lives downstream.

Changes:

  • Rename LODF_matrixMODF_matrix on NetworkModel, add MODF helpers (_build_network_reductions, _validate_provided_modf_reduction!, _template_has_outage_aware_branch, _consolidate_device_model_outages_with_modf!) and import/export VirtualMODF.
  • Add outages field, kwarg, accessor, and supports_outages trait on DeviceModel; warn on outages with unsupported formulations.
  • Add SparseAxisArray paths in _copy_dual_values!, _assign_dual_from_existing!, write_output!, plus BranchReductionOptimizationTracker isempty/empty! over constraint_map_by_type; minor === nothingisnothing(...) cleanups.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Project.toml Bumps PowerNetworkMatrices compat to ^0.20 for VirtualMODF.
src/InfrastructureOptimizationModels.jl Imports/exports VirtualMODF; renames get_LODF_matrixget_MODF_matrix; exports get_outages, supports_outages.
src/core/network_model.jl Renames LODF→MODF on the struct/kwarg/getter; adds MODF reduction/validation/outage-consolidation helpers and _template_has_outage_aware_branch.
src/core/device_model.jl Adds outages::Dict{UUID, Dict{DataType, Set{String}}} field, outages kwarg, _add_device_model_outages, get_outages, and supports_outages trait default.
src/core/dual_processing.jl Factors dual copying into _copy_dual_values! with Dense and Sparse axis-array methods.
src/core/optimization_container.jl Routes dual calculation through _copy_dual_values!; switches === nothing checks to isnothing(...).
src/core/standard_variables_expressions.jl Marks PostContingencyBranchFlow as a written result expression.
src/core/network_reductions.jl Extends isempty/empty! on BranchReductionOptimizationTracker to include constraint_map_by_type; idiom cleanup.
src/common_models/add_constraint_dual.jl Splits dual allocation into Dense/Sparse _assign_dual_from_existing! methods so sparse constraints get mirrored sparse dual containers.
src/operation/decision_model_store.jl Adds a write_output! method for SparseAxisArray that flattens non-time keys to encoded columns and stores as DenseAxisArray.
src/operation/operation_model_interface.jl === nothingisnothing(...) cleanups.
src/operation/emulation_model.jl === nothingisnothing(...) cleanup.
src/operation/decision_model.jl === nothingisnothing(...) cleanup.
src/operation/store_common.jl !== nothing!isnothing(...) cleanups across exporters.

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

Comment thread src/core/network_model.jl
Comment on lines 63 to +110
@@ -74,7 +77,7 @@ mutable struct NetworkModel{T <: AbstractPowerModel}
::Type{T};
use_slacks = false,
PTDF_matrix = nothing,
LODF_matrix = nothing,
MODF_matrix = nothing,
reduce_radial_branches = false,
reduce_degree_two_branches = false,
subnetworks = Dict{Int, Set{Int}}(),
@@ -86,7 +89,7 @@ mutable struct NetworkModel{T <: AbstractPowerModel}
new{T}(
use_slacks,
PTDF_matrix,
LODF_matrix,
MODF_matrix,
subnetworks,
Dict{IS.InfrastructureSystemsComponent, Int}(),
duals,
@@ -104,7 +107,7 @@ end

get_use_slacks(m::NetworkModel) = m.use_slacks
get_PTDF_matrix(m::NetworkModel) = m.PTDF_matrix
get_LODF_matrix(m::NetworkModel) = m.LODF_matrix
get_MODF_matrix(m::NetworkModel) = m.MODF_matrix
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.

Not important

Comment thread src/core/network_model.jl
use_slacks::Bool
PTDF_matrix::Union{Nothing, PNM.PowerNetworkMatrix}
LODF_matrix::Union{Nothing, PNM.PowerNetworkMatrix}
MODF_matrix::Union{Nothing, PNM.VirtualMODF}
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.

Ignoring for now.

Comment thread src/core/network_model.jl Outdated
Comment on lines +95 to +106
function _assign_dual_from_existing!(
container::OptimizationContainer,
key::ConstraintKey,
existing::SparseAxisArray,
::Type{D},
time_steps,
) where {D}
dual_container =
SparseAxisArray(Dict(k => zero(Float64) for k in keys(existing.data)))
_assign_container!(container.duals, key, dual_container)
return
end
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.

I don't mind this personally.

Comment on lines +134 to +148
function write_output!(
store::DecisionModelStore,
name::Symbol,
key::OptimizationContainerKey,
index::DecisionModelIndexType,
update_timestamp::Dates.DateTime,
array::SparseAxisArray{T},
) where {T}
columns = get_column_names_from_axis_array(array)[1]
matrix = to_matrix(array)
container = getfield(store, get_store_container_type(key))
container[key][index] =
DenseAxisArray(permutedims(matrix), columns, 1:size(matrix, 1))
return
end
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.

@copilot Does the most recent commit fix this?

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.

Yes — b0457e0 addresses it. The sparse write path now computes tuple_columns once, uses that ordering for _to_matrix, and then derives encoded column names from the same tuple list, so labels stay aligned with matrix values.

@acostarelli acostarelli marked this pull request as ready for review May 27, 2026 19:09
@acostarelli acostarelli requested a review from jd-lara May 27, 2026 19:09
Copy link
Copy Markdown
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.

The outages part isn't ideal. We can merge to continue the work but we need an issue to figure out how to remove that specific part

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

Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.

acostarelli and others added 2 commits May 27, 2026 20:59
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@acostarelli
Copy link
Copy Markdown
Member Author

#106

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.

5 participants