pyomo
pyomo copied to clipboard
New add_column() method for appsi solvers
Summary/Motivation:
This PR adds a new add_column() method to the solvers in the pyomo.contrib.appsi.solvers package. Like the add_column() method in the persistent solvers in the main pyomo.solvers package, this function allows you to add a new variable, and add it to the objective and to existing constraints, all in a single function call. This gives persistent solvers an opportunity to add the new variable more efficiently than is possible when modifying existing constraints one by one.
Changes proposed in this PR:
- Add a new
add_column()method to appsi solvers.
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:
- I agree my contributions are submitted under the BSD license.
- 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.
@michaelbynum, This is a PR you will probably want to review.
One thing I'd like to note is that I had to comment out some lines in the test for this PR (see these commented out lines). Those lines were intended to verify that the pyomo model components were modified as expected, but I could never get the comparisons to consistently work as expected, even though the string representation of the expressions showed they were equivalent. If there's a preferred way to make those comparisons, great, I'd love to have them added to the test.
Codecov Report
Attention: 4 lines in your changes are missing coverage. Please review.
Comparison is base (
ae85c0c) 87.98% compared to head (e9797f9) 87.99%. Report is 158 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| pyomo/contrib/appsi/base.py | 95.00% | 2 Missing :warning: |
| pyomo/contrib/appsi/solvers/gurobi.py | 85.71% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #2804 +/- ##
=======================================
Coverage 87.98% 87.99%
=======================================
Files 770 770
Lines 90242 90310 +68
=======================================
+ Hits 79400 79465 +65
- Misses 10842 10845 +3
| Flag | Coverage Δ | |
|---|---|---|
| linux | 85.32% <94.44%> (+<0.01%) |
:arrow_up: |
| osx | 75.12% <76.38%> (+<0.01%) |
:arrow_up: |
| other | 85.50% <94.44%> (+<0.01%) |
:arrow_up: |
| win | 82.57% <94.44%> (+0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@michaelbynum, can you take a look at this?
I think we should move this method into a "helper function" that takes in a model and a solver and modifies both. This way, we maintain that solver interfaces do not modify models as a "side effect".
@darrylmelander @michaelbynum this PR has been open for a while, any updates or plans for finishing this?
@darrylmelander @michaelbynum this PR has been open for a while, any updates or plans for finishing this?
If @darrylmelander is unavailable I could pick this up. It would still be a useful thing for APPSI (or any of our "in memory") solver interfaces to have.
I think we should move this method into a "helper function" that takes in a model and a solver and modifies both. This way, we maintain that solver interfaces do not modify models as a "side effect".
@michaelbynum would the function signature we currently use for persistent solvers be better? https://github.com/Pyomo/pyomo/blob/b5825e9e146552690d942903386102105251464c/pyomo/solvers/plugins/solvers/persistent_solver.py#L197-L214