Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incremental PW linear to MIP transformation #3287

Merged
merged 73 commits into from
Aug 20, 2024
Merged

Conversation

sadavis1
Copy link
Contributor

@sadavis1 sadavis1 commented Jun 10, 2024

Summary/Motivation:

Add another transformation, the incremental MIP formulation, to our slowly growing library of things to do with piecewise-linear functions. This one requires a specially ordered triangulation, so some infrastructure around triangulations is also needed.

Changes proposed in this PR:

  • Add contrib.piecewise.incremental transformation
  • Add Triangulation enum to tag piecewise-linear functions with their underlying triangulation
  • Add J1 and OrderedJ1 triangulations.

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

sadavis1 and others added 30 commits November 9, 2023 01:02
…ify variables mode does not work, gives infeasible models
… transformation.

However, gurobi is not magical enough to see the structure here so this basically amounts to asking for a Hamiltonian path in a big graph, and then some. I think I will have to do the 70s stuff after all. Some optimizations would reduce this to "only" asking for a Hamiltonian path on a sparser graph but that is still terrible.
Copy link

codecov bot commented Jul 22, 2024

Codecov Report

Attention: Patch coverage is 96.65924% with 15 lines in your changes missing coverage. Please review.

Project coverage is 88.64%. Comparing base (ccc96bf) to head (2a6213d).
Report is 222 commits behind head on main.

Files Patch % Lines
...omo/contrib/piecewise/piecewise_linear_function.py 84.31% 8 Missing ⚠️
pyomo/contrib/piecewise/triangulations.py 98.26% 6 Missing ⚠️
pyomo/contrib/piecewise/transform/incremental.py 97.95% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3287      +/-   ##
==========================================
+ Coverage   88.59%   88.64%   +0.04%     
==========================================
  Files         874      877       +3     
  Lines       99124    99579     +455     
==========================================
+ Hits        87822    88267     +445     
- Misses      11302    11312      +10     
Flag Coverage Δ
linux 86.18% <96.65%> (+0.05%) ⬆️
osx 75.90% <96.65%> (+0.09%) ⬆️
other 86.68% <96.65%> (+0.05%) ⬆️
win 84.01% <96.65%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@emma58 emma58 left a comment

Choose a reason for hiding this comment

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

@sadavis1, I put in a test of the 3D Hamiltonian paths, but I'm not quite sure how to check that they are the right paths given the keys, so would appreciate input if you get a chance. Otherwise this looks good to go. (We've agreed to refactor the simplices and triangulation arguments for PiecewiseLinearFunctions in a follow-on PR (not targeting the upcoming release)).

@emma58
Copy link
Contributor

emma58 commented Aug 12, 2024

@jsiirola this is ready for you to take a look! :)

Copy link
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

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

Some minor questions (that can be dealt with in the future), but overall looks good.

pyomo/contrib/piecewise/triangulations.py Outdated Show resolved Hide resolved
pyomo/contrib/piecewise/triangulations.py Outdated Show resolved Hide resolved
pyomo/contrib/piecewise/triangulations.py Outdated Show resolved Hide resolved
# starting permutation, such that a fixed target symbol is either the image
# rho(1), or it is rho(n), depending on whether first or last is requested,
# where rho is the final permutation.
def _get_Gn_hamiltonian(n, start_permutation, target_symbol, last, _cache={}):
Copy link
Member

Choose a reason for hiding this comment

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

This is dangerous (although maybe you intend it?): all calls to _get_Gn_hamiltonian that do not provide _cache share the same dict object.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this is intended. But maybe @sadavis1 can confirm at some point... (And perhaps we should add a comment).

@blnicho blnicho merged commit 4a25f86 into Pyomo:main Aug 20, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

5 participants