diff --git a/src/common_models/add_expressions.jl b/src/common_models/add_expressions.jl index 3ae8a7e4..89df0aa9 100644 --- a/src/common_models/add_expressions.jl +++ b/src/common_models/add_expressions.jl @@ -218,15 +218,10 @@ function add_expressions!( end """ -Per-service cost expression container (e.g. `ProductionCostExpression` for ORDC reserves). -One container per service, `meta`-keyed by the service name, matching the reads in +Merged cost-expression container for reserve services (e.g. `ProductionCostExpression` for ORDC): +one dense `(service_name, time)` container per service type, empty meta - the same shape as the +device `ProductionCostExpression`. Read/written by `add_to_expression!(container, ::CostExpressions, cost, ::ReserveDemand*, t)`. - -TODO(services PWL cost): this per-service `meta` keying is temporary. It will be -removed/folded to a per-type dense `(service, time)` container (like the device -`ProductionCostExpression`) once the 3D/4D PWL cost containers for services land. Kept on -`meta` with the rest of the ORDC cost path (slope/breakpoint params, delta-PWL machinery) -until that migration. """ function add_expressions!( container::OptimizationContainer, @@ -240,9 +235,12 @@ function add_expressions!( W <: AbstractReservesFormulation, } where {D <: PSY.Component} time_steps = get_time_steps(container) - for service in services - name = PSY.get_name(service) - add_expression_container!(container, T, D, [name], time_steps; meta = name) - end + add_expression_container!( + container, + T, + D, + [PSY.get_name(s) for s in services], + time_steps, + ) return end diff --git a/src/common_models/add_to_expression.jl b/src/common_models/add_to_expression.jl index 8c8550bd..7096fed7 100644 --- a/src/common_models/add_to_expression.jl +++ b/src/common_models/add_to_expression.jl @@ -2021,7 +2021,10 @@ function add_to_expression!( } expression = get_expression(container, InterfaceTotalFlow, PSY.TransmissionInterface) service_name = PSY.get_name(service) - variable = get_variable(container, T, PSY.TransmissionInterface, service_name) + # DELETE-AFTER-REVIEW: reviewer context on the container change; remove once the PR is approved. + # Merged slack container keyed `(interface_name, time)`; read the whole container and index + # this interface's slice (was a per-service `meta = service_name` container). + variable = get_variable(container, T, PSY.TransmissionInterface) time_steps = get_time_steps(container) for t in time_steps add_proportional_to_jump_expression!( @@ -2974,10 +2977,8 @@ function add_to_expression!( return end -# TODO(services PWL cost): reads the per-service `meta = service_name` ORDC cost-expression -# container. Update to the per-type folded container when the 3D/4D PWL cost containers for -# services land (see the create site in add_expressions.jl and the ORDC slope/breakpoint -# params in add_parameters.jl -- all kept on `meta` together until that migration). +# Merged dense `(service_name, time)` ORDC cost-expression container (empty meta), read/written +# by service name - same shape as the device `ProductionCostExpression` above. function add_to_expression!( container::OptimizationContainer, ::Type{S}, @@ -2988,9 +2989,8 @@ function add_to_expression!( S <: CostExpressions, T <: Union{PSY.ReserveDemandCurve, PSY.ReserveDemandTimeSeriesCurve}, } - if has_container_key(container, S, T, PSY.get_name(component)) - device_cost_expression = - get_expression(container, S, T, PSY.get_name(component)) + if has_container_key(container, S, T) + device_cost_expression = get_expression(container, S, T) component_name = PSY.get_name(component) JuMP.add_to_expression!( device_cost_expression[component_name, time_period], diff --git a/src/core/problem_template.jl b/src/core/problem_template.jl index f82c09ca..e33448c9 100644 --- a/src/core/problem_template.jl +++ b/src/core/problem_template.jl @@ -242,8 +242,8 @@ function _populate_contributing_devices!( # populating each reserve we require at least one such device: a modeled reserve with no # available provider can never meet its requirement - it would silently force slacks or # make the model infeasible - so error loudly and name it rather than dropping it. - # Non-reserve services (ConstantReserveGroup, TransmissionInterface, AGC) draw on other - # services or branches, not provider devices, so they are exempt from the check. + # A modeled TransmissionInterface likewise needs at least one available contributing branch, + # or its flow limit is meaningless. ConstantReserveGroup is exempt (see the check below). for (service_key, service_model) in service_models @debug "Populating service model $(service_key)" empty!(get_contributing_devices_map(service_model)) @@ -265,14 +265,15 @@ function _populate_contributing_devices!( ) end end - # TODO(transmission interface, Q5): the check is reserve-scoped, so a - # TransmissionInterface with no contributing branches (or all-unavailable - # branches) still populates silently. Extend an equivalent loud error to the - # interface path when the interface migration lands. - if service_type <: PSY.Reserve && + # Exempt ConstantReserveGroup: a GroupReserve aggregates other services, so its + # contributing-device map is empty by design; without this the error would fire on + # every group reserve (the bug that previously made ConstantReserveGroup unbuildable). + # Reserves and transmission interfaces DO draw on devices/branches, so an empty map + # there is a real misconfiguration. AGC is not modeled in POM, so it never reaches here. + if !(service_type <: PSY.ConstantReserveGroup) && isempty(get_contributing_devices_map(service_model, service_name)) error( - "Reserve service \"$(service_name)\" of type $(typeof(service)) has no available contributing devices. Assign available contributing devices to it in the system data, or remove its service model from the template.", + "Service \"$(service_name)\" of type $(typeof(service)) has no available contributing devices/branches. Assign available contributing devices/branches to it in the system data, or remove its service model from the template.", ) end end diff --git a/src/services_models/service_slacks.jl b/src/services_models/service_slacks.jl index d4522d80..ec3ee338 100644 --- a/src/services_models/service_slacks.jl +++ b/src/services_models/service_slacks.jl @@ -26,32 +26,39 @@ end function transmission_interface_slacks!( container::OptimizationContainer, - service::T, + services::Vector{T}, ) where {T <: PSY.TransmissionInterface} time_steps = get_time_steps(container) - name = PSY.get_name(service) + interface_names = [PSY.get_name(s) for s in services] + jump_model = get_jump_model(container) + # DELETE-AFTER-REVIEW: reviewer context on the container change; remove once the PR is approved. + # One dense 2D container per (slack variable type, TransmissionInterface) keyed + # `[interface_name, time]`, built once over all interfaces (`use_slacks` is per type), + # empty meta. Each interface's slacks carry its own violation penalty. for variable_type in [InterfaceFlowSlackUp, InterfaceFlowSlackDown] - variable = add_variable_container!( - container, - variable_type, - T, - [name], - time_steps; - meta = name, - ) - penalty = PSY.get_violation_penalty(service) - for t in time_steps - variable[name, t] = JuMP.@variable( - get_jump_model(container), - base_name = "$(T)_$(variable_type)_{$(name), $(t)}", - ) - JuMP.set_lower_bound(variable[name, t], 0.0) - - add_to_objective_invariant_expression!( + variable = + add_variable_container!( container, - variable[name, t] * penalty, + variable_type, + T, + interface_names, + time_steps, ) + for service in services + name = PSY.get_name(service) + penalty = PSY.get_violation_penalty(service) + for t in time_steps + variable[name, t] = JuMP.@variable( + jump_model, + base_name = "$(T)_$(variable_type)_{$(name), $(t)}", + lower_bound = 0.0, + ) + add_to_objective_invariant_expression!( + container, + variable[name, t] * penalty, + ) + end end end diff --git a/src/services_models/services_constructor.jl b/src/services_models/services_constructor.jl index 6c28369f..9c19c78c 100644 --- a/src/services_models/services_constructor.jl +++ b/src/services_models/services_constructor.jl @@ -239,6 +239,8 @@ function construct_service!( services, StepwiseCostReserve(), ) + # Merged dense `(service, time)` cost-expression container, built once over all services. + add_expressions!(container, ProductionCostExpression, services, model) for service in services contributing_devices = get_contributing_devices(model, PSY.get_name(service)) add_service_variables!( @@ -256,7 +258,6 @@ function construct_service!( model, devices_template, ) - add_expressions!(container, ProductionCostExpression, [service], model) end return end @@ -607,18 +608,17 @@ function construct_service!( incompatible_device_types::Set{<:DataType}, network_model::NetworkModel{<:AbstractNetworkModel}, ) where {T <: PSY.TransmissionInterface} - interfaces = get_available_components(model, sys) + interfaces = collect(get_available_components(model, sys)) # Lazy container addition for the expressions. lazy_container_addition!(container, InterfaceTotalFlow, T, PSY.get_name.(interfaces), get_time_steps(container), ) + if get_use_slacks(model) + transmission_interface_slacks!(container, interfaces) + end for interface in interfaces - if get_use_slacks(model) - # Adding the slacks can be done in a cleaner fashion - transmission_interface_slacks!(container, interface) - end add_feedforward_arguments!(container, model, interface) end return @@ -633,7 +633,7 @@ function construct_service!( incompatible_device_types::Set{<:DataType}, network_model::NetworkModel{AreaBalanceNetworkModel}, ) - interfaces = get_available_components(model, sys) + interfaces = collect(get_available_components(model, sys)) # Lazy container addition for the expressions. lazy_container_addition!(container, InterfaceTotalFlow, PSY.TransmissionInterface, @@ -641,11 +641,10 @@ function construct_service!( get_time_steps(container), ) @warn "AreaBalanceNetworkModel doesn't model individual line flows and it ignores the flows on AC Transmission Devices" + if get_use_slacks(model) + transmission_interface_slacks!(container, interfaces) + end for interface in interfaces - if get_use_slacks(model) - # Adding the slacks can be done in a cleaner fashion - transmission_interface_slacks!(container, interface) - end add_feedforward_arguments!(container, model, interface) end return @@ -864,7 +863,7 @@ function construct_service!( incompatible_device_types::Set{<:DataType}, network_model::NetworkModel{<:AbstractNetworkModel}, ) - interfaces = get_available_components(model, sys) + interfaces = collect(get_available_components(model, sys)) # Lazy container addition for the expressions. lazy_container_addition!(container, InterfaceTotalFlow, PSY.TransmissionInterface, @@ -877,12 +876,11 @@ function construct_service!( "Not all TransmissionInterfaces devices have time series. Check data to complete (or remove) time series.", ) end - for interface in interfaces - if get_use_slacks(model) - # Adding the slacks can be done in a cleaner fashion - transmission_interface_slacks!(container, interface) - end - if all(has_ts) + if get_use_slacks(model) + transmission_interface_slacks!(container, interfaces) + end + if !isempty(interfaces) && all(has_ts) + for interface in interfaces name = PSY.get_name(interface) num_ts = length(unique(PSY.get_name.(PSY.get_time_series_keys(interface)))) if num_ts < 2 @@ -890,9 +888,13 @@ function construct_service!( "TransmissionInterface $name has less than two time series. It is required to add both min_flow and max_flow time series.", ) end - add_parameters!(container, MinInterfaceFlowLimitParameter, interface, model) - add_parameters!(container, MaxInterfaceFlowLimitParameter, interface, model) end + # Merged per-type parameter containers over all interfaces (empty meta), filled per + # interface by the vector `_add_parameters!` path. + add_parameters!(container, MinInterfaceFlowLimitParameter, interfaces, model) + add_parameters!(container, MaxInterfaceFlowLimitParameter, interfaces, model) + end + for interface in interfaces add_feedforward_arguments!(container, model, interface) end return diff --git a/src/services_models/transmission_interface.jl b/src/services_models/transmission_interface.jl index 3b8a8287..66840ddb 100644 --- a/src/services_models/transmission_interface.jl +++ b/src/services_models/transmission_interface.jl @@ -101,17 +101,23 @@ function add_constraints!( meta = "lb", ) int_name = PSY.get_name(interface) + # DELETE-AFTER-REVIEW: reviewer context on the container change; remove once the PR is approved. + # Merged per-type parameter containers keyed by interface name (empty meta); fetch the whole + # container/multiplier array and index this interface's column/row (was a per-service + # `meta = int_name` container). param_container_min = - get_parameter(container, MinInterfaceFlowLimitParameter, PSY.TransmissionInterface, int_name) - param_multiplier_min = get_parameter_multiplier_array(container, MinInterfaceFlowLimitParameter, + get_parameter(container, MinInterfaceFlowLimitParameter, PSY.TransmissionInterface) + param_multiplier_min = get_parameter_multiplier_array( + container, + MinInterfaceFlowLimitParameter, PSY.TransmissionInterface, - int_name, ) param_container_max = - get_parameter(container, MaxInterfaceFlowLimitParameter, PSY.TransmissionInterface, int_name) - param_multiplier_max = get_parameter_multiplier_array(container, MaxInterfaceFlowLimitParameter, + get_parameter(container, MaxInterfaceFlowLimitParameter, PSY.TransmissionInterface) + param_multiplier_max = get_parameter_multiplier_array( + container, + MaxInterfaceFlowLimitParameter, PSY.TransmissionInterface, - int_name, ) param_min = get_parameter_column_refs(param_container_min, int_name) param_max = get_parameter_column_refs(param_container_max, int_name) diff --git a/test/test_services_constructor.jl b/test/test_services_constructor.jl index d4784ca3..7f413abc 100644 --- a/test/test_services_constructor.jl +++ b/test/test_services_constructor.jl @@ -727,6 +727,122 @@ end end end +@testset "Interface slacks are one merged container per type (use_slacks)" begin + # `use_slacks = true` on the interface ServiceModel builds InterfaceFlowSlackUp/Down as one + # dense container per (variable type, TransmissionInterface) keyed by interface name (empty + # meta), each wired to the interface's violation penalty. `deepcopy` so the added interface + # does not leak into the PSB cache. + # DELETE-AFTER-REVIEW: reviewer context on the container change; remove once the PR is approved. + # Ports the previously-blocked ServiceModel use_slacks case (the old per-name + meta build had + # no working container-builder method). + sys = deepcopy(PSB.build_system(PSITestSystems, "c_sys5_uc"; add_reserves = true)) + interface = TransmissionInterface(; + name = "west_east", + available = true, + active_power_flow_limits = (min = 0.0, max = 400.0), + violation_penalty = 1e5, + ) + add_service!(sys, interface, [get_component(Line, sys, l) for l in ("1", "2", "6")]) + + template = get_thermal_dispatch_template_network(DCPNetworkModel) + set_service_model!( + template, + ServiceModel(TransmissionInterface, ConstantMaxInterfaceFlow; use_slacks = true), + ) + model = DecisionModel(template, sys; optimizer = HiGHS_optimizer) + @test build!(model; output_dir = mktempdir(; cleanup = true)) == + IOM.ModelBuildStatus.BUILT + + container = get_optimization_container(model) + time_steps = get_time_steps(container) + for V in (POM.InterfaceFlowSlackUp, POM.InterfaceFlowSlackDown) + # 3-arg fetch (no meta) proves the merged, empty-meta container. + slack = IOM.get_variable(container, V, TransmissionInterface) + @test axes(slack) == (["west_east"], time_steps) + @test all(JuMP.lower_bound(slack["west_east", t]) == 0.0 for t in time_steps) + end + # Slacks are wired into the objective at the interface's violation penalty. + obj = JuMP.objective_function(get_jump_model(model)) + slack_up = IOM.get_variable(container, POM.InterfaceFlowSlackUp, TransmissionInterface) + @test all(JuMP.coefficient(obj, slack_up["west_east", t]) == 1e5 for t in time_steps) +end + +@testset "Interface flow-limit params are one merged container per type" begin + # VariableMaxInterfaceFlow builds Min/MaxInterfaceFlowLimitParameter as one container per type + # over all interface names (empty meta). + # DELETE-AFTER-REVIEW: reviewer context on the container change; remove once the PR is approved. + # The 3-arg fetch below would throw if the container were still per-service `meta = name`. + sys = deepcopy(PSB.build_system(PSITestSystems, "c_sys5_uc"; add_reserves = true)) + interface = TransmissionInterface(; + name = "west_east", + available = true, + active_power_flow_limits = (min = 0.0, max = 400.0), + ) + add_service!(sys, interface, [get_component(Line, sys, l) for l in ("1", "2", "6")]) + for (ts_name, sfm) in ( + ("min_active_power_flow_limit", PSY.get_min_active_power_flow_limit), + ("max_active_power_flow_limit", PSY.get_max_active_power_flow_limit), + ) + data = Dict( + DateTime("2024-01-01T00:00:00") => fill(0.5, 24), + DateTime("2024-01-02T00:00:00") => fill(0.5, 24), + ) + add_time_series!( + sys, + interface, + Deterministic(ts_name, data, Hour(1); scaling_factor_multiplier = sfm), + ) + end + + template = get_thermal_dispatch_template_network(DCPNetworkModel) + set_service_model!( + template, + ServiceModel(TransmissionInterface, VariableMaxInterfaceFlow), + ) + model = DecisionModel(template, sys; optimizer = HiGHS_optimizer) + @test build!(model; output_dir = mktempdir(; cleanup = true)) == + IOM.ModelBuildStatus.BUILT + + container = get_optimization_container(model) + for P in (POM.MinInterfaceFlowLimitParameter, POM.MaxInterfaceFlowLimitParameter) + # 3-arg fetch (no meta) succeeds only if the container is merged per type. + param = IOM.get_parameter(container, P, TransmissionInterface) + @test param !== nothing + end + # The flow-limit constraint (which reads the merged params) built for the interface. + @test size( + IOM.get_constraint(container, POM.InterfaceFlowLimit, TransmissionInterface, "ub"), + ) == (1, 24) +end + +@testset "Interface with no available contributing branches errors" begin + # A3: an interface whose contributing branches are all unavailable has an empty contributing + # map, so `_populate_contributing_devices!` (in the DecisionModel constructor) must error and + # name it rather than silently building a meaningless flow limit. + sys = deepcopy(PSB.build_system(PSITestSystems, "c_sys5_uc"; add_reserves = true)) + interface = TransmissionInterface(; + name = "west_east", + available = true, + active_power_flow_limits = (min = 0.0, max = 400.0), + ) + interface_lines = [get_component(Line, sys, l) for l in ("1", "2", "6")] + add_service!(sys, interface, interface_lines) + for l in interface_lines + PSY.set_available!(l, false) + end + + template = get_thermal_dispatch_template_network(DCPNetworkModel) + set_service_model!( + template, + ServiceModel(TransmissionInterface, ConstantMaxInterfaceFlow), + ) + @test_throws "no available contributing devices/branches" DecisionModel( + template, + sys; + optimizer = HiGHS_optimizer, + ) +end + @testset "Test Interfaces on Interchanges with AreaBalance" begin sys_rts_da = build_system(PSISystems, "modified_RTS_GMLC_DA_sys") transform_single_time_series!(sys_rts_da, Hour(24), Hour(1)) @@ -1115,11 +1231,6 @@ end # - "Test Reserves with Feedforwards": the concrete feedforward types # (`LowerBoundFeedforward`, `FixValueFeedforward`, …) are not defined in POM or IOM — # only the feedforward constraint types and the abstract construct hooks exist. -# - TransmissionInterface with `use_slacks = true` on the ServiceModel: the interface slack -# construction calls `add_variable_container!(..., InterfaceFlowSlackUp, -# TransmissionInterface, ::String, ::UnitRange)`, which has no method — build returns -# FAILED. The landed interface testsets omit ServiceModel slacks; the slack path needs a -# src/IOM container-builder fix. # Also not ported (feature/framework not in POM): AGC (no `template_agc_reserve_deployment`), # Hydro reserves (`HydroTurbineEnergyDispatch` absent), the old bare-`TimeSeriesKey` ORDC # tests (psy6 uses `ReserveDemandTimeSeriesCurve`; covered by the two ORDC testsets above),