Update POM for PSY MarketBidCost/ImportExportCost static/TS split#77
Update POM for PSY MarketBidCost/ImportExportCost static/TS split#77luke-kiernan wants to merge 12 commits intomainfrom
Conversation
- Port serialize_problem from IOM (removed there in deduplicate-opt-container) - Adapt to _check_branch_network_compatibility signature change (3-arg to 2-arg) - Remove system_to_file kwarg from tests (removed from IOM Settings) - Trim deserialization test (deserialize_problem not yet ported) - Bump PNM compat to ^0.19 to match IOM - Point test env at local IOM Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…serialization (restored in IOM);
PSY split MarketBidCost and ImportExportCost into separate static and time-series-backed types (MarketBidCost + MarketBidTimeSeriesCost, ImportExportCost + ImportExportTimeSeriesCost). This mirrors the IOM update (PR #74) on the POM side. Dispatch changes: - Consolidate MBC OnVariable proportional_cost + is_time_variant_term into a single generic pair in market_bid_overrides.jl, keyed on an _onvar_offer_direction trait (Generator → IncrementalOffer, ControllableLoad → DecrementalOffer). Per-device thermal/hydro/load overrides drop out. - Replace stale `isnothing(get_{input,output}_offer_curves(...))` checks with IOM.is_nontrivial_offer (ZERO_OFFER_CURVE is now the default, not nothing). Load dispatch rejects non-trivial supply with ArgumentError. - Delegate hydro's add_proportional_cost! to IOM's shared add_proportional_cost_maybe_time_variant! (drops a broken _lookup_maybe_time_variant_param path). - Collapse is_time_variant_term / Type-based signatures to match IOM. - _get_time_series_name for *CostAtMinParameter traverses through the curve (initial_input now lives inside TimeSeriesPiecewiseIncrementalCurve, not on MBC directly). Test utilities rewritten for the new types. Existing MBC/IEC test helpers (add_market_bid_cost.jl, mbc_system_utils.jl, iec_simulation_utils.jl) updated to use LinearCurve fields and the new TS cost constructors. New unit-test suite for MBC/IEC objective-function construction: - test/test_market_bid_cost.jl: Load (PowerLoadDispatch, PowerLoadInterruption) and Thermal (BasicUC, MultiStartUC) with static and TS costs. One testset per (device × formulation × cost-type); dedicated scaling testsets cover dt / unit conversion. - test/test_import_export_cost.jl: Source + ImportExportSourceModel with static and TS IEC, distinct import/export breakpoints. - test/test_utils/mbc_math_helpers.jl: minimal system builders, OptimizationContainer setup, JuMP variable seeding, objective coefficient / PWL delta coefficient / PWL delta width inspection, stub TS curve/MBC construction, parameter seeding helpers. Flagged FIXME: Source IEC dispatch always adds PWL for both directions (previous isnothing guard is dead; a one-directional Source would benefit from a cheap TS-curve emptiness check once available). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Clarification: first 4 commits are from #74. The actual changes start at |
There was a problem hiding this comment.
Pull request overview
Updates PowerOperationsModels (POM) to match PowerSystems (PSY) splitting MarketBidCost / ImportExportCost into separate static vs time-series cost types, and adds focused unit tests that assert the resulting objective-function coefficients/constraints are wired correctly.
Changes:
- Refactors MBC/IEC dispatch and PWL helpers to use the new static/TS cost-type split (and new IOM delta/lambda PWL APIs).
- Updates test utilities for new curve scalar-field types and introduces new unit test suites for MBC/IEC objective construction.
- Adds network-model branch compatibility validation using the updated IOM compatibility check signature.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/common_models/market_bid_overrides.jl |
Consolidates MBC OnVariable dispatch via offer-direction trait; updates IEC handling and delta-PWL helper naming. |
src/common_models/add_parameters.jl |
Updates time-series name discovery for shutdown and cost-at-min parameters to traverse through the new curve structures. |
src/static_injector_models/thermal_generation.jl |
Removes per-device MBC proportional-cost overrides and switches PWL helper naming for ThermalDispatchNoMin. |
src/static_injector_models/hydro_generation.jl |
Delegates hydro proportional-cost handling to shared IOM helper; updates time-variance trait. |
src/static_injector_models/electric_loads.jl |
Removes load-specific MBC proportional-cost override and updates time-variance trait. |
src/energy_storage_models/storage_models.jl |
Broadens MBC dispatch to accept static+TS MBC types and switches to delta-PWL helper name. |
src/services_models/reserves.jl |
Switches reserve variable-cost PWL term construction to the delta-PWL API. |
src/network_models/instantiate_network_model.jl |
Adds a pre-validation step that computes unmodeled AC-transmission branch types and checks network compatibility. |
src/core/problem_template.jl |
Updates template model-setter calls to the renamed IOM set_model! API. |
src/PowerOperationsModels.jl |
Adjusts imported IOM symbols to match new delta-PWL APIs and template setters. |
Project.toml |
Removes IOM [sources] override (now relying on registry resolution/compat). |
test/Project.toml |
Removes IOM [sources] override in the test environment. |
test/includes.jl |
Adds inclusion of new MBC/IEC math helper utilities. |
test/test_utils/mbc_math_helpers.jl |
New helper module to build minimal systems/containers and inspect objective/PWL coefficients. |
test/test_market_bid_cost.jl |
New unit tests covering static + TS MBC objective wiring across load/thermal formulations, including scaling. |
test/test_import_export_cost.jl |
New unit tests covering static + TS IEC objective wiring and breakpoint-width assertions. |
test/test_utils/add_market_bid_cost.jl |
Updates MBC construction/extension helpers for LinearCurve scalar fields and TS cost constructors. |
test/test_utils/mbc_system_utils.jl |
Updates system utilities for LinearCurve fields and adds helpers to promote static MBC → TS MBC. |
test/test_utils/iec_simulation_utils.jl |
Updates IEC simulation helpers to use TS IEC getters and type unions. |
test/test_device_load_constructors.jl |
Updates test fixtures to use LinearCurve(...) for no-load and shutdown fields. |
Comments suppressed due to low confidence (1)
test/Project.toml:41
test/Project.tomlno longer pinsInfrastructureOptimizationModelsin[sources]and also doesn’t constrain it in[compat]. Since the test suite now relies on the updated IOM APIs used in this PR, consider adding an explicit compat lower bound (or a source override) here as well to avoid CI / local test runs resolving an older IOM version that can’t load the code.
[sources]
InfrastructureSystems = {rev = "IS4", url = "https://github.com/NREL-Sienna/InfrastructureSystems.jl"}
[compat]
HiGHS = "1"
Ipopt = "=1.4.0"
julia = "^1.11"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Upstream PSY now types `MarketBidTimeSeriesCost.start_up` as
`TupleTimeSeries{StartUpStages}` instead of a bare `TimeSeriesKey`:
- `_get_time_series_name(::StartupCostParameter, ...)` extracts the key via
`IS.get_time_series_key` (mirrors the existing Shutdown path).
- `validate_occ_component` on `ThermalMultiStart` early-returns for TupleTimeSeries
— values are pre-validated to `NTuple{3, Float64}` at construction.
- Renewable/Storage startup-nonzero check compares via `any(!iszero, values(x))`
so it works for both static `StartUpStages` (NamedTuple) and TS-backed
`NTuple{3, Float64}` elements.
- Test helpers (`extend_mbc!`, `_promote_mbc_to_ts!`, `stub_ts_market_bid_cost`)
wrap the TimeSeriesKey in `IS.TupleTimeSeries{PSY.StartUpStages}` when
constructing `MarketBidTimeSeriesCost`.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…include New `test/test_mbc_parameter_population.jl` exercises the `system-with-TS → parameter container` half of the MBC pipeline, which was previously only covered transitively. IEC PWL is `@test_broken` pending the PSI→POM migration of slope/breakpoint overloads. Along the way: - Include `src/core/default_interface_methods.jl` from the main module so `get_multiplier_value` for OCC parameters resolves to the defaults instead of the catch-all error. - Drop the duplicate `requires_initialization` in that file (IOM already defines it; the duplicate caused a method-overwrite warning). - Add `IS.@assert_op op_cost isa TS_OFFER_CURVE_COST_TYPES` at the top of each OCC `_get_time_series_name` method so a future bypass of IOM's filter surfaces a clean precondition error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # Project.toml # src/common_models/market_bid_overrides.jl # src/core/default_interface_methods.jl # src/services_models/reserves.jl # src/static_injector_models/electric_loads.jl # src/static_injector_models/hydro_generation.jl # src/static_injector_models/thermal_generation.jl # test/Project.toml
|
This relies on IOM #74 but otherwise this is good to merge. Tests pass locally, ignoring one PSB-related failure. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| device::PSY.StaticInjection, | ||
| ) | ||
| op_cost = PSY.get_operation_cost(device) | ||
| # TS types are validated at parameter population time |
There was a problem hiding this comment.
TS types are validated at parameter population time
As far as Claude can tell, this doesn't actually happen. I'll open an issue for it and add a test.
Summary
PSY split
MarketBidCostandImportExportCostinto separate static and time-series variants (MarketBidCost+MarketBidTimeSeriesCost,ImportExportCost+ImportExportTimeSeriesCost). This PR mirrors the IOM refactor (IOM PR #74) on the POM side and adds a unit-test suite for the MBC/IEC objective-function construction.Depends on IOM PR #74.
Changes
Dispatch (src/)
OnVariabledispatch into a single generic pair incommon_models/market_bid_overrides.jl, keyed on an_onvar_offer_directiontrait (Generator → IncrementalOffer,ControllableLoad → DecrementalOffer). Per-device thermal/hydro/load overrides drop out.isnothing(get_{input,output}_offer_curves(...))checks withIOM.is_nontrivial_offer—ZERO_OFFER_CURVEis now the default, notnothing. Load dispatch rejects non-trivial supply withArgumentError.add_proportional_cost!to IOM's sharedadd_proportional_cost_maybe_time_variant!, dropping a broken_lookup_maybe_time_variant_parampath that never had a definition.is_time_variant_termto a one-arg form (output depends only on cost type)._get_time_series_namefor*CostAtMinParametertraverses through the curve —initial_inputnow lives insideTimeSeriesPiecewiseIncrementalCurve, not on MBC directly.Tests
Existing MBC/IEC helpers (
add_market_bid_cost.jl,mbc_system_utils.jl,iec_simulation_utils.jl) updated forLinearCurvescalar fields and the new TS cost constructors.New unit-test suite, focused on "do the objective function terms and constraints turn out correct":
test/test_market_bid_cost.jl— Load (PowerLoadDispatch,PowerLoadInterruption) and Thermal (BasicUC,MultiStartUC) with static and TS costs. One testset per (device × formulation × cost-type); dedicated scaling testsets coverdt/ unit conversion. Multi-start testset exercises per-stagestart_up_costdispatch with a Tuple-valuedStartupCostParameter.test/test_import_export_cost.jl— Source +ImportExportSourceModelwith static and TS IEC, distinct import/export breakpoints (widths asserted).test/test_utils/mbc_math_helpers.jl— minimal system builders,OptimizationContainersetup, JuMP variable seeding, objective coefficient / PWL delta coefficient / PWL delta width inspection, stub TS curve/MBC construction, parameter seeding helpers.Flagged for follow-up
// FIXMEin the Source IEC dispatch: we now always add PWL for both directions (previousisnothingguard was dead in the new PSY). A cheap TS-curve emptiness check would let us skip the unused side for a one-directional Source.Test plan
julia --project=test -e 'include(\"test/runtests.jl\")').test_market_bid_cost.jl(29 tests) andtest_import_export_cost.jl(13 tests) pass.🤖 Generated with Claude Code