Cost expression refactor, parameter broadcasting, and PF headroom slack porting by acostarelli · Pull Request #112 · Sienna-Platform/PowerOperationsModels.jl

acostarelli · 2026-05-06T19:33:30Z

Sienna-Platform/PowerSimulations.jl#1564
Sienna-Platform/PowerSimulations.jl#1609
Sienna-Platform/PowerSimulations.jl#1566

Requires IOM on Sienna-Platform/InfrastructureOptimizationModels.jl#97

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

This PR ports PowerSimulations.jl cost-expression refactors, parameter broadcasting fixes, and power-flow-in-the-loop (PFitL) functionality into PowerOperationsModels.jl, including headroom-proportional slack participation and expanded cost-expression reporting.

Changes:

Refactors device constructors to register cost expressions via a new add_cost_expressions! bundle and expands exported cost-expression types.
Reworks time-series parameter population/multiplier handling (including a regression fix for parallel-branch multipliers) using IOM fast-path setters.
Implements PFitL in PowerFlowsExt (input mapping, data update, solve/aux readback, headroom slack) and adds new regression/coverage tests.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/test_power_flow_in_the_loop.jl	Adds PFitL tests including headroom-proportional slack coverage and baseline PFitL scenarios.
test/test_parallel_branch_parameter_multipliers.jl	Adds regression test ensuring per-branch multiplier rows are populated (no NaNs) for parallel branches.
test/test_model_decision.jl	Updates expected expression counts and realized output export assertions for the expanded cost-expression set.
test/test_device_thermal_generation_constructors.jl	Expands thermal cost tests to validate constituent decomposition and startup-cost behavior.
test/test_device_renewable_generation_constructors.jl	Adds nonnegativity test for `CurtailmentCostExpression`.
test/Project.toml	Pins test dependency rev of InfrastructureOptimizationModels to the required branch.
src/static_injector_models/thermalgeneration_constructor.jl	Switches from individual `add_expressions!` calls to `add_cost_expressions!` for thermals.
src/static_injector_models/source_constructor.jl	Switches sources to `add_cost_expressions!`.
src/static_injector_models/renewablegeneration_constructor.jl	Switches renewables to `add_cost_expressions!`.
src/static_injector_models/renewable_generation.jl	Adds curtailment-cost hook in renewable objective building.
src/static_injector_models/load_constructor.jl	Switches loads to `add_cost_expressions!`.
src/static_injector_models/hydro_generation.jl	Uses IOM internal setters for faster parameter/multiplier writes in hydro parameter population.
src/PowerOperationsModels.jl	Adds explicit imports/exports for cost-expression bindings and includes new objective helper file.
src/core/interfaces.jl	Improves `add_power_flow_data!` fallback behavior and error message when PowerFlows extension isn’t loaded.
src/common_models/objective_function.jl	Introduces renewable curtailment-cost expression generation.
src/common_models/market_bid_plumbing.jl	Routes VOM costs into `VOMCostExpression` via updated IOM helper signature.
src/common_models/add_to_expression.jl	Refactors fuel-consumption expression construction (dispatch-based methods) and compact-formulation handling.
src/common_models/add_parameters.jl	Reworks time-series parameter/multiplier population via IOM fast setters and fixes parallel-branch multiplier axis bug.
src/common_models/add_expressions.jl	Adds `add_cost_expressions!` bundles and fuel-curve predicates for container setup decisions.
Project.toml	Pins main dependency rev of InfrastructureOptimizationModels to the required branch.
ext/PowerFlowsExt/PowerFlowsExt.jl	Sets up the PowerFlows extension module and includes PFitL implementation files.
ext/PowerFlowsExt/pf_input_mapping.jl	Implements PF input key mapping and aux-var container creation for PFitL.
ext/PowerFlowsExt/pf_data_update.jl	Implements writing optimization results into PF data/system for PFitL.
ext/PowerFlowsExt/pf_headroom.jl	Implements headroom-proportional slack participation factor recomputation.
ext/PowerFlowsExt/pf_solve_and_aux.jl	Implements PF solve dispatch and aux-variable readback from PF results.

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

github-actions · 2026-05-06T20:07:51Z

Performance Results

Version	Precompile Time
Main	2.968320492
This Branch	2.961960465

Version	Build Time
Main-Build Time Precompile	102.570013626
Main-Build Time Postcompile	13.586854927
This Branch-Build Time Precompile	102.880881618
This Branch-Build Time Postcompile	13.640438019

