Per-device bounds on approximations by acostarelli · Pull Request #86 · Sienna-Platform/InfrastructureOptimizationModels.jl

acostarelli · 2026-04-21T21:07:31Z

Thanks for opening a PR to InfrastructureOptimizationModels.jl, please take note of the following when making a PR:

Check the contributor guidelines

github-actions · 2026-04-21T21:22:16Z

Performance Results
Main

This branch

acostarelli · 2026-04-21T22:27:06Z

Closes #76

Copilot

Pull request overview

This PR extends InfrastructureOptimizationModels.jl to support per-device bounds for quadratic/bilinear approximation formulations and adds explicit forecast interval handling (including auto-transforming static time series into forecasts when building a DecisionModel).

Changes:

Refactors quadratic/bilinear approximation builders to accept Vector{MinMax} bounds (per component name) instead of global scalar min/max bounds.
Adds Settings.interval, interval-aware time-series cache keys, and interval-aware time-series lookup/plumbing (incl. network reductions and model store params).
Introduces automatic static→deterministic time-series transformation when a model is built with a specified (horizon, interval) and only static series exist.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/utils/powersystems_utils.jl	Adds interval/resolution conversion helpers and auto-transform + interval inspection utilities.
src/utils/jump_utils.jl	Adds `get_hinted_aff_expr` and a new long-format DataFrame conversion overload for 3D arrays with integer axes.
src/quadratic_approximations/solver_sos2.jl	Switches SOS2 quadratic approximation to per-name bounds and pre-sized variable containers.
src/quadratic_approximations/manual_sos2.jl	Switches manual SOS2 quadratic approximation to per-name bounds and pre-sized containers.
src/quadratic_approximations/sawtooth.jl	Switches sawtooth quadratic approximation to per-name bounds.
src/quadratic_approximations/epigraph.jl	Switches epigraph relaxation to per-name bounds.
src/quadratic_approximations/pwmcc_cuts.jl	Switches PWMCC cuts to per-name bounds and pre-sized containers.
src/quadratic_approximations/common.jl	Updates normalized-variable helper to use per-name bounds.
src/quadratic_approximations/no_approx.jl	Updates no-op quadratic approximation signature to accept per-name bounds.
src/quadratic_approximations/nmdt_common.jl	Updates NMDT shared utilities to carry per-name bounds through normalization/product assembly.
src/quadratic_approximations/nmdt.jl	Updates NMDT/DNMDT quadratic approximation interfaces to accept per-name bounds.
src/bilinear_approximations/bin2.jl	Updates Bin2 bilinear approximation to use per-name bounds (including auxiliary variable bounds).
src/bilinear_approximations/hybs.jl	Updates HybS bilinear approximation to use per-name bounds (including z bounds).
src/bilinear_approximations/mccormick.jl	Adds per-name bounds overload for McCormick envelope and updates reformulated cuts interface.
src/bilinear_approximations/nmdt.jl	Updates NMDT/DNMDT bilinear approximation interfaces to accept per-name bounds.
src/bilinear_approximations/no_approx.jl	Updates no-op bilinear approximation signature to accept per-name bounds.
src/core/settings.jl	Adds `interval` to `Settings` plus `get_interval`/`set_interval!`.
src/core/definitions.jl	Adds `UNSET_INTERVAL` and `PTDF_ZERO_TOL`.
src/operation/decision_model.jl	Adds interval keyword to constructor, auto-transform hook, and interval validation/selection logic.
src/operation/time_series_interface.jl	Adds interval/resolution-aware cache keys and forwarding of interval/resolution to time-series APIs.
src/core/optimization_container.jl	Extends initial time-series lookup to accept interval/resolution keywords.
src/common_models/get_time_series.jl	Threads interval through helper time-series getters.
src/core/network_reductions.jl	Adds interval-aware time-series UUID selection for reduced branch parameter axes.
src/operation/decision_model_store.jl	Adds `write_output!` overload for 3D arrays with integer axes.
src/common_models/add_constraint_dual.jl	Aligns dual containers’ row axes with existing constraint containers (handles reduced networks).
src/operation/model_numerical_analysis_utils.jl	Splits constraint coefficient vs RHS bound updates (new default fallbacks).
src/InfrastructureOptimizationModels.jl	Exports `get_interval`.

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

Copilot · 2026-04-21T23:21:50Z


-    for name in names, t in time_steps
+    for (i, name) in enumerate(names), t in time_steps
+        b = bounds[i]


lx is computed from per-device bounds and is later used as a divisor in the linking constraint. If any entry in bounds has min == max (or max < min), this will introduce division-by-zero/invalid scaling and break the model. Add an explicit per-name check (e.g. assert b.max > b.min) before computing/using lx, similar to the guards in epigraph.jl/sawtooth.jl.

Suggested change

b = bounds[i]

b = bounds[i]

@assert b.max > b.min "Invalid bounds for $(name): expected max > min, got min=$(b.min), max=$(b.max)"

Copilot · 2026-04-21T23:21:50Z

+    model_interval = get_interval(settings)
+    available_intervals = get_forecast_intervals(sys)
+    if model_interval == UNSET_INTERVAL && length(available_intervals) > 1
+        throw(
+            IS.ConflictingInputsError(
+                "The system contains multiple forecast intervals $(available_intervals). " *
+                "The `interval` keyword argument must be provided to the DecisionModel constructor " *
+                "to select which interval to use.",
+            ),
+        )
+    elseif model_interval != UNSET_INTERVAL && !isempty(available_intervals)
+        if model_interval ∉ available_intervals
+            throw(
+                IS.ConflictingInputsError(
+                    "Interval $(Dates.canonicalize(model_interval)) is not available in the system data. " *
+                    "Available forecast intervals: $(available_intervals)",
+                ),
+            )
+        end
+    end
+    interval_kwarg =
+        model_interval == UNSET_INTERVAL ? (;) : (; interval = model_interval)


The new interval-selection logic (multiple forecast intervals error, validating a user-specified interval, and passing interval into get_forecast_horizon) introduces new behavior that isn’t currently covered by the test suite. Please add tests that (1) build a system with multiple forecast intervals and assert the constructor errors unless interval is provided, and (2) verify that a provided interval is used (and rejected when unavailable).

Copilot · 2026-04-21T23:21:50Z

+function to_outputs_dataframe(
+    array::DenseAxisArray{
+        Float64,
+        3,
+        <:Tuple{Vector{String}, UnitRange{Int}, UnitRange{Int}},
+    },
+    ::Nothing,
+    ::Val{TableFormat.LONG},
+)
+    num_rows = length(array.data)
+    time_col = Vector{Int}(undef, num_rows)
+    name_col = Vector{String}(undef, num_rows)
+    name2_col = Vector{Int}(undef, num_rows)
+    vals = Vector{Float64}(undef, num_rows)
+
+    row_index = 1
+    for name in axes(array, 1)
+        for name2 in axes(array, 2)
+            for time_index in axes(array, 3)
+                time_col[row_index] = time_index
+                name_col[row_index] = name
+                name2_col[row_index] = name2
+                vals[row_index] = array[name, name2, time_index]
+                row_index += 1
+            end
+        end
+    end
+
+    return DataFrame(
+        :time_index => time_col,
+        :name => name_col,
+        :name2 => name2_col,
+        :value => vals,
+    )
+end


This adds a new to_outputs_dataframe overload for 3D DenseAxisArray with axes (Vector{String}, UnitRange{Int}, UnitRange{Int}) when timestamps are nothing, but test/test_jump_utils.jl only covers the (Vector{String}, Vector{String}, UnitRange) 3D case. Add a unit test for this new axis shape to ensure the long-format output has the expected row count and column values/order.

jd-lara · 2026-04-22T06:58:11Z

@acostarelli Address the comments from copilot and test again before merging. We can discuss when I come back from PTO

… for unsupported constraint in numerical analysis; added output handling methods for different variable arrays

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

luke-kiernan

Looks like a pretty straightforward refactor. If tests are passing I'd be ok with merging

luke-kiernan · 2026-04-23T00:16:16Z

    return
 end

+function write_output!(


Seems mildly unrelated to the topic of the PR. Needed to fix some test?

I wrote this in conjunction with Sienna-Platform/PowerOperationsModels.jl#87 , and I was getting an error trying to read the output in testing that formulation.

luke-kiernan · 2026-04-23T00:19:20Z


 """
-    _assemble_product!(container, C, names, time_steps, terms, dz_var, x_disc, y_disc, meta; result_type)
+    _assemble_product!(container, C, names, time_steps, terms, dz_var, xh_norm, yh_norm, x_bounds, y_bounds, meta; result_type)


Maybe it's obvious, but I'm not seeing wheter these extra arguments came from, what changed to make them needed.

The docstring was outdated from the method signature: the NMDTDiscretization structs used to store the min and max as well.

acostarelli requested a review from jd-lara April 21, 2026 21:23

jd-lara requested a review from Copilot April 21, 2026 23:13

Copilot started reviewing on behalf of jd-lara April 21, 2026 23:13 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Anthony Costarelli and others added 5 commits April 22, 2026 11:19

saving copilot's code here

b7a71b6

converted axis-less containers in some approximations; fixed fallback…

504d8f2

… for unsupported constraint in numerical analysis; added output handling methods for different variable arrays

Apply suggestions from code review

3b9f1b0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

fix type dispatch

ce70c8b

more type dispatch fix

d188616

acostarelli force-pushed the ac/per-device-bounds branch from 8d8764b to d188616 Compare April 22, 2026 15:22

Anthony Costarelli added 4 commits April 22, 2026 11:54

copilot review

d08afe5

fix assert_op class; remove zz tests (again?)

442b7da

tests use correct bounds; pruned pointless tests

ae90871

format

4a75aa4

luke-kiernan approved these changes Apr 23, 2026

View reviewed changes

Anthony Costarelli added 2 commits April 23, 2026 02:46

restore aqua tests

c87697b

resolved ambiguity

cd4f824

	b = bounds[i]
	b = bounds[i]
	@assert b.max > b.min "Invalid bounds for $(name): expected max > min, got min=$(b.min), max=$(b.max)"

Conversation

acostarelli commented Apr 21, 2026

Uh oh!

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

Uh oh!

Uh oh!

acostarelli commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

jd-lara commented Apr 22, 2026

Uh oh!

luke-kiernan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luke-kiernan Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

luke-kiernan Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Apr 21, 2026 •

edited

Loading

luke-kiernan left a comment •

edited

Loading