Cost expression refactor, parameter broadcasting, and PF headroom slack porting#112
Cost expression refactor, parameter broadcasting, and PF headroom slack porting#112acostarelli wants to merge 11 commits into
Conversation
Co-Authored-By: Claude Opus 4.7 <[email protected]>
There was a problem hiding this comment.
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.
| for t in get_time_steps(container) | ||
| offer_max = _renewable_offer_max(container, component, name, t) | ||
| dispatch = dispatch_vars[name, t] | ||
| curtailment_cost = proportional_term_per_unit * dt * (offer_max - dispatch) |
There was a problem hiding this comment.
@m-bossart This was also missing from your PR, is this intentional?
|
Performance Results
|
luke-kiernan
left a comment
There was a problem hiding this comment.
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_atwith HDF5 storage.
| 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. |
There was a problem hiding this comment.
Ick we should open an issue for this
| if sos_status == SOSStatusVariable.NO_VARIABLE | ||
| JuMP.add_to_expression!(expression[name, t], P_min * prop_term_per_unit * dt) | ||
| elseif sos_status == SOSStatusVariable.PARAMETER | ||
| param = get_default_on_parameter(d) | ||
| bin = get_parameter(container, param, V).parameter_array[name, t] | ||
| JuMP.add_to_expression!( | ||
| expression[name, t], P_min * prop_term_per_unit * dt, bin) | ||
| elseif sos_status == SOSStatusVariable.VARIABLE | ||
| var = get_default_on_variable(d) | ||
| bin = get_variable(container, var, V)[name, t] | ||
| JuMP.add_to_expression!( | ||
| expression[name, t], P_min * prop_term_per_unit * dt, bin) | ||
| else | ||
| @assert false | ||
| end |
There was a problem hiding this comment.
Call _determine_bin_lhs in IOM instead: up to hard-coded var = OnVariable there vs var = get_default_on_variable(d) here, this is
bin = _determine_bin_lhs(container, sos_status, V, name, t)
JuMP.add_to_expression!(expression[name, t], P_min * prop_term_per_unit * dt, bin)| # rather than a runtime branch. | ||
|
|
||
| # LinearCurve fuel: linear in the dispatch variable. | ||
| function _add_fuel_consumption_term!( |
There was a problem hiding this comment.
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 <[email protected]>
`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 <[email protected]>
luke-kiernan
left a comment
There was a problem hiding this comment.
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
Sienna-Platform/PowerSimulations.jl#1564
Sienna-Platform/PowerSimulations.jl#1609
Sienna-Platform/PowerSimulations.jl#1566
Requires IOM on Sienna-Platform/InfrastructureOptimizationModels.jl#97