Port hybrid systems#104
Conversation
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
JuliaFormatter
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybrid_systems.jl
Lines 819 to 820 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybrid_systems.jl
Lines 1380 to 1390 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybrid_systems.jl
Lines 1442 to 1443 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybrid_systems.jl
Lines 1483 to 1484 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybrid_systems.jl
Lines 1551 to 1553 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 132 to 134 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 183 to 184 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 191 to 192 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 211 to 214 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 216 to 217 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 223 to 224 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 252 to 253 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 296 to 297 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 303 to 304 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 321 to 324 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 326 to 327 in 4ad672f
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 332 to 333 in 4ad672f
|
Performance Results
|
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
JuliaFormatter
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 143 to 144 in e0d67dc
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 193 to 194 in e0d67dc
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 201 to 202 in e0d67dc
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 221 to 224 in e0d67dc
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 226 to 227 in e0d67dc
[JuliaFormatter] reported by reviewdog 🐶
PowerOperationsModels.jl/src/hybrid_system_models/hybridsystem_constructor.jl
Lines 233 to 234 in e0d67dc
There was a problem hiding this comment.
We need to add a couple of extra tests.
First, we need to test different attributes, that is using reservation = false, and energy target = true. Also, what happened with storage_reservation and regularization attributes?
Also, it would be good to test that the hybrid system builds when there is no thermal, or no storage, or no renewable.
Finally, it would be good to test when the hybrid does not participate in reserves.
Overall, the PR looks good. Once we have the tests, I will QA/QC the model for the different tests to ensure that the constraints are looking good.
| # Hybrid System Parameters | ||
| ################################################################################# | ||
|
|
||
| "Time-series parameter for the maximum active power available from a hybrid system's |
There was a problem hiding this comment.
@jd-lara This is a change from how things are currently defined in HSS with the time series attached to the hybrid, not the subcomponent, but I guess that works and is simpler for this case?
kdayday
left a comment
There was a problem hiding this comment.
Following Rodrigo's lead on the testing, but in addition, can you port over the updated equivalent of what's in the HybridDispatchWithReserves docstring on the HSS PR and have claude add hyperlinks to exported types and functions referenced in the docstring? It looks like the POM docs are way out of date, but would like to update as we go.
There was a problem hiding this comment.
Pull request overview
This PR ports/introduces PSY.HybridSystem support into PowerOperationsModels.jl by adding a new HybridDispatchWithReserves device formulation, its constructor plumbing, and a basic end-to-end test that builds/solves a hybrid co-optimization on RTS-GMLC.
Changes:
- Adds hybrid formulation types, variables/parameters/expressions/constraints, and objective plumbing for
HybridDispatchWithReserves. - Adds a two-stage
construct_device!implementation for HybridSystems (argument + model stages) and hooks it into the main module includes/exports. - Adds test utilities and a new test that builds and solves a model containing a HybridSystem with reserves.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/PowerOperationsModels.jl |
Includes hybrid model files and exports the new hybrid-related types. |
src/hybrid_system_models/hybrid_systems.jl |
Implements hybrid variable bounds, reserve aggregation, constraints, and objective-cost plumbing. |
src/hybrid_system_models/hybridsystem_constructor.jl |
Adds the two-stage construct_device! pipeline for HybridDispatchWithReserves. |
src/core/variables.jl |
Introduces HybridSystem-specific variable types. |
src/core/parameters.jl |
Introduces HybridSystem time-series parameter types and unit-conversion behavior. |
src/core/expressions.jl |
Adds HybridSystem reserve aggregation/served-reserve expression types. |
src/core/constraints.jl |
Adds HybridSystem constraint type definitions. |
src/core/formulations.jl |
Adds hybrid formulation types and user-facing documentation. |
test/test_utils/hybrid_test_utils.jl |
Adds fixtures for building a test HybridSystem in RTS-GMLC. |
test/test_device_hybrid_constructors.jl |
Adds an integration-style test that builds and solves a hybrid dispatch-with-reserves model. |
test/includes.jl |
Wires hybrid test utilities into the test suite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ularization attributes; enable formatting; use jump utils; port docstrings;
Collapse paired Charge/Discharge, In/Out, Up/Down add_constraints! and
add_to_expression! methods into single bodies that dispatch on small
type-keyed trait stubs. Same JuMP shapes, same constraint meta strings,
same dispatch reachability. No public API change.
Hybrid (src/hybrid_system_models/hybrid_systems.jl):
- Charge/DischargeRegularizationConstraint
- HybridStorageStatus{Charge,Discharge}OnConstraint
- HybridStorage{Charging,Discharging}ReservePowerLimitConstraint
(folds in the _ch/_ds_reserve_up_dn_exprs helpers)
- HybridTotalReserve{Up,Down}Expression /
HybridServedReserve{Out,In}{Up,Down}Expression add_to_expression!
- ReserveAssignment/Deployment{Up,Down}{Charge,Discharge} ←
Hybrid{Charging,Discharging}ReserveVariable add_to_expression!
- HybridStatus{Out,In}OnConstraint
Storage (src/energy_storage_models/storage_models.jl):
- StorageRegularizationConstraint{Charge,Discharge}
Verified: 71/71 hybrid + storage tests pass. Net -376 lines.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Replace three custom hybrid constraint types and their hand-written
add_constraints! bodies with a single call to IOM's
`add_semicontinuous_range_constraints!`, paralleling how
`AbstractThermalUnitCommitment` handles the same range-with-on-variable
pattern at thermal_generation.jl:405-419.
Mechanism:
- Define `get_min_max_limits(::PSY.HybridSystem, ::ActivePowerVariableLimitsConstraint, ::AbstractHybridFormulation)`
to read `PSY.get_active_power_limits(PSY.get_thermal_unit(d))`. IOM's
helper picks up `OnVariable` keyed by `PSY.HybridSystem` automatically.
- For the with-reserves case, introduce two expression types subtyping
`RangeConstraint{UB,LB}Expressions`: `HybridThermalActivePowerWithReserve{UB,LB}`.
Argument-stage `add_expressions!` aggregates `p_th + Σ r_up` (UB) and
`p_th − Σ r_dn` (LB) into them, after which IOM's expression-typed
dispatch emits `min·on ≤ p_th − r_dn` and `p_th + r_up ≤ max·on` directly.
Removes:
- HybridThermalOnVariableUbConstraint, HybridThermalOnVariableLbConstraint,
HybridThermalReserveLimitConstraint (constraint types + exports)
- _thermal_reserve_up_expr / _thermal_reserve_down_expr helpers
- Three add_constraints! bodies (~190 lines)
Renewable cases stay hand-written for now: IOM's parameterized helper
filters by `IS.has_time_series(d, ts_type, ts_name)`, and PSY's
HybridSystem doesn't expose its inner RenewableDispatch's time series
through that accessor. A separate change would be required.
Verified: 50/50 hybrid tests pass.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
This reverts commit 59c994b.
Per PR review (acostarelli): "domain" describes the value space of each variable (a discrete set or continuous interval) more accurately than "bounds", which suggests inequality-only. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Address PR review feedback:
- "Don't use `isa`. Add a method to handle this, or restructure
existing dispatch." Eliminates every `isa(service, PSY.Reserve{...})`
and `service isa skip_kind` site in hybrid_systems.jl. The
`_excluded_reserve_kind` trait stub goes away — its information is
now encoded by union types in helper method signatures.
Five sites refactored, all sharing the same shape (per-direction
no-op methods + a fallback `::PSY.Service` work method):
- `add_to_expression!` for HybridTotalReserveExpression /
HybridServedReserveExpression → `_accumulate_reserve!`
- `add_to_expression!` for the eight Reserve*Balance{Up,Down}
{Charge,Discharge} expressions → `_balance_term!` plus a
`_deployment_factor` per-T trait that replaces the
`is_up`/`is_deployment` Booleans
- `_renewable_reserve_up/down_expr` → `_renewable_reserve_*_term!`
thunks sharing `_accumulate_renewable_reserve!`
- `_thermal_reserve_up/down_expr` → analogous restructure
- `add_constraints!` for ReserveCoverageConstraint{,EndOfPeriod}
→ `_init_coverage_container!` + `_emit_coverage_constraint!`;
the `(service isa PSY.Reserve) || continue` guard becomes the
`::PSY.Service` fallback no-op
Helper arguments use concrete types (OptimizationContainer, String,
Int, Float64, PSY.Storage, …) plus parametric `::Type{T}`/`::Type{U}`/
`d::V`/`::W` to reduce precompilation overhead.
- "Combine these if statements." Merges the two adjacent setup
ternaries (`r_ub, r_lb = if has_reserves …` and
`con_lb = if has_reserves …`) in
`add_constraints!(::Type{HybridStatus{Out,In}OnConstraint}, …)` into
a single 3-tuple ternary.
Test.detect_ambiguities returns 0; full suite passes (50/50).
Co-Authored-By: Claude Opus 4.7 <[email protected]>
…milies
Hybrid reserve variables, expressions, and constraints had ~16 paired Charge/Discharge,
Up/Down, Total/Served (a.k.a. Assignment/Deployment), and UB/LB singleton structs plus
~14 paired trait helpers that all differed only by which sibling they referenced.
Introduce marker singletons for ReserveSide, ReserveDirection, ReserveScale (UnscaledReserve
/ DeployedReserve), reuse IOM's BoundDirection (UpperBound/LowerBound), and reparametrize
the family roots:
- ReserveAggregationExpression{D,S,Sd} umbrella with two concrete struct families
(HybridPCCReserveExpression, StorageReserveBalanceExpression) covering all 16
historical reserve-expression singletons.
- HybridPCCReserveVariable{Sd}, HybridStorageSubcomponentReserveVariable{Sd},
HybridStorageSubcomponentPower{Sd}, RegularizationVariable{Sd}.
- HybridStatusOnConstraint{Sd}, HybridStorageStatusOnConstraint{Sd},
HybridStorageReservePowerLimitConstraint{Sd}, RegularizationConstraint{Sd},
HybridThermalOnVariableConstraint{B}.
All 34 historical concrete names are retained as const aliases so external imports,
`get_expression`/`get_variable`/`get_constraint` lookups, and module exports are
byte-compatible. Inside hybrid_systems.jl this lets:
- _accumulate_reserve! + _balance_term! collapse into one _add_reserve_term! family
(PCC boundary and storage subcomponent share the no-op skip and scale dispatch);
- thermal/renewable subcomponent accumulators (10 helpers) collapse into one
_subcomponent_reserve_term! / _subcomponent_reserve_expr family parametric on
the variable type;
- the UB/LB thermal-on-variable add_constraints! methods merge;
- ~14 paired trait helpers (storage / PCC / regularization) become parametric
single-method definitions;
- 5 file-local Union consts (_BalanceUpExpr, _BalanceDownExpr, _BalanceDeploymentExpr,
_HybridReserveUpExpr, _HybridReserveDownExpr) and _StorageCharge/DischargeSide
Union consts get deleted.
Additional cleanups:
- get_variable_multiplier hybrid signatures take ::Type{<:Formulation} (matches the
rest of POM); all W() instance call-sites become type-keyed.
- Three `if W <: ...` body-level subtype checks split into separate parametric
dispatched methods (HybridStorageBalanceConstraint, RegularizationConstraint,
HybridStatusOnConstraint).
- _init_coverage_container! uses lazy_container_addition! (idempotent).
- add_proportional_cost!(OnVariable, hybrids) hoists the variant/invariant
function-handle selection out of the per-t loop.
Net: -142 lines across 5 files. Full Pkg.test passes (13125 / 0 fail / 0 error / 1
pre-existing broken). Zero method ambiguities.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Thanks for opening a PR to PowerOperationsModels.jl, please take note of the following when making a PR:
Check the contributor guidelines