Port G-1 security-constrained reserves (PSI #1617) - #148
Open
acostarelli wants to merge 16 commits into
Open
Conversation
Add the reserve/service-side security-constrained contingency layer:
post-contingency reserve deployment under generator (G-1) outages with
monitored-branch post-contingency flow constraints, across the CopperPlate,
AreaBalance, PTDF and AreaPTDF network models.
- New SecurityConstrained{Contingency,Ramp}Reserve formulations; contingency
variable/expression/constraint types; post-contingency slack-cost constant;
natural-unit conversions and exports.
- Service-side outage population in template validation
(_build_service_model_outages! and helpers); admit PSY.AreaInterchange as a
monitored component type.
- New services_models/static_injection_security_constrained_models.jl
(sparse, monitored post-contingency containers) and its test file.
Two AreaBalance objective expected-values were re-recorded for psy6
(two_area_pjm_DA system-data drift; model structure matches PSI exactly per
moi_tests/constraint-key checks).
[sources] pins InfrastructureOptimizationModels to its matching ac/g1-port
branch (ServiceModel.outages + 1-D time-only store output methods).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR ports G-1 (security-constrained) reserve formulations into PowerOperationsModels, adding service-side post-contingency reserve-deployment modeling and an extensive regression test suite to match the behavior being ported from PowerSimulations (PSI #1617).
Changes:
- Add security-constrained reserve formulations (contingency + ramp) with post-contingency deployment variables and sparse monitored-component flow constraints.
- Add template validation to build per-service outage scoping (
service_model.outages) based on outages attached to services. - Add comprehensive tests for multiple network models/formulations and adjust test dependencies to track required upstream IOM changes.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_static_injection_security_constrained_models.jl | New regression suite covering SC reserve build/solve behavior and outage scoping. |
| test/Project.toml | Pins test dependency InfrastructureOptimizationModels to ac/g1-port. |
| src/services_models/static_injection_security_constrained_models.jl | New service-side SC reserve model implementation (deployment vars, post-contingency expressions/constraints). |
| src/services_models/reserves.jl | Adds default requirement time-series mapping for SC reserve formulations. |
| src/PowerOperationsModels.jl | Includes new service-model file and exports SC types/keys. |
| src/operation/template_validation.jl | Adds _build_service_model_outages! and broadens monitored-type admission (incl. AreaInterchange). |
| src/core/variables.jl | Adds contingency variable supertypes + SC reserve deployment variable types and unit-conversion flags. |
| src/core/formulations.jl | Introduces SC reserve formulation types. |
| src/core/expressions.jl | Adds post-contingency expression types and output/unit-conversion hooks. |
| src/core/definitions.jl | Adds post-contingency slack penalty constant. |
| src/core/constraints.jl | Adds post-contingency balance/limit constraint types used by SC reserves. |
| Project.toml | Pins InfrastructureOptimizationModels to ac/g1-port. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Performance Results
|
acostarelli
commented
Jun 26, 2026
Bugs: - has_requirement_ts now checks for the specific requirement time series via PSY.has_time_series(service, ts_type, "requirement") instead of testing only that some time series exists on the service. - Test helper get_reserve_total_power_by_step_dict now scopes the deployment filter by outage id, so a device responding to multiple outages no longer mixes deployments across them. - compare_outage_power_and_deployed_reserves now forwards its tolerance kwarg to test_reserves_deployment. Docs/comments: - Reworded the SecurityConstrainedContingencyReserve docstring (outages scoped to the service with system fallback; requirement time series optional) and the misleading outage-attachment comment in the test. Cleanups: - Removed the redundant _add_to_jump_expression! wrapper (inline JuMP.add_to_expression!), pass the formulation by type to the slack builder, build the irregular sparse containers via the IOM sparse_container_spec explicit-keys overload, and use mapreduce for the test accumulators. Refactors: - Collapsed the 6 construct_service! methods + 3 near-duplicate helpers into one generic construct_service! dispatching the network model, a shared _construct_service_model_sc! core, and three thin _add_post_contingency_network_terms! methods; the boolean flags became a requires_requirement_ts trait. - Extracted the duplicated use_slacks logic into _make_post_contingency_slacks and _add_post_contingency_flow_rate_constraint!. Depends on the matching IOM sparse_container_spec overload (ac/g1-port). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
acostarelli
commented
Jun 28, 2026
Port unported upstream PSI (sm/g-1_monitored_c) changes:
- Broaden UnplannedOutage -> abstract PSY.Outage at the post-contingency
balance/deployment dispatch and supplemental-attribute lookups so planned
outages flow through when opted in (PSI b74bd31/9b819fe/2577880/5d5b3c6).
- Correct SecurityConstrainedContingencyReserve docstring: outages scoped to
the PSY.Service, requirement time series optional (PSI ced97500).
Review fixes:
- Skip zero-coefficient terms for outaged generators instead of adding them.
- Cache PTDF columns by arc to avoid repeated KLU solves per outage/time.
- add_variables! dispatches on ::Type{F} instead of a formulation instance.
- Inline the two single-call construct_service! helpers.
- Narrow the SC DCP path to AbstractPTDFModel, dropping PowerModels coupling.
- Trim/remove unnecessary comments; document the requirement time-series name.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the hand-built SparseAxisArray + manual ExpressionKey/ConstraintKey/ VariableKey + _assign_container! boilerplate in both security-constrained models with add_expression_container! (seed-by-keys) and the new add_variable_container!/add_constraints_container! pre-built overloads, per IOM's "never create container keys directly" rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Post-contingency expression, constraint, and slack containers are now created and prefilled via IOM's sparse_keys kwarg instead of building a SparseAxisArray in POM and handing it across. The container's keys are the resolved (outage, name, t) tuples — sparse storage, no cartesian holes. Branch slack containers are created only when use_slacks (dropping the conditional post-loop registration). Removed the stale docstring claiming the expression pre-fill is needed for the parallel PTDF build: those tasks return results and the writes into the container happen serially on the main thread. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
||
| [sources] | ||
| InfrastructureOptimizationModels = {rev = "main", url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl"} | ||
| InfrastructureOptimizationModels = {rev = "ac/g1-port", url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl"} |
Switch the security-constrained branch and reserve call sites from the removed IOM sparse_keys kwarg to sparse=true with type-carrying axes (String[], String[], time_steps); the containers start empty and the build loops fill the ragged keys by assignment. The AreaInterchange flow expression initializes a fresh zero(AffExpr) before accumulating (it previously read a prefilled entry). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
acostarelli
commented
Jun 29, 2026
- Replace direct JuMP.add_to_expression! with add_proportional_to_jump_expression! and skip outaged generators instead of adding zero-coefficient terms - Inline the one-line sparse-container helpers (service and branch sides); combine the parallel use_slacks if-blocks into one - Dispatch the post-contingency flow-rate constraint on Nothing slacks rather than branching at run time - Add POST_CONTINGENCY_LB/UB_META constants for the lb/ub constraint meta tags - Hoist per-device bounds/warm-start out of the time-step loop in add_variables! - Drop "Struct to ..." phrasing on the new G-1 variable/constraint docstrings; trim the slack-variable docstring - Clarify the PTDF cache comment (VirtualPTDF row solved via KLU, not a column) - Add method-level docstrings explaining the G-1 corrective-but-preventive model across construct_service!, the add_to_expression! pairs, the flow expression, the network-terms dispatches, and the copperplate-balance constraint - Use idiomatic sum() in the test deployment-aggregation helpers; condense the template-validation orphan-check comment Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
acostarelli
changed the title
acostarelli
marked this pull request as ready for review
June 30, 2026 17:07
PSY's heterogeneous outage/generator supplemental-attribute maps force typeof(d)-based get_variable/get_variable_multiplier lookups in the G-1 security-constrained reserve code. Wrap each lookup's downstream per-timestep loop in a function barrier so the unstable dispatch is isolated to once per device/outage instead of leaking into the inner loop; hoist one redundant per-(entries,t) lookup to once per outage. Also route two constraint sites through IOM.add_range_bound_constraint!/ add_range_equality_constraint! where they fit the existing 2-arg (name, t) signature, matching established POM convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
Two add_constraints! methods placed contributing_devices before service, reversed from the (service, contributing_devices) order every other service constraint uses (reserves.jl and the other outage add_constraints! methods). No dispatch impact — just matches the established convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…atch The system-wide, PTDF-nodal, and AreaBalance-area post-contingency balance/deployment expressions differed only in which extra index component sits between outage_id and t; the surrounding accumulation logic was duplicated three times per (healthy-deployment, outaged-generator) pair. Mirrors the _system_expression_type/ _balance_expression_targets pattern already used in add_to_expression.jl: _post_contingency_target_axes/_post_contingency_target_index dispatch on the expression type T (not on network_model's runtime type, since PostContingencyActivePowerBalance is built unconditionally under every network model) to resolve the axis/index, and one generic method per pair replaces the six network-model-specific copies. Also threads attribute_device_map through to the AreaBalance pre-contingency call site instead of recomputing it via a second PSY.get_component_supplemental_attribute_pairs call, which the unified signature now requires consistently across all three network models. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
luke-kiernan
requested changes
Jul 1, 2026
Collaborator
There was a problem hiding this comment.
Big-picture structural issues:
- Type instability in code downstream of
typeof. Things like the following are bad:
# types cannot determined...
variable = get_variable(container, U, typeof(d))
mult = get_variable_multiplier(U, typeof(d), F)
# ...which forces runtime function resolution in what follows
for t in time_steps
add_proportional_to_jump_expression!(
expression[outage_id, t],
# ambiguous: compiler cannot predict correct get_index
variable[name, t],
mult,
)
endIdeally you'd avoid the typeof entirely, but doesn't look possible here without major changes. Adding function barrier helps. [I don't fully understand where to put the function barrier and why.] I had Claude code address this: see #166 [stacked on top of #148]
2. Code repetition: I'm trying to set a new precedent for handling copperplate vs nodal vs area. Instead of writing 3 functions, write one and then multiple-dispatch over what to index on. See #167 [stacked on top of #166]
There's a couple places where I'm seeking clarification.
edit: oh also tests. I only went thru src thoroughly. I'll leave it up to you to check test reasonableness and coverage.
| slack_lb = SparseAxisArray(Dict{Tuple{String, String, Int}, JuMP.VariableRef}()) | ||
| # Relaxation-slack containers keyed by `(outage_id, name, t)`, registered only | ||
| # when slacks are enabled. Built here (not via `add_variables!`) because the | ||
| # post-contingency keys are ragged; the axes only fix the key tuple type and |
Collaborator
There was a problem hiding this comment.
I had to look up what "ragged" means. Not sure if that's common parlance
| service via `add_supplemental_attribute!(sys, service, outage)`; template | ||
| validation mirrors those attachments into `service_model.outages`. | ||
| A `RequirementTimeSeriesParameter` is optional: when present the requirement / | ||
| ramp / participation stack is built; when absent, per-generator |
Collaborator
There was a problem hiding this comment.
the requirement / ramp / participation stack is built
huh?
| end | ||
|
|
||
| # Monitored-component names grouped by their concrete (modeled) type. Returns | ||
| # `(per_type, uncovered)` where `uncovered` is the set of monitored component |
Collaborator
There was a problem hiding this comment.
Worth adding something about what's per_type here imo: looks like it's the set of monitored components of each datatype?
…ncy-target-dispatch Consolidate post-contingency balance expressions via target-index dispatch
Contain type instability in G-1 outage-driven typeof dispatches
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.
Sienna-Platform/PowerSimulations.jl#1617