Open
Conversation
IOM PR #71 changed interface functions from instance dispatch (::T) to type dispatch (::Type{T}) for VariableType, ConstraintType, ExpressionType, ParameterType, and formulation subtypes. This commit updates POM accordingly: - Convert ~800+ call sites from SomeKey() to SomeKey for get_variable, get_expression, get_parameter, add_*_container!, lazy_container_addition! - Convert all get_variable_binary/upper_bound/lower_bound/warm_start_value definitions to use ::Type{X} for variable and formulation args - Convert objective function interfaces (proportional_cost, objective_function_multiplier, variable_cost, start_up_cost, etc.) - Fix POM-local add_variables! overrides to extract formulation type via ::F where F pattern - Add device type annotations to get_min_max_limits to resolve ambiguities with IOM default - Update test helpers and test call sites Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e-dispatch' into ac/shift-load
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new shiftable load formulation (PowerLoadShift) and wires it through the optimization container interfaces, while also updating large portions of the codebase to use type-based dispatch (passing T instead of T()) for variables/constraints/expressions.
Changes:
- Introduces
PowerLoadShiftforPSY.ShiftablePowerLoad, including new variables, parameters, expression(s), and constraints (incl. non-anticipativity). - Refactors many
get_variable/get_expression/add_variables!/bounds & multiplier hooks to dispatch on::Type{...}rather than instances. - Updates and adds tests (including a new testset for the non-anticipativity behavior) and reduces initialization test horizons for runtime.
Reviewed changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_utils/model_checks.jl | Updates container lookups to use type-based get_variable / get_expression. |
| test/test_network_constructors_with_dlr.jl | Updates PTDF branch flow expression lookup to type-based API. |
| test/test_model_decision.jl | Updates variable lookup to type-based API. |
| test/test_initialization_problem.jl | Reduces horizon and updates variable/constraint count helpers to type-based API. |
| test/test_device_load_constructors.jl | Adds a new testset covering PowerLoadShift + NonAnticipativityConstraint. |
| test/Project.toml | Removes [sources] override for InfrastructureSystems in test environment. |
| src/twoterminal_hvdc_models/branch_constructor.jl | Refactors formulation arguments to be passed as types. |
| src/twoterminal_hvdc_models/AC_branches.jl | Refactors variable/parameter hooks and add_variables!/constraints to type-based dispatch. |
| src/static_injector_models/thermalgeneration_constructor.jl | Refactors add_variables! formulation arguments to types. |
| src/static_injector_models/source_constructor.jl | Refactors add_variables! formulation arguments to types. |
| src/static_injector_models/source.jl | Refactors multiplier/bounds hooks to dispatch on ::Type{...} and updates objective additions. |
| src/static_injector_models/renewablegeneration_constructor.jl | Refactors add_variables! formulation arguments to types. |
| src/static_injector_models/renewable_generation.jl | Refactors hooks and objective additions to type-based dispatch. |
| src/static_injector_models/reactivepowerdevice_constructor.jl | Refactors add_variables! formulation arguments to types. |
| src/static_injector_models/reactivepower_device.jl | Refactors variable property hooks to type-based dispatch. |
| src/static_injector_models/load_constructor.jl | Adds ShiftablePowerLoad constructors for PowerLoadShift and refactors other calls to type-based dispatch. |
| src/static_injector_models/hydrogeneration_constructor.jl | Refactors add_variables! formulation arguments to types. |
| src/static_injector_models/electric_loads.jl | Implements PowerLoadShift: defaults, RealizedShiftedLoad, balance + non-anticipativity + limit constraints, and costs; refactors existing load code to type-based dispatch. |
| src/services_models/transmission_interface.jl | Refactors interface slack hooks and expression/constraint lookups to type-based dispatch. |
| src/services_models/services_constructor.jl | Refactors service construction to type-based dispatch (e.g., RangeReserve, InterfaceTotalFlow). |
| src/services_models/service_slacks.jl | Refactors slack variable container creation to type-based dispatch. |
| src/services_models/reserves.jl | Refactors reserve hooks, constraint additions, and cost plumbing to type-based dispatch. |
| src/services_models/reserve_group.jl | Refactors reserve group variable/constraint lookups to type-based dispatch. |
| src/services_models/agc.jl | Refactors AGC hooks/containers to type-based dispatch. |
| src/network_models/security_constrained_models.jl | Refactors constraint container and expression lookup to type-based dispatch. |
| src/network_models/powermodels_interface.jl | Updates PM→PS container creation to use typeof(ps_v) for variable/constraint types. |
| src/network_models/network_slack_variables.jl | Refactors slack variable hooks and lookups to type-based dispatch. |
| src/network_models/hvdc_networks.jl | Refactors DCVoltage hooks to type-based dispatch and updates expression/constraint lookups. |
| src/network_models/copperplate_model.jl | Refactors expression/constraint lookup to type-based dispatch. |
| src/network_models/area_balance_model.jl | Refactors expression/constraint/variable lookup to type-based dispatch. |
| src/mt_hvdc_models/hvdcsystems_constructor.jl | Refactors HVDC system add_variables!/interpolation helper calls to type-based dispatch. |
| src/mt_hvdc_models/HVDCsystems.jl | Refactors HVDC hooks/expressions/constraints to type-based dispatch. |
| src/energy_storage_models/storage_constructor.jl | Refactors storage+reserves helper calls to type-based dispatch. |
| src/core/variables.jl | Adds ShiftUpActivePowerVariable and ShiftDownActivePowerVariable. |
| src/core/parameters.jl | Adds ShiftUpActivePowerTimeSeriesParameter and ShiftDownActivePowerTimeSeriesParameter and conversion hooks. |
| src/core/interfaces.jl | Updates base interface hooks (get_variable_multiplier, get_multiplier_value, etc.) to accept ::Type{...}. |
| src/core/formulations.jl | Adds PowerLoadShift formulation type. |
| src/core/expressions.jl | Adds RealizedShiftedLoad expression type and output conversion/write flags. |
| src/core/default_interface_methods.jl | Refactors default multiplier hooks to type-based dispatch. |
| src/core/constraints.jl | Adds new shift-related constraint types. |
| src/common_models/reserve_range_constraints.jl | Refactors reserve range helpers to type-based dispatch. |
| src/common_models/market_bid_overrides.jl | Refactors market-bid override plumbing and cost hooks to type-based dispatch. |
| src/common_models/make_system_expressions.jl | Updates HVDC initialization to use type-based DCVoltage variable creation. |
| src/common_models/add_parameters.jl | Refactors parameter addition internals to use type-based multiplier/hooks. |
| src/common_models/add_expressions.jl | Refactors expression container creation to type-based dispatch. |
| src/area_interchange.jl | Refactors interchange hooks/containers/expressions to type-based dispatch. |
| src/ac_transmission_models/branch_constructor.jl | Refactors formulation arguments to be passed as types. |
| src/ac_transmission_models/AC_branches.jl | Refactors branch hooks/variables/constraints/expressions to type-based dispatch. |
| src/PowerOperationsModels.jl | Exports new load-shift types and imports add_variable_cost! for new objective logic. |
| Project.toml | Removes [sources] override for InfrastructureSystems in main environment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+251
to
+252
| convert_result_to_natural_units(::Type{ShiftUpActivePowerTimeSeriesParameter}) = true | ||
| convert_result_to_natural_units(::Type{ShiftDownActivePowerTimeSeriesParameter}) = true |
There was a problem hiding this comment.
convert_result_to_natural_units is not used anywhere else in this codebase (only convert_output_to_natural_units is extended for parameters). As written, this will define a new local function and the ShiftUp/ShiftDown time-series parameters likely won't be converted to natural units when outputs/results are written. Consider changing these to convert_output_to_natural_units(::Type{ShiftUpActivePowerTimeSeriesParameter}) = true and likewise for ShiftDown (or use the existing conversion hook used by other parameters).
Suggested change
| convert_result_to_natural_units(::Type{ShiftUpActivePowerTimeSeriesParameter}) = true | |
| convert_result_to_natural_units(::Type{ShiftDownActivePowerTimeSeriesParameter}) = true | |
| convert_output_to_natural_units(::Type{ShiftUpActivePowerTimeSeriesParameter}) = true | |
| convert_output_to_natural_units(::Type{ShiftDownActivePowerTimeSeriesParameter}) = true |
| get_variable(container, AdditionalDeltaActivePowerUpVariable, PSY.Area) | ||
| R_dn_emergency = | ||
| get_variable(container, AdditionalDeltaActivePowerUpVariable(), PSY.Area) | ||
| get_variable(container, AdditionalDeltaActivePowerUpVariable, PSY.Area) |
There was a problem hiding this comment.
R_dn_emergency is currently retrieved using AdditionalDeltaActivePowerUpVariable (same as R_up_emergency). This makes the emergency up/down terms identical and will apply the wrong sign in the frequency-response balance. It should likely use AdditionalDeltaActivePowerDownVariable for the down-emergency variable.
Suggested change
| get_variable(container, AdditionalDeltaActivePowerUpVariable, PSY.Area) | |
| get_variable(container, AdditionalDeltaActivePowerDownVariable, PSY.Area) |
Comment on lines
+339
to
+360
| additional_balance_interval = get_attribute(model, "additional_balance_interval") | ||
| if !isnothing(additional_balance_interval) | ||
| constraint_aux = add_constraints_container!( | ||
| container, | ||
| T, | ||
| V, | ||
| PSY.get_name.(devices); | ||
| meta = "additional", | ||
| ) | ||
| resolution = get_resolution(container) | ||
| interval_length = | ||
| Dates.Millisecond(additional_balance_interval).value ÷ | ||
| Dates.Millisecond(resolution).value | ||
| for d in devices | ||
| name = PSY.get_name(d) | ||
| constraint_aux[name] = JuMP.@constraint( | ||
| container.JuMPmodel, | ||
| sum( | ||
| up_variable[name, t] - down_variable[name, t] for | ||
| t in 1:interval_length | ||
| ) == 0.0 | ||
| ) |
There was a problem hiding this comment.
The optional additional_balance_interval constraint only enforces balance over t in 1:interval_length. For horizons longer than 2 * interval_length, this does not enforce per-interval balance (only the first block), and it also assumes time steps are 1-based contiguous. Consider iterating over time_steps in blocks of interval_length (and validating interval_length is >= 1 and <= length(time_steps)) so the intended interval-based energy balance is correctly enforced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port of Sienna-Platform/PowerSimulations.jl#1592