From 805c07ea844fbd4ec29274ef3fa44ee297909a92 Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Tue, 12 May 2026 10:22:58 -0600 Subject: [PATCH 1/3] Use FlowSign trait for variable multipliers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire `get_variable_multiplier` default through IOM's flow_sign trait; delete ~30 redundant ±1.0 overrides. NaN catch-alls become loud errors. Rename 5-arg storage AS overload to `get_reserve_balance_factor` (0/1 incidence, distinct from sign). Co-Authored-By: Claude Opus 4.7 (1M context) --- Project.toml | 2 +- src/ac_transmission_models/AC_branches.jl | 1 - src/core/interfaces.jl | 30 +++++++++++---- src/core/variables.jl | 8 ++++ src/energy_storage_models/storage_models.jl | 38 +++++++++---------- src/mt_hvdc_models/HVDCsystems.jl | 2 - src/network_models/network_slack_variables.jl | 7 ---- src/services_models/agc.jl | 2 +- src/services_models/reserves.jl | 2 +- src/services_models/transmission_interface.jl | 6 --- .../hydro_generation.jl | 4 -- .../reactivepower_device.jl | 1 - .../renewable_generation.jl | 1 - src/static_injector_models/source.jl | 3 -- .../thermal_generation.jl | 2 - .../TwoTerminalDC_branches.jl | 2 +- test/Project.toml | 2 +- 17 files changed, 55 insertions(+), 58 deletions(-) diff --git a/Project.toml b/Project.toml index 504d67e..8836b4a 100644 --- a/Project.toml +++ b/Project.toml @@ -29,7 +29,7 @@ PowerFlowsExt = "PowerFlows" [sources] InfrastructureSystems = {url = "https://github.com/NREL-Sienna/InfrastructureSystems.jl", rev = "IS4"} PowerSystems = {url = "https://github.com/NREL-Sienna/PowerSystems.jl", rev = "psy6"} -InfrastructureOptimizationModels = {url = "https://github.com/NREL-Sienna/InfrastructureOptimizationModels.jl", rev = "ac/psi-costexp-parambroad-pfslack"} +InfrastructureOptimizationModels = {url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl", rev = "lk/flow-sign-trait"} [compat] Dates = "1" diff --git a/src/ac_transmission_models/AC_branches.jl b/src/ac_transmission_models/AC_branches.jl index ea24559..7fd065d 100644 --- a/src/ac_transmission_models/AC_branches.jl +++ b/src/ac_transmission_models/AC_branches.jl @@ -26,7 +26,6 @@ get_parameter_multiplier(::Type{LowerBoundValueParameter}, ::PSY.ACTransmission, get_parameter_multiplier(::Type{UpperBoundValueParameter}, ::PSY.ACTransmission, ::Type{<:AbstractBranchFormulation}) = 1.0 # Per-device reactance multiplier (1/get_x(d)) computed inline at add_to_expression! call sites. -get_variable_multiplier(::Type{PhaseShifterAngle}, ::Type{<:PSY.PhaseShiftingTransformer}, ::Type{PhaseAngleControl}) = 1.0 get_multiplier_value(::Type{<:AbstractDynamicBranchRatingTimeSeriesParameter}, d::PSY.ACTransmission, ::Type{StaticBranch}) = PSY.get_rating(d) get_multiplier_value(::Type{<:AbstractDynamicBranchRatingTimeSeriesParameter}, d::PNM.BranchesParallel, ::Type{StaticBranch}) = PNM.get_equivalent_rating(d) diff --git a/src/core/interfaces.jl b/src/core/interfaces.jl index ace4855..242e5cf 100644 --- a/src/core/interfaces.jl +++ b/src/core/interfaces.jl @@ -83,13 +83,29 @@ end """ Get the multiplier for a variable type when adding to an expression. -Default implementation returns 1.0. -""" -get_variable_multiplier( - ::Type{<:IS.Optimization.VariableType}, - ::Type{<:IS.InfrastructureSystemsComponent}, - ::Type{<:IOM.AbstractDeviceFormulation}, -) = 1.0 + +Default consults [`IOM.flow_sign`](@ref) on the variable type: variables marked +`Injection` contribute `+1.0`, `Withdrawal` contributes `-1.0`, and `Unsigned` +falls back to `1.0`. Device-driven sign rules (e.g. anything on `PSY.ElectricLoad`) +should be expressed as more-specific dispatches that override this default. +""" +function get_variable_multiplier( + ::Type{V}, + # second arg is any system component, or `PSY.System` which is a ComponentContainer + ::Type{<:Union{IS.InfrastructureSystemsComponent, IS.ComponentContainer}}, + ::Type, # could be PowerModel (network) or DeviceFormulation (device) +) where {V <: IS.Optimization.VariableType} + return IOM.multiplier_from_sign(IOM.flow_sign(V)) +end + +# Hard-error fallback for variable/device/formulation triples that intentionally +# have no defined multiplier (must be specialized at the call site). Used by +# devices like Storage/AGC/Reserve/TwoTerminalHVDC to fail loudly instead of +# returning NaN, which silently poisons JuMP expressions. +@noinline _unsupported_multiplier(V, D, F) = error( + "get_variable_multiplier not implemented for variable $V on $D under $F. " * + "This combination must be specialized.", +) """ Get the multiplier for an expression type based on parameter type. diff --git a/src/core/variables.jl b/src/core/variables.jl index 660ef5e..ae829e1 100644 --- a/src/core/variables.jl +++ b/src/core/variables.jl @@ -101,6 +101,9 @@ Docs abbreviation: ``p^\\text{sl,dn}`` """ struct SystemBalanceSlackDown <: VariableType end +IOM.flow_sign(::Type{SystemBalanceSlackUp}) = IOM.Injection +IOM.flow_sign(::Type{SystemBalanceSlackDown}) = IOM.Withdrawal + """ Struct to dispatch the creation of Reserve requirement slack variables. Used when there is not reserves in the system to satisfy the requirement. @@ -476,6 +479,9 @@ Docs abbreviation: ``f^\\text{sl,dn}`` """ struct InterfaceFlowSlackDown <: VariableType end +IOM.flow_sign(::Type{InterfaceFlowSlackUp}) = IOM.Injection +IOM.flow_sign(::Type{InterfaceFlowSlackDown}) = IOM.Withdrawal + """ Struct to dispatch the creation of Slack variables for UpperBoundFeedforward @@ -562,6 +568,8 @@ Struct to dispatch the creation of a variable for pumped power in a hydro pump t """ struct ActivePowerPumpVariable <: VariableType end +IOM.flow_sign(::Type{ActivePowerPumpVariable}) = IOM.Withdrawal + """ Auxiliary Variable for Hydro Models that solve for total energy output diff --git a/src/energy_storage_models/storage_models.jl b/src/energy_storage_models/storage_models.jl index 4010772..d295728 100644 --- a/src/energy_storage_models/storage_models.jl +++ b/src/energy_storage_models/storage_models.jl @@ -1,7 +1,7 @@ #! format: off requires_initialization(::AbstractStorageFormulation) = false -get_variable_multiplier(::Type{<:VariableType}, ::Type{<:PSY.Storage}, ::Type{<:AbstractStorageFormulation}) = NaN +get_variable_multiplier(::Type{V}, ::Type{D}, ::Type{F}) where {V <: VariableType, D <: PSY.Storage, F <: AbstractStorageFormulation} = _unsupported_multiplier(V, D, F) ########################### ActivePowerInVariable, Storage ################################# get_variable_binary(::Type{ActivePowerInVariable}, ::Type{<:PSY.Storage}, ::Type{<:AbstractStorageFormulation}) = false get_variable_lower_bound(::Type{ActivePowerInVariable}, d::PSY.Storage, ::Type{<:AbstractStorageFormulation}) = 0.0 @@ -431,7 +431,7 @@ function add_variables!( end ############################# Expression Logic for Ancillary Services ###################### -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveAssignmentBalanceDownCharge}, d::PSY.Storage, @@ -439,7 +439,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 0.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveAssignmentBalanceDownCharge}, d::PSY.Storage, @@ -447,7 +447,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveDown}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveAssignmentBalanceUpCharge}, d::PSY.Storage, @@ -455,7 +455,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveAssignmentBalanceUpCharge}, d::PSY.Storage, @@ -463,7 +463,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveDown}, ) = 0.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveAssignmentBalanceDownDischarge}, d::PSY.Storage, @@ -471,7 +471,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 0.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveAssignmentBalanceDownDischarge}, d::PSY.Storage, @@ -479,7 +479,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveDown}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveAssignmentBalanceUpDischarge}, d::PSY.Storage, @@ -487,7 +487,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveAssignmentBalanceUpDischarge}, d::PSY.Storage, @@ -496,7 +496,7 @@ get_variable_multiplier( ) = 0.0 ### Deployment ### -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveDeploymentBalanceDownCharge}, d::PSY.Storage, @@ -504,7 +504,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 0.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveDeploymentBalanceDownCharge}, d::PSY.Storage, @@ -512,7 +512,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveDown}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveDeploymentBalanceUpCharge}, d::PSY.Storage, @@ -520,7 +520,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableCharge}, ::Type{ReserveDeploymentBalanceUpCharge}, d::PSY.Storage, @@ -528,7 +528,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveDown}, ) = 0.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveDeploymentBalanceDownDischarge}, d::PSY.Storage, @@ -536,7 +536,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 0.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveDeploymentBalanceDownDischarge}, d::PSY.Storage, @@ -544,7 +544,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveDown}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveDeploymentBalanceUpDischarge}, d::PSY.Storage, @@ -552,7 +552,7 @@ get_variable_multiplier( ::PSY.Reserve{PSY.ReserveUp}, ) = 1.0 -get_variable_multiplier( +get_reserve_balance_factor( ::Type{AncillaryServiceVariableDischarge}, ::Type{ReserveDeploymentBalanceUpDischarge}, d::PSY.Storage, @@ -631,7 +631,7 @@ function add_to_expression!( for s in services s_name = PSY.get_name(s) variable = get_variable(container, U, V, "$(typeof(s))_$s_name") - mult = get_variable_multiplier(U, T, d, W, s) * get_fraction(T, s) + mult = get_reserve_balance_factor(U, T, d, W, s) * get_fraction(T, s) for t in get_time_steps(container) add_proportional_to_jump_expression!( expression[name, t], @@ -663,7 +663,7 @@ function add_to_expression!( for s in services s_name = PSY.get_name(s) variable = get_variable(container, U, V, "$(typeof(s))_$s_name") - mult = get_variable_multiplier(U, T, d, W, s) * get_fraction(T, s) + mult = get_reserve_balance_factor(U, T, d, W, s) * get_fraction(T, s) for t in get_time_steps(container) add_proportional_to_jump_expression!( expression[name, t], diff --git a/src/mt_hvdc_models/HVDCsystems.jl b/src/mt_hvdc_models/HVDCsystems.jl index cfa40e1..79ec44e 100644 --- a/src/mt_hvdc_models/HVDCsystems.jl +++ b/src/mt_hvdc_models/HVDCsystems.jl @@ -3,7 +3,6 @@ get_variable_binary(::Type{ActivePowerVariable}, ::Type{PSY.InterconnectingConve get_variable_warm_start_value(::Type{ActivePowerVariable}, d::PSY.InterconnectingConverter, ::Type{<:AbstractConverterFormulation}) = PSY.get_active_power(d) get_variable_lower_bound(::Type{ActivePowerVariable}, d::PSY.InterconnectingConverter, ::Type{<:AbstractConverterFormulation}) = PSY.get_active_power_limits(d).min get_variable_upper_bound(::Type{ActivePowerVariable}, d::PSY.InterconnectingConverter, ::Type{<:AbstractConverterFormulation}) = PSY.get_active_power_limits(d).max -get_variable_multiplier(::Type{<:VariableType}, ::Type{PSY.InterconnectingConverter}, ::Type{<:AbstractConverterFormulation}) = 1.0 function _get_flow_bounds(d::PSY.TModelHVDCLine) @@ -65,7 +64,6 @@ function get_variable_upper_bound(::Type{DCLineCurrent}, d::PSY.TModelHVDCLine, max_v = max(PSY.get_magnitude(bus_from), PSY.get_magnitude(bus_to)) return p_max_flow / max_v end -get_variable_multiplier(::Type{<:VariableType}, ::Type{PSY.TModelHVDCLine}, ::Type{<:AbstractBranchFormulation}) = 1.0 requires_initialization(::AbstractConverterFormulation) = false requires_initialization(::LosslessLine) = false diff --git a/src/network_models/network_slack_variables.jl b/src/network_models/network_slack_variables.jl index f7351a3..ff15ccc 100644 --- a/src/network_models/network_slack_variables.jl +++ b/src/network_models/network_slack_variables.jl @@ -1,10 +1,3 @@ -#! format: off -get_variable_multiplier(::Type{SystemBalanceSlackUp}, ::Type{<: Union{PSY.ACBus, PSY.Area, PSY.System}}, ::Type{<:AbstractDeviceFormulation}) = 1.0 -get_variable_multiplier(::Type{SystemBalanceSlackDown}, ::Type{<: Union{PSY.ACBus, PSY.Area, PSY.System}}, ::Type{<:AbstractDeviceFormulation}) = -1.0 -get_variable_multiplier(::Type{SystemBalanceSlackUp}, ::Type{<: Union{PSY.ACBus, PSY.Area, PSY.System}}, ::Type{<:AbstractPowerModel}) = 1.0 -get_variable_multiplier(::Type{SystemBalanceSlackDown}, ::Type{<: Union{PSY.ACBus, PSY.Area, PSY.System}}, ::Type{<:AbstractPowerModel}) = -1.0 -#! format: on - function add_variables!( container::OptimizationContainer, ::Type{T}, diff --git a/src/services_models/agc.jl b/src/services_models/agc.jl index 00de25e..813a675 100644 --- a/src/services_models/agc.jl +++ b/src/services_models/agc.jl @@ -1,5 +1,5 @@ #! format: off -get_variable_multiplier(::Type{<:VariableType}, ::Type{<:PSY.AGC}, ::Type{<:AbstractAGCFormulation}) = NaN +get_variable_multiplier(::Type{V}, ::Type{D}, ::Type{F}) where {V <: VariableType, D <: PSY.AGC, F <: AbstractAGCFormulation} = _unsupported_multiplier(V, D, F) ########################## ActivePowerVariable, AGC ########################### ########################## SteadyStateFrequencyDeviation ################################## diff --git a/src/services_models/reserves.jl b/src/services_models/reserves.jl index c8eccb0..550fa07 100644 --- a/src/services_models/reserves.jl +++ b/src/services_models/reserves.jl @@ -1,7 +1,7 @@ #! format: off ############################### Reserve Variables ######################################### -get_variable_multiplier(::Type{<:VariableType}, ::Type{<:PSY.Reserve}, ::Type{<:AbstractReservesFormulation}) = NaN +get_variable_multiplier(::Type{V}, ::Type{D}, ::Type{F}) where {V <: VariableType, D <: PSY.Reserve, F <: AbstractReservesFormulation} = _unsupported_multiplier(V, D, F) ############################### PostContingencyActivePowerReserveDeploymentVariable, Reserve ######################################### get_variable_binary(::Type{PostContingencyActivePowerReserveDeploymentVariable}, ::Type{<:PSY.Reserve}, ::Type{<:AbstractSecurityConstrainedReservesFormulation}) = false function get_variable_upper_bound(::Type{PostContingencyActivePowerReserveDeploymentVariable}, r::PSY.Reserve, d::PSY.Device, ::Type{<:AbstractSecurityConstrainedReservesFormulation}) diff --git a/src/services_models/transmission_interface.jl b/src/services_models/transmission_interface.jl index 021ca66..79955d6 100644 --- a/src/services_models/transmission_interface.jl +++ b/src/services_models/transmission_interface.jl @@ -3,12 +3,6 @@ get_variable_binary(::Type{<:VariableType}, ::Type{PSY.TransmissionInterface}, : get_variable_lower_bound(::Type{InterfaceFlowSlackUp}, ::PSY.TransmissionInterface, ::Type{ConstantMaxInterfaceFlow}) = 0.0 get_variable_lower_bound(::Type{InterfaceFlowSlackDown}, ::PSY.TransmissionInterface, ::Type{ConstantMaxInterfaceFlow}) = 0.0 -get_variable_multiplier(::Type{InterfaceFlowSlackUp}, ::Type{PSY.TransmissionInterface}, ::Type{ConstantMaxInterfaceFlow}) = 1.0 -get_variable_multiplier(::Type{InterfaceFlowSlackDown}, ::Type{PSY.TransmissionInterface}, ::Type{ConstantMaxInterfaceFlow}) = -1.0 - -get_variable_multiplier(::Type{InterfaceFlowSlackUp}, ::Type{PSY.TransmissionInterface}, ::Type{VariableMaxInterfaceFlow}) = 1.0 -get_variable_multiplier(::Type{InterfaceFlowSlackDown}, ::Type{PSY.TransmissionInterface}, ::Type{VariableMaxInterfaceFlow}) = -1.0 - get_multiplier_value(::Type{MinInterfaceFlowLimitParameter}, d::PSY.TransmissionInterface, ::Type{VariableMaxInterfaceFlow}) = PSY.get_min_active_power_flow_limit(d) get_multiplier_value(::Type{MaxInterfaceFlowLimitParameter}, d::PSY.TransmissionInterface, ::Type{VariableMaxInterfaceFlow}) = PSY.get_max_active_power_flow_limit(d) diff --git a/src/static_injector_models/hydro_generation.jl b/src/static_injector_models/hydro_generation.jl index 8b8d7f3..05ce1c4 100644 --- a/src/static_injector_models/hydro_generation.jl +++ b/src/static_injector_models/hydro_generation.jl @@ -4,8 +4,6 @@ requires_initialization(::AbstractHydroReservoirFormulation) = false requires_initialization(::AbstractHydroUnitCommitment) = true -get_variable_multiplier(::Type{<:VariableType}, ::Type{<:PSY.HydroGen}, ::Type{<:AbstractHydroFormulation}) = 1.0 -get_variable_multiplier(::Type{ActivePowerPumpVariable}, ::Type{<:PSY.HydroPumpTurbine}, ::Type{<:AbstractHydroPumpFormulation}) = -1.0 get_expression_type_for_reserve(::Type{ActivePowerReserveVariable}, ::Type{<:PSY.HydroGen}, ::Type{<:PSY.Reserve{PSY.ReserveUp}}) = ActivePowerRangeExpressionUB get_expression_type_for_reserve(::Type{ActivePowerReserveVariable}, ::Type{<:PSY.HydroGen}, ::Type{<:PSY.Reserve{PSY.ReserveDown}}) = ActivePowerRangeExpressionLB @@ -36,13 +34,11 @@ get_variable_upper_bound(::Type{EnergyVariable}, d::PSY.HydroGen, ::Type{<:Abstr get_variable_binary(::Type{ActivePowerInVariable}, ::Type{<:PSY.HydroGen}, ::Type{<:AbstractHydroReservoirFormulation}) = false get_variable_lower_bound(::Type{ActivePowerInVariable}, d::PSY.HydroGen, ::Type{<:AbstractHydroReservoirFormulation}) = 0.0 get_variable_upper_bound(::Type{ActivePowerInVariable}, d::PSY.HydroGen, ::Type{<:AbstractHydroReservoirFormulation}) = nothing -get_variable_multiplier(::Type{ActivePowerInVariable}, d::Type{<:PSY.HydroGen}, ::Type{<:AbstractHydroReservoirFormulation}) = -1.0 ########################### ActivePowerOutVariable, HydroGen ################################# get_variable_binary(::Type{ActivePowerOutVariable}, ::Type{<:PSY.HydroGen}, ::Type{<:AbstractHydroReservoirFormulation}) = false get_variable_lower_bound(::Type{ActivePowerOutVariable}, d::PSY.HydroGen, ::Type{<:AbstractHydroReservoirFormulation}) = 0.0 get_variable_upper_bound(::Type{ActivePowerOutVariable}, d::PSY.HydroGen, ::Type{<:AbstractHydroReservoirFormulation}) = nothing -get_variable_multiplier(::Type{ActivePowerOutVariable}, d::Type{<:PSY.HydroGen}, ::Type{<:AbstractHydroReservoirFormulation}) = 1.0 ############## OnVariable, HydroGen #################### # These methods are defined in PowerSimulations diff --git a/src/static_injector_models/reactivepower_device.jl b/src/static_injector_models/reactivepower_device.jl index ccc5b2b..becf6a4 100644 --- a/src/static_injector_models/reactivepower_device.jl +++ b/src/static_injector_models/reactivepower_device.jl @@ -1,7 +1,6 @@ #! format: off requires_initialization(::AbstractReactivePowerDeviceFormulation) = false -get_variable_multiplier(::Type{<:VariableType}, ::Type{<:PSY.SynchronousCondenser}, ::Type{<:AbstractReactivePowerDeviceFormulation}) = 1.0 ############## ReactivePowerVariable, SynchronousCondensers #################### get_variable_binary(::Type{ReactivePowerVariable}, ::Type{PSY.SynchronousCondenser}, ::Type{<:AbstractReactivePowerDeviceFormulation}) = false diff --git a/src/static_injector_models/renewable_generation.jl b/src/static_injector_models/renewable_generation.jl index a2e3a6c..9f1bb94 100644 --- a/src/static_injector_models/renewable_generation.jl +++ b/src/static_injector_models/renewable_generation.jl @@ -1,5 +1,4 @@ #! format: off -get_variable_multiplier(::Type{<:VariableType}, ::Type{<:PSY.RenewableGen}, ::Type{<:AbstractRenewableFormulation}) = 1.0 get_expression_type_for_reserve(::Type{ActivePowerReserveVariable}, ::Type{<:PSY.RenewableGen}, ::Type{<:PSY.Reserve{PSY.ReserveUp}}) = ActivePowerRangeExpressionUB get_expression_type_for_reserve(::Type{ActivePowerReserveVariable}, ::Type{<:PSY.RenewableGen}, ::Type{<:PSY.Reserve{PSY.ReserveDown}}) = ActivePowerRangeExpressionLB ########################### ActivePowerVariable, RenewableGen ################################# diff --git a/src/static_injector_models/source.jl b/src/static_injector_models/source.jl index 71eee08..fafa7bc 100644 --- a/src/static_injector_models/source.jl +++ b/src/static_injector_models/source.jl @@ -3,9 +3,6 @@ requires_initialization(::ImportExportSourceModel) = false -get_variable_multiplier(::Type{ActivePowerOutVariable}, ::Type{<:PSY.Source}, ::Type{<:AbstractSourceFormulation}) = 1.0 -get_variable_multiplier(::Type{ActivePowerInVariable}, ::Type{<:PSY.Source}, ::Type{<:AbstractSourceFormulation}) = -1.0 -get_variable_multiplier(::Type{ReactivePowerVariable}, ::Type{<:PSY.Source}, ::Type{<:AbstractSourceFormulation}) = 1.0 ############## ActivePowerVariables, Source #################### get_variable_binary(::Type{ActivePowerInVariable}, ::Type{<:PSY.Source}, ::Type{<:AbstractSourceFormulation}) = false get_variable_binary(::Type{ActivePowerOutVariable}, ::Type{<:PSY.Source}, ::Type{<:AbstractSourceFormulation}) = false diff --git a/src/static_injector_models/thermal_generation.jl b/src/static_injector_models/thermal_generation.jl index 6582331..74157c5 100644 --- a/src/static_injector_models/thermal_generation.jl +++ b/src/static_injector_models/thermal_generation.jl @@ -30,9 +30,7 @@ requires_initialization(::ThermalStandardDispatch) = true requires_initialization(::ThermalBasicCompactUnitCommitment) = false requires_initialization(::ThermalBasicUnitCommitment) = false -get_variable_multiplier(::Type{<:VariableType}, ::Type{<:PSY.ThermalGen}, ::Type{<:AbstractThermalFormulation}) = 1.0 # Per-device P_min multiplier computed inline at add_to_expression! call sites. -get_variable_multiplier(::Type{OnVariable}, ::Type{<:PSY.ThermalGen}, ::Type{<:Union{AbstractCompactUnitCommitment, ThermalCompactDispatch}}) = 1.0 get_expression_type_for_reserve(::Type{ActivePowerReserveVariable}, ::Type{<:PSY.ThermalGen}, ::Type{<:PSY.Reserve{PSY.ReserveUp}}) = ActivePowerRangeExpressionUB get_expression_type_for_reserve(::Type{ActivePowerReserveVariable}, ::Type{<:PSY.ThermalGen}, ::Type{<:PSY.Reserve{PSY.ReserveDown}}) = ActivePowerRangeExpressionLB diff --git a/src/twoterminal_hvdc_models/TwoTerminalDC_branches.jl b/src/twoterminal_hvdc_models/TwoTerminalDC_branches.jl index 5422b6f..f7bb6b1 100644 --- a/src/twoterminal_hvdc_models/TwoTerminalDC_branches.jl +++ b/src/twoterminal_hvdc_models/TwoTerminalDC_branches.jl @@ -33,7 +33,7 @@ get_variable_binary(::Type{HVDCPiecewiseBinaryLossVariable}, ::Type{<:PSY.TwoTer get_variable_binary(::Type{<:VariableType}, ::Type{<:PSY.TwoTerminalHVDC}, ::Type{<:AbstractTwoTerminalDCLineFormulation}) = false get_variable_binary(::Type{FlowActivePowerVariable}, ::Type{<:PSY.TwoTerminalHVDC}, ::Type{<:AbstractTwoTerminalDCLineFormulation}) = false get_variable_binary(::Type{HVDCFlowDirectionVariable}, ::Type{<:PSY.TwoTerminalHVDC}, ::Type{<:AbstractTwoTerminalDCLineFormulation}) = true -get_variable_multiplier(::Type{FlowActivePowerVariable}, ::Type{<:PSY.TwoTerminalHVDC}, ::Type{<:AbstractTwoTerminalDCLineFormulation}) = NaN +get_variable_multiplier(::Type{V}, ::Type{D}, ::Type{F}) where {V <: FlowActivePowerVariable, D <: PSY.TwoTerminalHVDC, F <: AbstractTwoTerminalDCLineFormulation} = _unsupported_multiplier(V, D, F) get_parameter_multiplier(::Type{FixValueParameter}, ::PSY.TwoTerminalHVDC, ::Type{<:AbstractTwoTerminalDCLineFormulation}) = 1.0 get_variable_multiplier(::Type{FlowActivePowerFromToVariable}, ::Type{<:PSY.TwoTerminalHVDC}, ::Type{<:AbstractTwoTerminalDCLineFormulation}) = -1.0 get_variable_multiplier(::Type{FlowActivePowerToFromVariable}, ::Type{<:PSY.TwoTerminalHVDC}, ::Type{<:AbstractTwoTerminalDCLineFormulation}) = -1.0 diff --git a/test/Project.toml b/test/Project.toml index b0085e0..14436d0 100644 --- a/test/Project.toml +++ b/test/Project.toml @@ -34,7 +34,7 @@ UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4" [sources] InfrastructureSystems = {url = "https://github.com/NREL-Sienna/InfrastructureSystems.jl", rev = "IS4"} PowerSystems = {url = "https://github.com/NREL-Sienna/PowerSystems.jl", rev = "psy6"} -InfrastructureOptimizationModels = {url = "https://github.com/NREL-Sienna/InfrastructureOptimizationModels.jl", rev = "ac/psi-costexp-parambroad-pfslack"} +InfrastructureOptimizationModels = {url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl", rev = "lk/flow-sign-trait"} PowerSystemCaseBuilder = {url = "https://github.com/NREL-Sienna/PowerSystemCaseBuilder.jl", rev = "psy6"} [compat] From 134d9c24d82b77fb311813061190ed65337d3760 Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Wed, 13 May 2026 21:35:58 -0600 Subject: [PATCH 2/3] Rename FlowSign variants and add multiplier-contract tests Mirror the IOM rename in POM's flow_sign dispatches and docstring. Add test/test_flow_sign_multipliers.jl pinning power-balance multiplier conventions from first principles. Address Copilot review on #117. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/core/interfaces.jl | 4 +- src/core/variables.jl | 10 +-- test/test_flow_sign_multipliers.jl | 108 +++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 test/test_flow_sign_multipliers.jl diff --git a/src/core/interfaces.jl b/src/core/interfaces.jl index 242e5cf..9f18abe 100644 --- a/src/core/interfaces.jl +++ b/src/core/interfaces.jl @@ -85,8 +85,8 @@ end Get the multiplier for a variable type when adding to an expression. Default consults [`IOM.flow_sign`](@ref) on the variable type: variables marked -`Injection` contribute `+1.0`, `Withdrawal` contributes `-1.0`, and `Unsigned` -falls back to `1.0`. Device-driven sign rules (e.g. anything on `PSY.ElectricLoad`) +`FlowInjection` contribute `+1.0`, `FlowWithdrawal` contributes `-1.0`, and +`FlowUndirected` falls back to `1.0`. Device-driven sign rules (e.g. anything on `PSY.ElectricLoad`) should be expressed as more-specific dispatches that override this default. """ function get_variable_multiplier( diff --git a/src/core/variables.jl b/src/core/variables.jl index ae829e1..f2e004e 100644 --- a/src/core/variables.jl +++ b/src/core/variables.jl @@ -101,8 +101,8 @@ Docs abbreviation: ``p^\\text{sl,dn}`` """ struct SystemBalanceSlackDown <: VariableType end -IOM.flow_sign(::Type{SystemBalanceSlackUp}) = IOM.Injection -IOM.flow_sign(::Type{SystemBalanceSlackDown}) = IOM.Withdrawal +IOM.flow_sign(::Type{SystemBalanceSlackUp}) = IOM.FlowInjection +IOM.flow_sign(::Type{SystemBalanceSlackDown}) = IOM.FlowWithdrawal """ Struct to dispatch the creation of Reserve requirement slack variables. Used when there is not reserves in the system to satisfy the requirement. @@ -479,8 +479,8 @@ Docs abbreviation: ``f^\\text{sl,dn}`` """ struct InterfaceFlowSlackDown <: VariableType end -IOM.flow_sign(::Type{InterfaceFlowSlackUp}) = IOM.Injection -IOM.flow_sign(::Type{InterfaceFlowSlackDown}) = IOM.Withdrawal +IOM.flow_sign(::Type{InterfaceFlowSlackUp}) = IOM.FlowInjection +IOM.flow_sign(::Type{InterfaceFlowSlackDown}) = IOM.FlowWithdrawal """ Struct to dispatch the creation of Slack variables for UpperBoundFeedforward @@ -568,7 +568,7 @@ Struct to dispatch the creation of a variable for pumped power in a hydro pump t """ struct ActivePowerPumpVariable <: VariableType end -IOM.flow_sign(::Type{ActivePowerPumpVariable}) = IOM.Withdrawal +IOM.flow_sign(::Type{ActivePowerPumpVariable}) = IOM.FlowWithdrawal """ Auxiliary Variable for Hydro Models that solve for total energy output diff --git a/test/test_flow_sign_multipliers.jl b/test/test_flow_sign_multipliers.jl new file mode 100644 index 0000000..da69c1f --- /dev/null +++ b/test/test_flow_sign_multipliers.jl @@ -0,0 +1,108 @@ +@testset "Power-balance multiplier contract" begin + # `get_variable_multiplier(V, D, F)` returns the coefficient with which a + # decision variable enters a power-balance expression. The contract below + # is the *meaning* of that coefficient — independent of which dispatches + # currently exist. If a test fails, the question to ask is "is the test + # wrong, or has someone added a dispatch that violates the convention?" + # + # Conventions (all from physics, not from the existing implementation): + # * Injection into the bus contributes +1.0 + # * Withdrawal from the bus contributes -1.0 + # * The "In/Out" naming on a port refers to the *device's* port: + # ActivePowerInVariable = power into device = grid withdrawal (-1) + # ActivePowerOutVariable = power out of device = grid injection (+1) + # * A device's nature can override the variable's default sign: + # ActivePowerVariable on a generator → +1 (the device injects) + # ActivePowerVariable on a load → -1 (the device withdraws) + # * Multipliers are exactly ±1.0 — no fractions, no NaN, no zero. + + combos = POM.generate_device_formulation_combinations() + + @testset "All defined multipliers are exactly ±1.0 (no NaN, no fractions)" begin + # The contract is: *if* `get_variable_multiplier` returns a number, that + # number must be exactly ±1.0. Triples with no defined power-balance + # role are expected to throw (via `_unsupported_multiplier`) — that's + # the desired behavior vs. silently returning NaN, so we treat throwing + # as a valid outcome here. + for V in ( + IOM.ActivePowerVariable, + IOM.ActivePowerInVariable, + IOM.ActivePowerOutVariable, + ) + for c in combos + D, F = c["device_type"], c["formulation"] + m = try + get_variable_multiplier(V, D, F) + catch + continue + end + @test m == 1.0 || m == -1.0 + @test !isnan(m) + end + end + end + + @testset "Generators inject: ActivePowerVariable contributes +1 on generator devices" begin + # Anything in PSY's StaticInjection-generator subtree should produce + # power, not consume it. + for c in combos + D, F = c["device_type"], c["formulation"] + D <: PSY.Generator || continue + @test get_variable_multiplier(IOM.ActivePowerVariable, D, F) == +1.0 + end + end + + @testset "Loads withdraw: ActivePowerVariable contributes -1 on load devices" begin + # Restrict to pairs whose formulation actually belongs to the load + # family. `generate_device_formulation_combinations` enumerates via + # `methodswith(...; supertypes=true)`, which also matches abstract + # supertype methods, so it emits pairings like + # `(PowerLoad, PhaseAngleControl)` that the codebase never instantiates. + for c in combos + D, F = c["device_type"], c["formulation"] + D <: PSY.ElectricLoad && F <: POM.AbstractLoadFormulation || continue + @test get_variable_multiplier(IOM.ActivePowerVariable, D, F) == -1.0 + end + end + + @testset "Port semantics: In = grid withdrawal, Out = grid injection" begin + # Only devices with two grid-side ports actually use In/Out variables. + # For everything else the call returns the trait default and the test + # would be vacuous (or worse, exercise nonsense pairings emitted by + # `generate_device_formulation_combinations`). + port_having = Union{PSY.Storage, PSY.HybridSystem, PSY.TwoTerminalHVDC} + for c in combos + D, F = c["device_type"], c["formulation"] + D <: port_having || continue + @test get_variable_multiplier(IOM.ActivePowerInVariable, D, F) == -1.0 + @test get_variable_multiplier(IOM.ActivePowerOutVariable, D, F) == +1.0 + end + end + + @testset "Balance slacks: Up closes a shortage (+), Down closes a surplus (-)" begin + # A slack on a power balance is a synthetic injection/withdrawal that + # closes an infeasibility. "Up" means "we need more power here" → + # it must enter the balance with the same sign as a generator. + @test get_variable_multiplier( + SystemBalanceSlackUp, PSY.System, CopperPlatePowerModel, + ) == +1.0 + @test get_variable_multiplier( + SystemBalanceSlackDown, PSY.System, CopperPlatePowerModel, + ) == -1.0 + @test get_variable_multiplier( + InterfaceFlowSlackUp, PSY.TransmissionInterface, CopperPlatePowerModel, + ) == +1.0 + @test get_variable_multiplier( + InterfaceFlowSlackDown, PSY.TransmissionInterface, CopperPlatePowerModel, + ) == -1.0 + end + + # TODO: purposefully unsupported combinations. Certain ones used to return NaN, + # which seems hazardous. Changed to error, but + # The new implementation hard-errors (via `_unsupported_multiplier`) on + # combinations that have no defined power-balance role, instead of silently + # returning NaN. We want a test that pins this contract — "no multiplier + # defined → throw, never NaN" — but choosing *which* triples count as + # semantically meaningless is itself a design decision that needs review + # before being codified. Defer until that conversation happens. +end From 0fc50288f198ec08632c91aa500c9aed3f1a08af Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Wed, 13 May 2026 21:46:16 -0600 Subject: [PATCH 3/3] cleanup test cases --- test/test_flow_sign_multipliers.jl | 49 ++++++------------------------ 1 file changed, 10 insertions(+), 39 deletions(-) diff --git a/test/test_flow_sign_multipliers.jl b/test/test_flow_sign_multipliers.jl index da69c1f..13aa8f0 100644 --- a/test/test_flow_sign_multipliers.jl +++ b/test/test_flow_sign_multipliers.jl @@ -1,29 +1,13 @@ @testset "Power-balance multiplier contract" begin # `get_variable_multiplier(V, D, F)` returns the coefficient with which a # decision variable enters a power-balance expression. The contract below - # is the *meaning* of that coefficient — independent of which dispatches - # currently exist. If a test fails, the question to ask is "is the test - # wrong, or has someone added a dispatch that violates the convention?" - # - # Conventions (all from physics, not from the existing implementation): - # * Injection into the bus contributes +1.0 - # * Withdrawal from the bus contributes -1.0 - # * The "In/Out" naming on a port refers to the *device's* port: - # ActivePowerInVariable = power into device = grid withdrawal (-1) - # ActivePowerOutVariable = power out of device = grid injection (+1) - # * A device's nature can override the variable's default sign: - # ActivePowerVariable on a generator → +1 (the device injects) - # ActivePowerVariable on a load → -1 (the device withdraws) - # * Multipliers are exactly ±1.0 — no fractions, no NaN, no zero. + # is the *meaning* of that coefficient--add tests based on the intended behavior, + # not the implementation. combos = POM.generate_device_formulation_combinations() @testset "All defined multipliers are exactly ±1.0 (no NaN, no fractions)" begin - # The contract is: *if* `get_variable_multiplier` returns a number, that - # number must be exactly ±1.0. Triples with no defined power-balance - # role are expected to throw (via `_unsupported_multiplier`) — that's - # the desired behavior vs. silently returning NaN, so we treat throwing - # as a valid outcome here. + # Triples with no defined power-balance throw; defined ones are exactly ±1.0. for V in ( IOM.ActivePowerVariable, IOM.ActivePowerInVariable, @@ -53,11 +37,7 @@ end @testset "Loads withdraw: ActivePowerVariable contributes -1 on load devices" begin - # Restrict to pairs whose formulation actually belongs to the load - # family. `generate_device_formulation_combinations` enumerates via - # `methodswith(...; supertypes=true)`, which also matches abstract - # supertype methods, so it emits pairings like - # `(PowerLoad, PhaseAngleControl)` that the codebase never instantiates. + # restrict to sensible formulations for loads. for c in combos D, F = c["device_type"], c["formulation"] D <: PSY.ElectricLoad && F <: POM.AbstractLoadFormulation || continue @@ -65,15 +45,11 @@ end end - @testset "Port semantics: In = grid withdrawal, Out = grid injection" begin - # Only devices with two grid-side ports actually use In/Out variables. - # For everything else the call returns the trait default and the test - # would be vacuous (or worse, exercise nonsense pairings emitted by - # `generate_device_formulation_combinations`). - port_having = Union{PSY.Storage, PSY.HybridSystem, PSY.TwoTerminalHVDC} + @testset "Bidirectional: In = grid withdrawal, Out = grid injection" begin + bidirectional = Union{PSY.Storage, PSY.HybridSystem, PSY.Source} for c in combos D, F = c["device_type"], c["formulation"] - D <: port_having || continue + D <: bidirectional || continue @test get_variable_multiplier(IOM.ActivePowerInVariable, D, F) == -1.0 @test get_variable_multiplier(IOM.ActivePowerOutVariable, D, F) == +1.0 end @@ -97,12 +73,7 @@ ) == -1.0 end - # TODO: purposefully unsupported combinations. Certain ones used to return NaN, - # which seems hazardous. Changed to error, but - # The new implementation hard-errors (via `_unsupported_multiplier`) on - # combinations that have no defined power-balance role, instead of silently - # returning NaN. We want a test that pins this contract — "no multiplier - # defined → throw, never NaN" — but choosing *which* triples count as - # semantically meaningless is itself a design decision that needs review - # before being codified. Defer until that conversation happens. + # TODO: check that purposefully unsupported combinations error. + # Certain ones used to return NaN, which seems hazardous. Changed to error, but not + # sure exactly which should error long-term versus just "unimplemented for now" end