Contain type instability in G-1 outage-driven typeof dispatches by luke-kiernan · Pull Request #166 · Sienna-Platform/PowerOperationsModels.jl

luke-kiernan · 2026-07-01T14:27:18Z

Summary

Stacked on #148. Reviewing that PR surfaced type-instability from typeof(d)-driven variable/multiplier lookups scattered through the outage/generator code — PSY's get_component_supplemental_attribute_pairs/get_contributing_devices return heterogeneous, abstractly-typed collections, so these dispatches are unavoidable without a much larger refactor (discussed inline, not pursued here as low-leverage for the cost).

Wrapped each typeof(d)-driven lookup's downstream per-t loop in a function barrier, so the dynamic dispatch is isolated to once per device/outage instead of leaking into the inner loop (verified with @code_warntype: the loop body compiles fully specialized post-barrier).
Hoisted one lookup (_outaged_generator_terms) that was redundantly re-executed per (entries, t) pair despite not depending on either, down to once per outage.
Routed two constraint sites through IOM.add_range_bound_constraint!/add_range_equality_constraint! where they cleanly fit the existing (name, t) signature, matching established convention (storage_models.jl, thermal_generation.jl). Deliberately did not extend IOM's API (e.g. a Tuple-index overload) to force-fit the 3-dim outage containers into the same helpers — that would've bent a shared abstraction around a single caller for marginal benefit; those sites stay as direct JuMP.@constraint calls.

Test plan

julia --project=test test/runtests.jl test_static_injection_security_constrained_models — 1219/1219 pass
julia --project=test test/runtests.jl test_services_constructor test_device_hvdc test_device_branch_constructors — 162/162 pass
Formatter run (scripts/formatter/formatter_code.jl)

🤖 Generated with Claude Code

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>

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>

Copilot

Pull request overview

This PR reduces type-instability overhead in the G-1 security-constrained reserves / outage-driven code paths by introducing function barriers around typeof(device)-based lookups, hoisting a redundant per-(entries, t) generator-term lookup to once-per-outage, and aligning some constraint construction with existing IOM.add_range_*_constraint! helper conventions.

Changes:

Added function barriers around runtime-type (typeof(d)) variable/multiplier lookups so inner per-time-step loops can compile specialized.
Hoisted _outaged_generator_terms to be computed once per outage (instead of repeatedly inside nested loops) and reused in flow expression construction.
Replaced a few direct JuMP.@constraint assignments with IOM.add_range_bound_constraint! / IOM.add_range_equality_constraint! where the (name, t) shape matches existing helper patterns.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/services_models/static_injection_security_constrained_models.jl`	Introduces type aliases for resolved monitored arcs, adds multiple function barriers around dynamic-typed lookups, hoists per-outage generator term resolution, and routes one constraint site through an IOM range helper.
`src/services_models/reserves.jl`	Routes several bounds constraints through `IOM.add_range_bound_constraint!` and adds a function barrier to isolate dynamic `get_variable(..., typeof(d))` dispatch from the inner time-step loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-07-01T17:11:12Z

Performance Results

Version	Precompile Time
Main	3.120292201
This Branch	3.008624296

Version	Build Time
Main-Build Time Precompile	87.973089299
Main-Build Time Postcompile	1.42635348
This Branch-Build Time Precompile	85.482709245
This Branch-Build Time Postcompile	1.396836768

Version	Solve Time
Main-Solve Time Precompile	3375.424269664
Main-Solve Time Postcompile	3336.714472781
This Branch-Solve Time Precompile	2142.661427447
This Branch-Solve Time Postcompile	2102.147375476

acostarelli

Main takeaways:

Some redundancies in helpers and comments
If this type issue is everywhere in services, maybe we should think of a more robust solution

acostarelli · 2026-07-01T17:39:35Z

    service_model::ServiceModel,
    net_reduction_data::PNM.NetworkReductionData,
-)
+)::ResolvedMonitoredArcs


Is this here just for clarity, or does it help performance? I could be in favor of either reason, but if only the former, it does stick out to me as unusual.

Just for clarity and readability. No performance benefit.

acostarelli · 2026-07-01T17:41:16Z

    sys::PSY.System,
    ::Type{T},
    ::Type{U},
    attribute_device_map::Vector{


Do we also want to create an alias for this like you did for MonitoredArcs?

I didn't but we could, I'm not against it

luke-kiernan · 2026-07-01T20:34:27Z

If this type issue is everywhere in services, maybe we should think of a more robust solution

Yeah the more robust solution would be to group all of our iterables by generator type.

I just realized that if I push things here, I'll have to rebase the stacked PR...I guess I'll go address the comments on that one first

…ncy-target-dispatch Consolidate post-contingency balance expressions via target-index dispatch

luke-kiernan and others added 3 commits July 1, 2026 08:26

This was referenced Jul 1, 2026

Consolidate post-contingency balance expressions via target-index dispatch #167

Merged

Port G-1 security-constrained reserves (PSI #1617) #148

Open

luke-kiernan requested review from acostarelli and Copilot July 1, 2026 15:32

Copilot started reviewing on behalf of luke-kiernan July 1, 2026 15:33 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

acostarelli requested changes Jul 1, 2026

View reviewed changes

acostarelli mentioned this pull request Jul 1, 2026

Make sure all variables have bounds. #137

Open

luke-kiernan and others added 4 commits July 1, 2026 14:56

fix formatting strangeness

ce6f902

Merge pull request #167 from Sienna-Platform/ac/g1-port-post-continge…

96266ef

…ncy-target-dispatch Consolidate post-contingency balance expressions via target-index dispatch

anthony review comments

29d8e55

trim AI comment

32cff2f

luke-kiernan force-pushed the ac/g1-port-typestability branch from d851e99 to 32cff2f Compare July 7, 2026 19:35

luke-kiernan merged commit 8723ad0 into ac/g1-port Jul 7, 2026
2 of 6 checks passed

luke-kiernan deleted the ac/g1-port-typestability branch July 7, 2026 19:37

Uh oh!

Conversation

luke-kiernan commented Jul 1, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acostarelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acostarelli Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

luke-kiernan Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

luke-kiernan Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

luke-kiernan commented Jul 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 1, 2026 •

edited

Loading