Phase A+B: TransmissionInterface merge + ORDC cost-expression fold (stacked on #206) - #207
Merged
rodrigomha merged 5 commits intoJul 30, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Continues the per-type merged-container migration introduced in #206 by consolidating TransmissionInterface slack/parameter containers and folding ORDC (StepwiseCostReserve) ProductionCostExpression into a single dense per-type container, while adding tests that exercise the previously untestable use_slacks ServiceModel path for interfaces.
Changes:
- Build
TransmissionInterfaceslack variables as one dense(interface_name, time)container per slack type (empty meta), and wire per-interface violation penalties into the objective. - Merge
Min/MaxInterfaceFlowLimitParameterinto per-type containers (empty meta) and updateVariableMaxInterfaceFlowconstraints to read them without per-servicemeta. - Fold ORDC
ProductionCostExpressioninto one dense(service_name, time)container per reserve service type (empty meta), updating expression creation and reads accordingly; expand coverage with new interface-focused tests and broaden the “no available contributing devices/branches” error to include interfaces (while exemptingConstantReserveGroup).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_services_constructor.jl | Adds regression tests for merged interface slacks/params and for the new “no contributing branches” error behavior. |
| src/services_models/transmission_interface.jl | Updates VariableMaxInterfaceFlow constraint construction to read merged min/max flow-limit parameter containers (no per-service meta). |
| src/services_models/services_constructor.jl | Builds interface slack containers once per type (when use_slacks), merges interface time-series parameter population, and folds ORDC cost-expression container creation outside the per-service loop. |
| src/services_models/service_slacks.jl | Refactors interface slack construction to create one dense container per slack type across all interfaces, applying per-interface penalties. |
| src/core/problem_template.jl | Extends the “no available contributing devices” loud error to include interfaces (and keeps ConstantReserveGroup exempt). |
| src/common_models/add_to_expression.jl | Updates interface slack reads and ORDC cost-expression reads to use the merged (empty-meta) containers. |
| src/common_models/add_expressions.jl | Changes reserve-service cost-expression container creation from per-service meta=name to one merged dense (service_name, time) container per service type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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).
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.
…stantReserveGroup
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.
Per Phase A review: mark the transmission_interface_slacks! merged-container comment with DELETE-AFTER-REVIEW (reviewer context on the container change).
…ice, 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.
rodrigomha
force-pushed
the
rh/dev_refactor_services_pt2
branch
from
July 30, 2026 16:45
2423224 to
9b3b8e2
Compare
rodrigomha
marked this pull request as ready for review
July 30, 2026 18:36
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.
Summary
Stacked on top of PR #206 (base branch is
rh/dev_refactor_services, notmain) — review/merge #206 first. Continues the per-type merged-container migration for the two service families #206 left on the service-name-as-metapattern.Phase A — TransmissionInterface
InterfaceFlowSlackUp/Down→ one dense(interface_name, time)container per type, built once over all interfaces, empty meta (was per-interfacemeta = name). This also closes theuse_slacks-on-ServiceModel gap (moved to the vector-axis builder reserves already use), so that path is now testable.Min/MaxInterfaceFlowLimitParameter→ merged per-type container (empty meta); theVariableMaxInterfaceFlowLimitreads them 3-arg.InterfaceTotalFlow(already name-keyed) andInterfaceFlowLimit(meta = "ub"/"lb"= direction) unchanged.!(service_type <: PSY.ConstantReserveGroup).Phase B — ORDC cost expression
ProductionCostExpressionfor ORDC (StepwiseCostReserve) folded from per-servicemeta = nameinto one dense(service, time)container per service type, empty meta — same shape as the deviceProductionCostExpression. Read/write is now 3-arg.Deferred to Phase C (pt3 branch, later)
The ORDC block-offer variables/constraints are already 3D
(service, segment, time)merged. The ORDC slope/breakpoint PWL params stay onmetafor now — merging them needs a shared/padded tranche axis and is coupled with the delta-PWL machinery, so they land with the 4D per-device AS-offer work in Phase C.Reviewer notes
DELETE-AFTER-REVIEWare before/after container-change context for reviewers; remove on approval (grep -rn "DELETE-AFTER-REVIEW").rh/dev_service_refactornotes from Per-type service models with merged sparse/dense reserve containers #206 carry over unchanged; this branch adds no new dependency changes.Opening as draft, stacked into #206.
🤖 Generated with Claude Code