Skip to content

Update POM for PSY MarketBidCost/ImportExportCost static/TS split#77

Open
luke-kiernan wants to merge 12 commits intomainfrom
lk/mbc_iec_refactor
Open

Update POM for PSY MarketBidCost/ImportExportCost static/TS split#77
luke-kiernan wants to merge 12 commits intomainfrom
lk/mbc_iec_refactor

Conversation

@luke-kiernan
Copy link
Copy Markdown
Collaborator

Summary

PSY split MarketBidCost and ImportExportCost into 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/)

  • Consolidated MBC OnVariable dispatch into a single generic pair in common_models/market_bid_overrides.jl, keyed on an _onvar_offer_direction trait (Generator → IncrementalOffer, ControllableLoad → DecrementalOffer). Per-device thermal/hydro/load overrides drop out.
  • Replaced stale isnothing(get_{input,output}_offer_curves(...)) checks with IOM.is_nontrivial_offerZERO_OFFER_CURVE is now the default, not nothing. Load dispatch rejects non-trivial supply with ArgumentError.
  • Delegated hydro add_proportional_cost! to IOM's shared add_proportional_cost_maybe_time_variant!, dropping a broken _lookup_maybe_time_variant_param path that never had a definition.
  • Collapsed is_time_variant_term to a one-arg form (output depends only on cost type).
  • _get_time_series_name for *CostAtMinParameter traverses through the curve — initial_input now lives inside TimeSeriesPiecewiseIncrementalCurve, not on MBC directly.

Tests

Existing MBC/IEC helpers (add_market_bid_cost.jl, mbc_system_utils.jl, iec_simulation_utils.jl) updated for LinearCurve scalar 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 cover dt / unit conversion. Multi-start testset exercises per-stage start_up_cost dispatch with a Tuple-valued StartupCostParameter.
  • test/test_import_export_cost.jl — Source + ImportExportSourceModel with static and TS IEC, distinct import/export breakpoints (widths asserted).
  • 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 for follow-up

  • // FIXME in the Source IEC dispatch: we now always add PWL for both directions (previous isnothing guard 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

  • Full POM test suite passes locally (julia --project=test -e 'include(\"test/runtests.jl\")').
  • New test_market_bid_cost.jl (29 tests) and test_import_export_cost.jl (13 tests) pass.
  • IOM test suite still passes after the coupled bug-fix commit in IOM PR Update POM to work with IOM main #74.

🤖 Generated with Claude Code

luke-kiernan and others added 5 commits April 10, 2026 15:30
- 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>
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>
@luke-kiernan
Copy link
Copy Markdown
Collaborator Author

luke-kiernan commented Apr 14, 2026

Clarification: first 4 commits are from #74. The actual changes start at 5d00c45, "update POM for PSY MarketBidCost/ImportExportCost static/TS split."

@luke-kiernan luke-kiernan changed the base branch from main to lk/update-pom-to-iom-main April 14, 2026 17:42
@luke-kiernan luke-kiernan changed the base branch from lk/update-pom-to-iom-main to main April 14, 2026 17:42
@jd-lara jd-lara requested a review from Copilot April 14, 2026 19:43
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

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.toml no longer pins InfrastructureOptimizationModels in [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.

Comment thread src/common_models/add_parameters.jl Outdated
Comment thread src/common_models/add_parameters.jl Outdated
Comment thread src/common_models/add_parameters.jl Outdated
luke-kiernan and others added 4 commits April 15, 2026 13:21
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
@luke-kiernan
Copy link
Copy Markdown
Collaborator Author

This relies on IOM #74 but otherwise this is good to merge. Tests pass locally, ignoring one PSB-related failure.

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 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.

Comment thread src/common_models/market_bid_overrides.jl
Comment thread src/common_models/market_bid_plumbing.jl Outdated
Comment thread src/common_models/market_bid_plumbing.jl Outdated
Comment thread src/static_injector_models/thermal_generation.jl Outdated
device::PSY.StaticInjection,
)
op_cost = PSY.get_operation_cost(device)
# TS types are validated at parameter population time
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

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.

4 participants