Sienna-Platform / Sienna-Platform/PowerOperationsModels.jl

Build-loop type-stability: branch-divergent PTDF balance values and AbstractArray staging dict

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

Nobody has claimed this yet.

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

Description

Summary

Two build-loop type-stability findings from a time-series-chain audit. Both compound the container type-erasure documented in Sienna-Platform/InfrastructureOptimizationModels.jl#155 — fixing them without that one helps little, so they're documented here for when that issue is addressed.

Findings

  1. Branch-divergent per-timestep values in the PTDF balance expression builders. src/common_models/add_to_expression.jl:1533-1578 (branch at 1565-1572) and _add_load_ts_parameter_to_balance! (:224-256, branch at 246-255): param/mult are either a JuMP scalar from get_parameter_column_refs(...)[t] or a literal Float64, chosen by a runtime has_ts flag resolved per device but not per type. In isolation a fine 2-way union; through the erased containers of IOM#155 it collapses to Any, so add_proportional_to_jump_expression! (concrete dispatch arms in add_jump_expressions.jl:24-43) redispatches dynamically 2 × devices × timesteps. Fix direction: resolve has_ts into a Val{true}/Val{false} split (or two typed helpers called once per device) so the timestep body is monomorphic.

  2. Dict{String, AbstractArray} staging in the time-series parameter build. src/common_models/add_parameters.jl:179 (initial_values = Dict{String, AbstractArray}()) erases the concrete Vector{Float64} the accessors return; the values re-emerge as ::AbstractArray, the unwrap_for_param.(...) broadcast widens to Any eltype, and IOM._set_parameter_at! (5 concrete dispatch arms) redispatches per step in the loop at :257-259. Mirror pattern in _add_objective_function_parameters! (~:727-782). Bounded today — runs once per build!, and no update_container_parameter_values! implementation exists yet on this branch to re-execute it per step — but it becomes hot the moment that update path is ported. Fix: type the dict to the concrete family (Dict{String, Vector{Float64}}), a signature-only change.

Found during a type-stability audit (Julia 1.12.5, faithful minimal reproductions; _set_parameter_at! and the multiplier dispatch tables themselves verified clean — the callers hand them Any).

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 with the cited branches in src/common_models/add_to_expression.jl and the concrete dispatch arms in add_jump_expressions.jl, then inspect src/common_models/add_parameters.jl:179 and the mirrored objective-parameter code around lines 727-782. Use the stated Julia 1.12.5 minimal reproductions to verify the callers no longer hand Any values to the dispatch tables; done means the timestep paths remain type-stable without changing their behavior, alongside the container-type work tracked in IOM#155.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.