Skip to content

split add_param_container! into separate functions - #148

Merged
jd-lara merged 2 commits into
mainfrom
lk/issue-147
Aug 15, 2026
Merged

split add_param_container! into separate functions#148
jd-lara merged 2 commits into
mainfrom
lk/issue-147

Conversation

@luke-kiernan

Copy link
Copy Markdown
Collaborator

See issue #147. I added shims just so CI will keep passing in POM...but really we should probably just migrate over POM to the new names as soon as this is merged

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #147 by splitting the overloaded add_param_container! API into four explicitly named parameter-container builder functions, while keeping a compatibility layer so downstream packages can migrate incrementally.

Changes:

  • Introduces and exports four dedicated builder functions: add_time_series_parameter_container!, add_cost_function_parameter_container!, add_variable_value_parameter_container!, and add_event_parameter_container!.
  • Adds add_param_container! compatibility shims forwarding to the new builders, and includes them after add_param_container.jl.
  • Exports add_parameterized_lower_bound_range_constraints alongside the existing upper-bound helper.

Reviewed changes

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

File Description
src/InfrastructureOptimizationModels.jl Updates exports and include order to expose the new builder APIs and include the compatibility shims.
src/common_models/add_param_container.jl Replaces the multi-method add_param_container! wrappers with four clearly named builder functions plus updated docstrings.
src/common_models/add_param_container_shims.jl Adds forwarder methods to preserve the legacy add_param_container! call sites during migration.

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

Comment thread src/InfrastructureOptimizationModels.jl Outdated
Comment thread src/common_models/add_param_container.jl
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Performance Results
Main

Network: 10 nodes, 13 edges, 3 cost segments
Generators: 5, Demands: 5
Loss coefficients (a, b, c) per generator:
  n1: a=0.007287  b=0.005968  c=0.003850
  n2: a=0.001094  b=0.009002  c=0.007028
  n3: a=0.007897  b=0.008552  c=0.003001
  n4: a=0.008179  b=0.002530  c=0.009637
  n5: a=0.009778  b=0.009934  c=0.009099

Solver logs: /home/runner/work/InfrastructureOptimizationModels.jl/InfrastructureOptimizationModels.jl/main/test/performance/logs/solver_2026-08-05T23-56-23.log

==============================================================================================================================================
Bilinear Approximation Benchmarks
  Refinement = depth for all methods
==============================================================================================================================================
Method          R   Vars Constrs   Bins    Objective   Gap(%) MIPGap(%)     LowerBnd  rmse δbi   max δbi   rmse δq    max δq  build_t  solve_t
----------------------------------------------------------------------------------------------------------------------------------------------
NLP (Ipopt)     -     40     105      0     0.956760        -         -            -  0.00e+00  0.00e+00  0.00e+00  0.00e+00   0.0320   0.0044

NLP (Uno)       -     40     105      0     0.956760   0.0000         -            -  0.00e+00  0.00e+00  0.00e+00  0.00e+00   0.0013   0.0011

Bin2+sSOS       4    190     535      0     1.180700  23.4061    0.0000     1.180700  8.07e-02  1.41e-01  2.49e+01  5.48e+01   0.0053   1.7686
Bin2+sSOS       6    250     655      0     1.098175  14.7806    0.0000     1.098175  1.26e-01  2.86e-01  5.47e+00  9.07e+00   0.0056   7.3938
Bin2+sSOS       8    310     775      0     1.053069  10.0661    0.0088     1.052976  8.62e-02  2.43e-01  4.18e+01  9.32e+01   0.0057  26.1674

Bin2+mSOS       4    310     805    120     1.180700  23.4061    0.0000     1.180700  8.07e-02  1.41e-01  2.49e+01  5.48e+01   0.0022   1.7307
Bin2+mSOS       6    430    1045    180     1.098175  14.7806    0.0092     1.098074  1.26e-01  2.86e-01  5.47e+00  9.07e+00   0.0023   7.4173
Bin2+mSOS       8    550    1285    240     1.053069  10.0661    0.0045     1.053021  8.62e-02  2.43e-01  4.18e+01  9.32e+01   0.0026  17.7320