Version	Solve Time
Main-Solve Time Precompile	412.175637142
Main-Solve Time Postcompile	377.694636645
This Branch-Solve Time Precompile	172.60790733
This Branch-Solve Time Postcompile	136.232628488

acostarelli · 2026-05-07T14:17:16Z

Closes #106 and #109

luke-kiernan

Big picture things:

Just generally: before adding code to POM, ask yourself "is there device specific handling here?" If the answer is "no," it probably belongs better in IOM. And there may be a helper for it there already.
All the expression stuff is currently in POM, not in IOM, whether device-specific or generic. That's purely due to copy-pasting from PSI, not due to intentional organization choices. We should probably change that at some point and move the generic implementations into IOM.
Test coverage. I ran the tests locally and they pass, but with the level of complexity here, I wouldn't be surprised if some amount of the code isn't actually getting run. Lots of different multiple dispatch cases: e.g. _set_parameter_at with HDF5 storage.

luke-kiernan · 2026-05-07T17:43:41Z

    reduced_branch_tracker = get_reduced_branch_tracker(network_model)
    all_branch_maps_by_type = PNM.get_all_branch_maps_by_type(net_reduction_data)

    # TODO: Temporary workaround to get the name where we assume all the names are the same across devices.


Ick we should open an issue for this

luke-kiernan · 2026-05-07T18:07:09Z

+# rather than a runtime branch.
+
+# LinearCurve fuel: linear in the dispatch variable.
+function _add_fuel_consumption_term!(


Identical to IOM's add_proportional_cost_invariant!, except we skip the final step where we add to the objective function and to the ProductionCostExpression. Refactor?

- Rename solve_powerflow! → solve_power_flow! to match PowerFlows naming (kiernan). - Fix aux-var readback guard to compare AuxVarKey entry-type, not the whole key, against branch_aux_vars / bus_aux_vars; add a regression test covering an unsupported PowerFlowAuxVariable key. - Rename _get_variable_if_exists → _get_cost_if_exists so it no longer reads as an optimization-variable accessor. - Route LinearCurve fuel cost and renewable curtailment cost through the new IOM helpers add_cost_term_to_expression! and add_cost_term_invariant!; route compact-fuel SOS dispatch through IOM._determine_bin_lhs to drop the duplicated three-way branch. - Replace Ref(x) with singleton tuples (x,) in broadcast calls for stack allocation; drop unnecessary Float64(multiplier) casts. - Tighten devices argument on _add_time_series_parameters! to Union{Vector{D}, IS.FlattenIteratorWrapper{D}}. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

`findlast` returns `nothing` when no `PowerFlowEvaluationData` entry has solved, which previously bubbled up as an opaque `MethodError` from `datas[nothing]`. Raise an explicit error and mark the broader handling as a FIXME pending the PF-failure design discussion. Addresses kiernan's review comment on PR #112. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

luke-kiernan

Looks good. I'm noticing the following sequence of calls in a couple different places

power_units = PSY.get_power_units(var_cost)
proportional_term = PSY.get_proportional_term(value_curve)
prop_term_per_unit = get_proportional_cost_per_system_unit(
    proportional_term, power_units, base_power, device_base_power)
rate = prop_term_per_unit * dt # sometimes also * multipler.

but that's something we can address later

Replaces the removed PF-specific IOM API (AbstractPowerFlowEvaluationModel, solve_power_flow!, get_power_flow_data, get_power_flow_evaluation_data, reset_power_flow_is_solved!, is_from_power_flow, NetworkModel.power_flow_evaluation, OptimizationContainer.power_flow_evaluation_data) with IOM's generic AbstractEvaluator / AbstractEvaluationData / EvaluationContainer surface. PowerFlowEvaluationData now subtypes IOM.AbstractEvaluationData and implements IOM.evaluate!, IOM.reset!, IOM.is_solved, IOM.get_inner_data. add_power_flow_data! shares the EvaluationContainer instance between the NetworkModel and the OptimizationContainer so IOM's calculate_aux_variables! sees the runtime state POM registers. Adds power_flow_evaluations(ev) helper so call sites stay ergonomic: `evaluations = power_flow_evaluations(ACPowerFlow())`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Copilot

Pull request overview

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

luke-kiernan · 2026-06-01T14:55:13Z

+    has_container_key(container, ActivePowerTimeSeriesParameter, C) ||
+        return PSY.get_max_active_power(component)
+    param_container = get_parameter(container, ActivePowerTimeSeriesParameter, C)
+    multiplier = get_multiplier_array(param_container)


Good catch, yeah check this

+        for br in parallel_brs
+            sample_line = first(parallel_brs)
+            impedance = PSY.get_r(sample_line) + im * PSY.get_x(sample_line)
+            first_name = PSY.get_name(sample_line)


IOM #104 ("Redistribute operation models") de-power-ified IOM, leaving the power-flavoured problem taxonomy to POM. This renames POM's problem-type hierarchy and operations template to the explicit de-power-ified convention: PowerOperationModel -> AbstractPowerOperationProblem DecisionProblem -> AbstractPowerDecisionProblem EmulationProblem -> AbstractPowerEmulationProblem DefaultDecisionProblem -> DefaultPowerDecisionProblem DefaultEmulationProblem -> DefaultPowerEmulationProblem GenericOpProblem -> GenericPowerDecisionProblem GenericEmulationProblem -> GenericPowerEmulationProblem OperationsProblemTemplate -> PowerOperationsProblemTemplate Also: - Add the bare-system ArgumentError constructor stub for DefaultPowerDecisionProblem (a template is required). - Remove dead Simulation* test scaffolding that referenced IOM types removed by #104 (test_utils/run_simulation.jl and two unused mock helpers); mirrors IOM #104's own deletion of run_simulation.jl. POM precompiles and loads cleanly against the merged IOM main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

luke-kiernan

Mostly small things. The 2 that require attention/discussion: CoPilot's comment on _renewable_offer_max and sharing the EvaluationContainer.

luke-kiernan · 2026-06-01T14:37:55Z

 PrettyTables = "08abe8d2-0d0c-5749-adfa-8a2ac140af0d"

+[sources]
+PowerOperationsModels = {path = ".."}


Do we want this POM pin or is this a temporary testing thing? Not sure--would have to check other projects for how they handle the docs environment.

Testing thing will remove

luke-kiernan · 2026-06-01T14:44:54Z

 )
