Sienna-Platform / Sienna-Platform/PowerOperationsModels.jl

Deduplicate the 22 `construct_service!` methods via a shared pipeline

Open
#223 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Julia
Stars
2
Forks
1
Avg merge
1d 16h
Merged PRs (30d)
26

Description

Sibling of #172 (construct_device!) and #195 (network-family constructor pairs), for the service side.

Raised by @luke-kiernan reviewing #206:

Maybe have AI do a quick pass on this file for low-hanging fruit for code deduplication.
Pre-existing but these construct_device! calls all look fairly similar...

Deferred out of #206 to keep an already-large refactor reviewable.

Current shape

src/services_models/services_constructor.jl is 943 lines holding 22 construct_service! methods: roughly one per (stage, service type, formulation), split ~500 lines reserve family and ~340 lines transmission interface.

Formulation Argument stage Model stage
RangeReserve 102 (Reserve), 137 (ConstantReserve) 171
StepwiseCostReserve 222 263
GroupReserve 396 413
RampReserve 445 478
NonSpinningReserve 523 549
ConstantMaxInterfaceFlow 600, 625 651, 695, 739
VariableMaxInterfaceFlow 855 794, 839, 901

The repetition is near-exact, not just thematic

RangeReserve (102-133) and RampReserve (445-476) argument stages are identical apart from the formulation passed to add_service_variables!. Both are:

services = _services_with_contributors(model, sys)
isempty(services) && return
add_parameters!(container, RequirementTimeSeriesParameter, services, model)
for service in services
    contributing_devices = get_contributing_devices(model, PSY.get_name(service))
    add_service_variables!(container, ActivePowerReserveVariable, service,
                           contributing_devices, <Formulation>)
    add_to_expression!(container, ActivePowerReserveVariable, service, model, devices_template)
    add_feedforward_arguments!(container, model, service)
end

NonSpinningReserve (523-547) is the same minus add_to_expression!.
ConstantReserve (137) is the same minus add_parameters!.

Model stages repeat just as closely.
RangeReserve (171-213) and RampReserve (478-521) share the container build, the use_slacks line, the per-service loop, add_to_objective_function!, add_feedforward_constraints! and add_constraint_dual!.
The entire difference is that RampReserve inserts one extra add_constraints!(container, RampConstraint, ...) call.

Proposed direction

A shared pipeline for each stage, with the per-formulation variation expressed as small dispatching predicates rather than as whole copied method bodies.

The file already contains the shape to generalize, at line 215:

_maybe_process_stepwise(container, model,
    services::Vector{<:PSY.ReserveDemandTimeSeriesCurve}) =
    process_stepwise_cost_reserve_parameters!(container, model, services)
_maybe_process_stepwise(container, model, services) = nothing

Extending that idiom gives roughly:

  • _maybe_add_requirement_parameters! - no-op for ConstantReserve, which has no requirement time series
  • _maybe_add_reserve_expression! - no-op for NonSpinningReserve
  • _maybe_add_ramp_constraints! - active only for RampReserve
  • _maybe_add_participation_constraints! - no-op for StepwiseCostReserve

Each pair is two lines and dispatches on the formulation or service type, so the specialization stays statically resolved and the generic body is written once per stage.

Open questions for whoever picks this up:

  • Whether the transmission-interface methods fold into the same pipeline or want their own. They share the _services_with_contributors / loop / dual skeleton but differ in nearly everything else, and there are already five ModelConstructStage methods there worth looking at on their own.
  • Whether GroupReserve participates. It is deferred to last in construct_services! and reads its inputs from other services rather than from contributing devices, so it may be genuinely separate.

Constraints

  • Keep the two-stage ArgumentConstructStage / ModelConstructStage split intact; it is load-bearing for expression-before-constraint ordering.
  • Run Test.detect_ambiguities afterwards. Collapsing methods onto shared abstract bounds is exactly how ambiguities appear.
  • No behavior change. test_services_constructor.jl should pass untouched.

Worth coordinating with #216, which will also be editing these same method bodies for type stability.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/services_models/services_constructor.jl, comparing the 22 construct_service! methods and the existing _maybe_process_stepwise dispatch near line 215. Run test_services_constructor.jl and Test.detect_ambiguities before and after the refactor, coordinating with #216 because it edits the same methods. Done means the two-stage split remains intact, tests pass unchanged, no ambiguities are introduced, and behavior is unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.