From 5992977a88b5315d2262c17fa9c90aa4acbcf184 Mon Sep 17 00:00:00 2001 From: rodrigomha Date: Thu, 23 Jul 2026 14:05:44 -0700 Subject: [PATCH 1/5] Phase A: merge TransmissionInterface slack + flow-limit-param containers Migrate the two remaining meta = interface_name interface containers to merged per-type containers (keyed by interface name, empty meta), matching the reserve refactor: - InterfaceFlowSlackUp/Down: one dense (interface_name, time) container per type, built once over all interfaces with per-interface penalty (service_slacks.jl); constructors call it once, and the slack fold reads it 3-arg. This also closes the use_slacks-on-ServiceModel gap (moving to the vector-axis builder reserves already use), so that path is now testable. - Min/MaxInterfaceFlowLimitParameter: routed to the merged vector _add_parameters! (empty meta); the VariableMax InterfaceFlowLimit constraint reads them 3-arg and indexes by interface name. InterfaceTotalFlow (already name-keyed) and InterfaceFlowLimit (meta = ub/lb direction) are unchanged. A3: extend the no-contributing-devices loud error to TransmissionInterface (no available contributing branches), parallel to the reserve arm. Tests: merged slack shape + penalty wiring (ports the previously-blocked use_slacks case), merged param shape, and a no-contributing-branch error; removed the stale NOT-PORTED note. Full suite green except the known converter parallel-flake (test_converter_network_coverage, passes in isolation). Interface construction verified by smoke test (mapping + containers + solve). --- src/common_models/add_to_expression.jl | 4 +- src/core/problem_template.jl | 24 ++-- src/services_models/service_slacks.jl | 46 ++++--- src/services_models/services_constructor.jl | 39 +++--- src/services_models/transmission_interface.jl | 17 ++- test/test_services_constructor.jl | 118 +++++++++++++++++- 6 files changed, 186 insertions(+), 62 deletions(-) diff --git a/src/common_models/add_to_expression.jl b/src/common_models/add_to_expression.jl index 8c8550bd..e3feb9dd 100644 --- a/src/common_models/add_to_expression.jl +++ b/src/common_models/add_to_expression.jl @@ -2021,7 +2021,9 @@ 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) + # 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!( diff --git a/src/core/problem_template.jl b/src/core/problem_template.jl index f82c09ca..4e76c184 100644 --- a/src/core/problem_template.jl +++ b/src/core/problem_template.jl @@ -242,8 +242,9 @@ 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 and AGC draw on other services (not + # devices/branches) and stay exempt. for (service_key, service_model) in service_models @debug "Populating service model $(service_key)" empty!(get_contributing_devices_map(service_model)) @@ -265,15 +266,16 @@ 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 && - 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.", - ) + if isempty(get_contributing_devices_map(service_model, service_name)) + if service_type <: PSY.Reserve + 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.", + ) + elseif service_type <: PSY.TransmissionInterface + error( + "Transmission interface \"$(service_name)\" of type $(typeof(service)) has no available contributing branches. Assign available contributing branches to it in the system data, or remove its service model from the template.", + ) + end end end end diff --git a/src/services_models/service_slacks.jl b/src/services_models/service_slacks.jl index d4522d80..3c781367 100644 --- a/src/services_models/service_slacks.jl +++ b/src/services_models/service_slacks.jl @@ -26,32 +26,38 @@ 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) + # 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..12c02e30 100644 --- a/src/services_models/services_constructor.jl +++ b/src/services_models/services_constructor.jl @@ -607,18 +607,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 +632,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 +640,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 +862,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 +875,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 +887,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..73d416d6 100644 --- a/src/services_models/transmission_interface.jl +++ b/src/services_models/transmission_interface.jl @@ -101,17 +101,22 @@ function add_constraints!( meta = "lb", ) int_name = PSY.get_name(interface) + # 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..445cfc28 100644 --- a/test/test_services_constructor.jl +++ b/test/test_services_constructor.jl @@ -727,6 +727,119 @@ 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. Ports the previously-blocked + # ServiceModel use_slacks case (the old per-name + meta build had no working container-builder + # method). `deepcopy` so the added interface does not leak into the PSB cache. + 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). 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 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 +1228,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), From fb0e53ab0e26d010eb76a438db52f6a9b8d94c8e Mon Sep 17 00:00:00 2001 From: rodrigomha Date: Thu, 23 Jul 2026 14:35:19 -0700 Subject: [PATCH 2/5] Flag Phase A old-per-service-meta comments for post-approval removal Mark the reviewer-context comments added in Phase A that narrate the old per-service meta interface containers (the slack read, the flow-limit-param read, and the two interface test comments) with DELETE-AFTER-REVIEW, keeping the durable design/test descriptions. grep the tag to remove them once the PR is approved. --- src/common_models/add_to_expression.jl | 1 + src/services_models/transmission_interface.jl | 1 + test/test_services_constructor.jl | 13 ++++++++----- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/common_models/add_to_expression.jl b/src/common_models/add_to_expression.jl index e3feb9dd..5cf8bbe0 100644 --- a/src/common_models/add_to_expression.jl +++ b/src/common_models/add_to_expression.jl @@ -2021,6 +2021,7 @@ function add_to_expression!( } expression = get_expression(container, InterfaceTotalFlow, PSY.TransmissionInterface) service_name = PSY.get_name(service) + # 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) diff --git a/src/services_models/transmission_interface.jl b/src/services_models/transmission_interface.jl index 73d416d6..66840ddb 100644 --- a/src/services_models/transmission_interface.jl +++ b/src/services_models/transmission_interface.jl @@ -101,6 +101,7 @@ 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). diff --git a/test/test_services_constructor.jl b/test/test_services_constructor.jl index 445cfc28..bb27765f 100644 --- a/test/test_services_constructor.jl +++ b/test/test_services_constructor.jl @@ -730,9 +730,11 @@ 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. Ports the previously-blocked - # ServiceModel use_slacks case (the old per-name + meta build had no working container-builder - # method). `deepcopy` so the added interface does not leak into the PSB cache. + # 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", @@ -767,8 +769,9 @@ 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). The 3-arg fetch below would throw if the container - # were still per-service `meta = name`. + # 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", From 1ab5a3092ca2dc735c5d0512b05cf89c974203be Mon Sep 17 00:00:00 2001 From: rodrigomha Date: Thu, 23 Jul 2026 14:52:48 -0700 Subject: [PATCH 3/5] Merge the no-contributing-devices/branches error into one, exempt ConstantReserveGroup Collapse the reserve/interface arms into a single error ("... no available contributing devices/branches ...") and guard it with !(service_type <: PSY.ConstantReserveGroup) instead of a positive Reserve/TransmissionInterface allow-list. A GroupReserve aggregates other services, so its contributing-device map is empty by design; the exemption keeps it buildable (the bug that previously blocked ConstantReserveGroup). Reserves and interfaces do draw on devices/branches, so an empty map there stays a loud error. AGC is not modeled in POM, so it never reaches here. Comment above the check explains the exemption. Update the interface error test substring to match the merged message. --- src/core/problem_template.jl | 23 +++++++++++------------ test/test_services_constructor.jl | 2 +- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/core/problem_template.jl b/src/core/problem_template.jl index 4e76c184..e33448c9 100644 --- a/src/core/problem_template.jl +++ b/src/core/problem_template.jl @@ -243,8 +243,7 @@ function _populate_contributing_devices!( # 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. # A modeled TransmissionInterface likewise needs at least one available contributing branch, - # or its flow limit is meaningless. ConstantReserveGroup and AGC draw on other services (not - # devices/branches) and stay exempt. + # 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)) @@ -266,16 +265,16 @@ function _populate_contributing_devices!( ) end end - if isempty(get_contributing_devices_map(service_model, service_name)) - if service_type <: PSY.Reserve - 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.", - ) - elseif service_type <: PSY.TransmissionInterface - error( - "Transmission interface \"$(service_name)\" of type $(typeof(service)) has no available contributing branches. Assign available contributing branches to it in the system data, or remove its service model from the template.", - ) - end + # 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( + "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 end diff --git a/test/test_services_constructor.jl b/test/test_services_constructor.jl index bb27765f..7f413abc 100644 --- a/test/test_services_constructor.jl +++ b/test/test_services_constructor.jl @@ -836,7 +836,7 @@ end template, ServiceModel(TransmissionInterface, ConstantMaxInterfaceFlow), ) - @test_throws "no available contributing branches" DecisionModel( + @test_throws "no available contributing devices/branches" DecisionModel( template, sys; optimizer = HiGHS_optimizer, From 8e1721b4fd5a66cda9085285ff5fc12a72381747 Mon Sep 17 00:00:00 2001 From: rodrigomha Date: Thu, 23 Jul 2026 15:51:55 -0700 Subject: [PATCH 4/5] Flag the merged interface-slack comment for post-approval removal Per Phase A review: mark the transmission_interface_slacks! merged-container comment with DELETE-AFTER-REVIEW (reviewer context on the container change). --- src/services_models/service_slacks.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services_models/service_slacks.jl b/src/services_models/service_slacks.jl index 3c781367..ec3ee338 100644 --- a/src/services_models/service_slacks.jl +++ b/src/services_models/service_slacks.jl @@ -32,6 +32,7 @@ function transmission_interface_slacks!( 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. From 9b3b8e24e1f66f0405205f148a42bbc795e14d3d Mon Sep 17 00:00:00 2001 From: rodrigomha Date: Thu, 23 Jul 2026 16:51:26 -0700 Subject: [PATCH 5/5] Phase B: fold ORDC ProductionCostExpression into a merged dense (service, time) container Migrate the ORDC (StepwiseCostReserve) production-cost expression off the per-service meta = name container to one dense (service, time) container per service type, empty meta - the same shape as the device ProductionCostExpression: - add_expressions.jl builds one merged container over all the type's services; - the StepwiseCostReserve constructor calls add_expressions! once (out of the per-service loop); - the ORDC add_to_expression! read/write is now 3-arg (no meta). The ORDC block-offer variables/constraints are already 3D (service, segment, time) merged; the slope/breakpoint PWL params stay on meta for now - merging them needs a shared/padded tranche axis and is coupled with the delta-PWL machinery, so it lands with the 4D AS-offer work (Phase C). Both ORDC testsets (build + build&solve with two different-tranche ORDCs) green. --- src/common_models/add_expressions.jl | 22 ++++++++++----------- src/common_models/add_to_expression.jl | 11 ++++------- src/services_models/services_constructor.jl | 3 ++- 3 files changed, 16 insertions(+), 20 deletions(-) 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 5cf8bbe0..7096fed7 100644 --- a/src/common_models/add_to_expression.jl +++ b/src/common_models/add_to_expression.jl @@ -2977,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}, @@ -2991,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/services_models/services_constructor.jl b/src/services_models/services_constructor.jl index 12c02e30..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