-    container.power_flow_evaluation_data = Vector{PowerFlowEvaluationData}()
-    sizehint!(container.power_flow_evaluation_data, length(evaluators))
+    evaluations = get_evaluations(network_model)


# Share the same EvaluationContainer instance between NetworkModel and # OptimizationContainer so IOM.calculate_aux_variables! sees the same # runtime state we register here.

Is this a bugfix or consequence of how we're restructuring things? Are these also shared in PSI?

luke-kiernan · 2026-06-01T14:55:13Z

+    has_container_key(container, ActivePowerTimeSeriesParameter, C) ||
+        return PSY.get_max_active_power(component)
+    param_container = get_parameter(container, ActivePowerTimeSeriesParameter, C)
+    multiplier = get_multiplier_array(param_container)


Good catch, yeah check this

luke-kiernan · 2026-06-01T14:57:19Z

+common single-evaluator case at call sites such as
+`NetworkModel(...; evaluations = power_flow_evaluations(ACPowerFlow()))`.
+"""
+function power_flow_evaluations(ev)


typeof(ev)

Switch to a type parameter, ev::T.

luke-kiernan · 2026-06-01T15:00:50Z

+`ProblemTemplate` (the common case). `DecisionModel{<:DefaultPowerDecisionProblem}`
+gets the generic template-driven `build_model!`/`validate_template` methods.
+"""
+abstract type DefaultPowerDecisionProblem <: AbstractPowerDecisionProblem end


nitpick on naming clarity: default sounds more specific than generic. So I'd be inclined to switch the order, have Generic as the supertype and Default as the instance.

Agreed the naming is confusing I'll see how that looks

luke-kiernan · 2026-06-01T15:03:30Z

    network_model.subnetworks = get_subnetworks(get_network_model(model.template))
-    # Initialization does not support PowerFlow evaluation - use empty vector
-    network_model.power_flow_evaluation = AbstractPowerFlowEvaluationModel[]
+    # Initialization does not run external evaluators


# Initialization does not run external evaluators

clarifying question: are there any internal evaluators? I don't think so, which makes the comment a bit deceptive

luke-kiernan · 2026-06-01T15:08:21Z

+    return DecisionModel{GenericPowerDecisionProblem}(template, sys, jump_model; kwargs...)
+end
+
+# Template-driven problems require a template; the bare-system constructor is an error.


Confusing comment imo. Just say "default decision problems require a AbstractProblemTemplate subtype."