Bin2+Saw        4    310    1075    120     0.985823   3.0376    0.0000     0.985823  1.08e-01  2.49e-01  1.11e+02  2.44e+02   0.0030   5.7743
Bin2+Saw        6    430    1495    180     0.958677   0.2004    0.0077     0.958603  7.52e-02  1.68e-01  8.94e+01  1.71e+02   0.0035  13.2010
Bin2+Saw        8    550    1915    240     0.956996   0.0247    0.0072     0.956927  7.52e-02  1.68e-01  1.37e+02  2.09e+02   0.0043  49.6505

HybS+sSOS       4    310    1165      0     0.812261  15.1030    0.0000     0.812261  5.52e-01  1.00e+00  2.04e+02  2.97e+02   0.0066   2.6480
HybS+sSOS       6    410    1545      0     0.891208   6.8515    0.0000     0.891208  5.49e-01  1.00e+00  3.69e+02  7.09e+02   0.0071   4.9269
HybS+sSOS       8    510    1925      0     0.934789   2.2965    0.0087     0.934707  5.48e-01  1.00e+00  3.47e+02  5.22e+02   0.0076  15.4481

HybS+mSOS       4    390    1345     80     0.812261  15.1030    0.0046     0.812224  5.52e-01  1.00e+00  2.04e+02  2.97e+02   0.0032   2.8709
HybS+mSOS       6    530    1805    120     0.891208   6.8515    0.0000     0.891208  5.49e-01  1.00e+00  3.69e+02  7.09e+02   0.0037   6.1971
HybS+mSOS       8    670    2265    160     0.934789   2.2965    0.0047     0.934744  5.48e-01  1.00e+00  3.47e+02  5.22e+02   0.0042  16.0245

HybS+Saw        4    390    1525     80     0.951869   0.5113    0.0021     0.951849  5.48e-01  1.00e+00  2.16e+03  4.23e+03   0.0040  13.2719
HybS+Saw        6    530    2105    120     0.956386   0.0391    0.0099     0.956291  5.48e-01  1.00e+00  4.57e+02  7.57e+02   0.0045  32.8522
HybS+Saw        8    670    2685    160     0.956734   0.0027    0.0014     0.956721  5.48e-01  1.00e+00  6.08e+00  1.07e+01   0.0053 105.0045

DNMDT           4    395    1640     80     0.954878   0.1967    0.0004     0.954874  1.39e-03  3.13e-03  4.70e-04  1.05e-03   0.0030   3.7913
DNMDT           6    550    2315    120     0.956636   0.0130    0.0098     0.956541  6.75e-05  1.57e-04  4.26e+03  9.54e+03   0.0034  16.8589
DNMDT           8    705    2990    160     0.956754   0.0007    0.0098     0.956660  4.62e-06  1.14e-05  9.42e+01  1.49e+02   0.0043  47.9399

==============================================================================================================================================

This branch

Network: 10 nodes, 13 edges, 3 cost segments
Generators: 5, Demands: 5
Loss coefficients (a, b, c) per generator:
  n1: a=0.007287  b=0.005968  c=0.003850
  n2: a=0.001094  b=0.009002  c=0.007028
  n3: a=0.007897  b=0.008552  c=0.003001
  n4: a=0.008179  b=0.002530  c=0.009637
  n5: a=0.009778  b=0.009934  c=0.009099

Solver logs: /home/runner/work/InfrastructureOptimizationModels.jl/InfrastructureOptimizationModels.jl/branch/test/performance/logs/solver_2026-08-06T00-04-20.log

==============================================================================================================================================
Bilinear Approximation Benchmarks
  Refinement = depth for all methods
==============================================================================================================================================
Method          R   Vars Constrs   Bins    Objective   Gap(%) MIPGap(%)     LowerBnd  rmse δbi   max δbi   rmse δq    max δq  build_t  solve_t
----------------------------------------------------------------------------------------------------------------------------------------------
NLP (Ipopt)     -     40     105      0     0.956760        -         -            -  0.00e+00  0.00e+00  0.00e+00  0.00e+00   0.0335   0.0046

NLP (Uno)       -     40     105      0     0.956760   0.0000         -            -  0.00e+00  0.00e+00  0.00e+00  0.00e+00   0.0013   0.0012