(Are there other problems that don't require a template? If so, which?)

Yeah this comment confused me too

luke-kiernan · 2026-06-01T15:11:17Z

+
+# POM-side implementation of IOM's `validate_time_series!` extension point for
+# emulation problems. Emulation models solve a single step, so horizon == resolution.
+function validate_time_series!(model::EmulationModel{<:DefaultPowerEmulationProblem})


could combine with decision problem to cut down on code repetition

luke-kiernan · 2026-06-01T15:12:22Z

+        pf = sin(
+            atan((
+                PSY.get_max_reactive_power(d, PSY.SU) / PSY.get_max_active_power(d, PSY.SU)
+            )),


Has closed form via Pythagorean identities: if r is for reactive and a for active, I'm getting r/sqrt(r^2+a^2)

PSY6/IS4 made physical-quantity getters require an explicit unit-system argument; the bare 1-arg getters no longer exist. Append `, PSY.SU` (system base, matching the optimization's working units) across branch, HVDC, reserve, and transmission-interface getters. Notable fixes: - TwoTerminalVSC PQ-approx registration: broadcasted limit getters need the unit marker wrapped in `Ref(PSY.SU)`, since `SystemBaseUnit()` is a plain singleton struct and is not on Julia's broadcast scalar allow-list (without Ref, broadcast tries to iterate it -> `length(::SystemBaseUnit)` MethodError). - DynamicBranchRating multiplier / branch rating: generalize the parallel-group dispatch from PNM.BranchesParallel to PNM.AbstractBranchesParallel so the new PNM.MixedBranchesParallel is handled (it otherwise fell through to the PSY.ACTransmission method and called get_rating on the group itself), and route the multiplier through get_rating(d). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- _renewable_offer_max: guard against devices missing a time-series entry when the type-level ActivePowerTimeSeriesParameter container exists; fall back to static max_active_power. Add mixed-TS regression test. - electric_loads: compute power factor via closed form q/sqrt(q^2+p^2) (also avoids NaN when p == q == 0). - pf_solve_and_aux: hoist per-parallel-group invariants out of inner loop. - power_flow_evaluations: use a type parameter instead of typeof(ev). - Swap problem-type names: Generic* is now the abstract supertype, Default* the concrete struct. - Extract shared resolution reconciliation into _reconcile_resolution!, shared by DecisionModel/EmulationModel validate_time_series!. - decision_model: throw (not just construct) the no-template ArgumentError; clarify comment. - Clarify EvaluationContainer-sharing and initialization comments. - docs/Project.toml: drop temporary local POM path source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

luke-kiernan

Second pass: the get_rating name collision here is the big one

luke-kiernan · 2026-06-04T20:54:48Z

                IS.@assert_op T <: POM.BranchFlowAuxVariableType ||
                              (T == POM.PowerFlowBranchActivePowerLoss)
-                if !isapprox(PSY.get_r(br) + im * PSY.get_x(br), impedance)
+                if !isapprox(PSY.get_r(br, PSY.SU) + im * PSY.get_x(br, PSY.SU), impedance)


Still needed? With updating PSI to be compatible with network reductions, I think it might be fine: see compute_parallel_multiplier in PNM.

luke-kiernan · 2026-06-04T21:00:45Z

 ################################## Rate Limits constraint_infos ############################

-function get_rating(double_circuit::PNM.BranchesParallel)
+function get_rating(double_circuit::PNM.AbstractBranchesParallel)


nitpick: looks near-identical to PNM.get_sum_of_max_rating

luke-kiernan · 2026-06-04T21:35:55Z

+# A zero offer curve in SYSTEM_BASE. Used as the "absent" side when only one of the
+# incremental/decremental curves is meaningful: PSY requires both offer curves to share
+# a unit system, and the default `ZERO_OFFER_CURVE` is NATURAL_UNITS, so we can't leave
+# the other side defaulted when the meaningful side is SYSTEM_BASE. A zero value curve is


Should we go change the default in PSY? Could open an issue there to discuss--if most curves are in SU, then imo we should.

luke-kiernan · 2026-06-04T21:38:11Z

 InfrastructureSystems = {rev = "IS4", url = "https://github.com/Sienna-Platform/InfrastructureSystems.jl"}
 PowerSystems = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystems.jl"}
-InfrastructureOptimizationModels = {rev = "main", url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl"}
+InfrastructureOptimizationModels = {path = "InfrastructureOptimizationModels.jl"}


I'd be surprised if the CI passes with this environment: you probably need IOM, PF, and PNM pinned to the correct branches (and not to local paths).

luke-kiernan · 2026-06-04T22:05:27Z

 end

-function get_model(template::OperationsProblemTemplate, ::Type{T}) where {T <: PSY.Device}
+function get_model(


(Pre-existing issue.) Where do we use get_model? Between here and main branch of IOM, the only call sites I can find are in test/test_utils/model_checks.jl. Perhaps we need it for the eventual PSI that sits on top of POM and IOM?

luke-kiernan · 2026-06-04T22:17:01Z

+
+# Predicates for value-curve shape. Used to decide whether a FuelConsumptionExpression
+# container needs JuMP.QuadExpr storage (vs the cheaper GAE).
+_value_curve_is_quadratic(::PSY.LinearCurve) = false


Could just write

_value_curve_is_quadratic(::PSY.InputOutputCurve{QuadraticFunctionData}) = true _value_curve_is_quadratic(::PSY.InputOutputCurve{<:FunctionData}) = false

IS.FunctionData is mostly an internal type, though, so this is perhaps a little bit less-than-kosher.

luke-kiernan · 2026-06-04T22:42:13Z

 function get_rating(series_chain::PNM.BranchesSeries)
    return minimum([get_rating(segment) for segment in series_chain])
 end
 function get_rating(device::T) where {T <: PSY.ACTransmission}


I like the forced-function of having phased-out functions (like single-argument get_rating) error. I'd vote for re-naming this function to _get_rating: internal only.

Consider the following:

using PowerSimulations from PowerSimulations import get_rating using PowerSystems from PowerSystems import get_rating # set g equal to some generator from sys. set_units_base_system!(sys, "DEVICE_BASE") # pre-unitful PSY6: returns rating in device base, via PSY's single arg get_rating # post unitful PSY6: returns rating in system base, via PSI's single arg get_rating. r = get_rating(g)

Now, we're not exporting PSI's get_rating, nor importing PSY's get_rating. So you'd really have to try to trip upon this. But still, my vote would be to rename, to avoid confusion and bugs.

I asked Claude to search for similar name collisions: get_base_power is close, but PSI's 1-arg version only accepts OptimizationContainer and ModelStoreParams types.

Anthony Costarelli and others added 8 commits May 5, 2026 15:48

claude port

0ff1f94

formatting

70a4967

claude port

98ae53b

claude port

27cb611

ported more pf tests

060bbac

Merge ac/cost-expression-refactor

0ab2bf4

Merge ac/headroom-slack (incl. ac/fix-parameter-broadcasting)

9369016

Point IOM source to consolidated branch for combined PR

e517c2e

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

acostarelli requested a review from Copilot May 6, 2026 19:33

Copilot started reviewing on behalf of acostarelli May 6, 2026 19:34 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

Comment thread src/common_models/objective_function.jl Outdated

Comment thread ext/PowerFlowsExt/pf_solve_and_aux.jl Outdated

Comment thread ext/PowerFlowsExt/pf_solve_and_aux.jl Outdated

luke-kiernan reviewed May 7, 2026

View reviewed changes

Anthony Costarelli and others added 2 commits May 11, 2026 10:58

acostarelli requested review from jd-lara and luke-kiernan and removed request for jd-lara and luke-kiernan May 11, 2026 16:50

luke-kiernan approved these changes May 11, 2026

View reviewed changes

move bustype check inside loop

0ba657d

luke-kiernan mentioned this pull request May 12, 2026

Lk/flow sign trait #117

Open

This was linked to issues May 12, 2026

Move over PSI PR #1609 #106

Open

Implement power flow in-the-loop #109

Open

luke-kiernan mentioned this pull request May 26, 2026

Fix a scoping issue in _get_irreducible_buses_due_to_dlrs #113

Merged

Merge branch 'main' into ac/psi-costexp-parambroad-pfslack

0f4867d

jd-lara requested a review from Copilot May 28, 2026 23:24

Copilot started reviewing on behalf of jd-lara May 28, 2026 23:25 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

jd-lara and others added 3 commits May 29, 2026 08:25

WIP: try to adopt the units

d7809d8

update tomls to point to github; formatter

0582669

luke-kiernan requested changes Jun 1, 2026

View reviewed changes

Anthony Costarelli and others added 4 commits June 1, 2026 14:45

restore tests

12207f7

formatting

2075bf8

luke-kiernan reviewed Jun 4, 2026

View reviewed changes

Conversation

acostarelli commented May 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acostarelli commented May 7, 2026

Uh oh!

luke-kiernan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luke-kiernan left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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!

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!

github-actions Bot commented May 6, 2026 •

edited

Loading