Bin2+sSOS       4    190     535      0     1.180700  23.4061    0.0000     1.180700  8.07e-02  1.41e-01  2.49e+01  5.48e+01   0.0050   1.7711
Bin2+sSOS       6    250     655      0     1.098175  14.7806    0.0000     1.098175  1.26e-01  2.86e-01  5.47e+00  9.07e+00   0.0054   7.3908
Bin2+sSOS       8    310     775      0     1.053069  10.0661    0.0088     1.052976  8.62e-02  2.43e-01  4.18e+01  9.32e+01   0.0055  26.0458

Bin2+mSOS       4    310     805    120     1.180700  23.4061    0.0000     1.180700  8.07e-02  1.41e-01  2.49e+01  5.48e+01   0.0023   1.8504
Bin2+mSOS       6    430    1045    180     1.098175  14.7806    0.0092     1.098074  1.26e-01  2.86e-01  5.47e+00  9.07e+00   0.0023   7.4179
Bin2+mSOS       8    550    1285    240     1.053069  10.0661    0.0045     1.053021  8.62e-02  2.43e-01  4.18e+01  9.32e+01   0.0025  17.8994

Bin2+Saw        4    310    1075    120     0.985823   3.0376    0.0000     0.985823  1.08e-01  2.49e-01  1.11e+02  2.44e+02   0.0030   5.7928
Bin2+Saw        6    430    1495    180     0.958677   0.2004    0.0077     0.958603  7.52e-02  1.68e-01  8.94e+01  1.71e+02   0.0036  13.2673
Bin2+Saw        8    550    1915    240     0.956996   0.0247    0.0072     0.956927  7.52e-02  1.68e-01  1.37e+02  2.09e+02   0.0042  49.5493

HybS+sSOS       4    310    1165      0     0.812261  15.1030    0.0000     0.812261  5.52e-01  1.00e+00  2.04e+02  2.97e+02   0.0066   2.6424
HybS+sSOS       6    410    1545      0     0.891208   6.8515    0.0000     0.891208  5.49e-01  1.00e+00  3.69e+02  7.09e+02   0.0069   4.9024
HybS+sSOS       8    510    1925      0     0.934789   2.2965    0.0087     0.934707  5.48e-01  1.00e+00  3.47e+02  5.22e+02   0.0075  15.5422

HybS+mSOS       4    390    1345     80     0.812261  15.1030    0.0046     0.812224  5.52e-01  1.00e+00  2.04e+02  2.97e+02   0.0034   2.8906
HybS+mSOS       6    530    1805    120     0.891208   6.8515    0.0000     0.891208  5.49e-01  1.00e+00  3.69e+02  7.09e+02   0.0039   6.2166
HybS+mSOS       8    670    2265    160     0.934789   2.2965    0.0047     0.934744  5.48e-01  1.00e+00  3.47e+02  5.22e+02   0.0045  16.1632

HybS+Saw        4    390    1525     80     0.951869   0.5113    0.0021     0.951849  5.48e-01  1.00e+00  2.16e+03  4.23e+03   0.0038  13.3422
HybS+Saw        6    530    2105    120     0.956386   0.0391    0.0099     0.956291  5.48e-01  1.00e+00  4.57e+02  7.57e+02   0.0045  32.9408
HybS+Saw        8    670    2685    160     0.956734   0.0027    0.0014     0.956721  5.48e-01  1.00e+00  6.08e+00  1.07e+01   0.0054 105.9632

DNMDT           4    395    1640     80     0.954878   0.1967    0.0004     0.954874  1.39e-03  3.13e-03  4.70e-04  1.05e-03   0.0033   3.8433
DNMDT           6    550    2315    120     0.956636   0.0130    0.0098     0.956541  6.75e-05  1.57e-04  4.26e+03  9.54e+03   0.0035  17.0254
DNMDT           8    705    2990    160     0.956754   0.0007    0.0098     0.956660  4.62e-06  1.14e-05  9.42e+01  1.49e+02   0.0038  48.0448

==============================================================================================================================================

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@acostarelli acostarelli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this.

@luke-kiernan
luke-kiernan requested a review from jd-lara August 7, 2026 21:52

@jd-lara jd-lara left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great improvement probably we can specialize even more

@jd-lara
jd-lara merged commit 49146f3 into main Aug 15, 2026
7 checks passed
@jd-lara
jd-lara deleted the lk/issue-147 branch August 15, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants