Mindtpy rewrite #2887

ZedongPeng · 2023-06-26T15:42:41Z

Summary/Motivation:

This Pull Request completes several tasks left in MindtPy rewrite TODO list (#2709).

Changes proposed in this PR:

Redesign of single-tree implementation.
Add support for appsi_highs and set it as the default MIP solver. Support more milp solver in mindtpy #2874
Add available() and license_is_valid() check for subsolvers. (Related issue)
Check and improve the var_list generation in MindtPy. Mindtpy rewrite #2654 (comment)
Remove feasible_no_good_cuts. Mindtpy rewrite #2654 (comment)
Fix docstring Mindtpy rewrite #2654 (comment)
Replace setatter Mindtpy rewrite #2654 (comment)
Remove configuration reduce_level_coef and activate use_bb_tree_incumbent by default.
Make good use of appsi_solver to improve the performance of MindtPy.

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.

…ndtpy-rewrite

bernalde

Good PR @ZedongPeng I put several comments there. Some of them are really minor, others are considerable. I'm not quite following why the mip and nlp solve files are gone.
There are some open TODOs which should be addressed now.
Finally, I'm concerned about the effect on performance. While this PR is in review, can you submit the runs in Euler to see that there are not making much difference?

bernalde · 2023-06-26T15:52:47Z

pyomo/contrib/mindtpy/tests/test_mindtpy_feas_pump.py

@@ -17,7 +17,8 @@
 from pyomo.contrib.mindtpy.tests.feasibility_pump1 import Feasibility_Pump1
 from pyomo.contrib.mindtpy.tests.feasibility_pump2 import Feasibility_Pump2

-required_solvers = ('ipopt', 'glpk', 'cplex')
+required_solvers = ('ipopt', 'cplex')
+# TODO: 'appsi_highs' will fail here.


Why does it fail? It breaks or just does not converge?

I thinks this is related to a bug inside appsi_solver. I just submitted an issue #2888.

Add the issue # in a comment and let's not wait for that to be solved to merge this PR

#2888 was closed yesterday, so you should verify that this works now and remove the comment.

+1 on this; does this work now?

appsi_highs still does not work well with MindtPy, more specifically the feasibility pump method.
Here is the related issue #2951

pyomo/contrib/mindtpy/tests/test_mindtpy_feas_pump.py

pyomo/contrib/mindtpy/util.py

pyomo/contrib/mindtpy/algorithm_base_class.py

pyomo/contrib/mindtpy/nlp_solve.py

pyomo/contrib/mindtpy/single_tree.py

ZedongPeng · 2023-06-26T16:54:55Z

@bernalde No problem. I'll run the benchmarks on Euler.
For your confusion about nlp_solve.py and mip_solver.py, here is the answer.

Before PR Mindtpy rewrite #2654 , the whole MindtPy algorithm relies heavily on solve_data.
In Mindtpy rewrite #2654 , we defined the algorithm base class and defined the elements in solve_data as the attributes of the class and defined the util functions as the methods of the class. However, this only applies to the multi-tree implementation since I didn't find a good way to interact between the algorithm base class and the callback functions of the MIP solvers.
After PR Mindtpy rewrite #2654 , the functions in nlp_solve.py and mip_solver.py are only used in the single-tree implementation. The multi-tree implementation can use the corresponding methods defined in algorithm_base_class.py.
The biggest change in this pull request is the single-tree implementation. Previously, the single-tree implementation relies on solve_data and can not access the attributes and methods of the algorithm base class. Now, I pass the mindtpy object to the persistent solvers, so that we can access the attributes and methods of the algorithm base class inside the callbacks of MIP solvers. Therefore, we can remove nlp_solve.py and mip_solver.py.

bernalde · 2023-06-26T17:06:02Z

Thank you for the clarification! The tests seem to be running well, and once you address the comments in my review, this should be ready for revision and merging. I would like to have the performance results by then just to be sure that we are not merging anything that makes us way slower. Feel free to report on those here.

ZedongPeng · 2023-06-26T17:16:01Z

No problem. Since this PR includes the UpdateConfig of appsi_solver, I'll also run benchmarks using appsi_solvers to see if it will improve the performance.

jsiirola · 2023-06-26T22:04:14Z

Note: test failures on Jenkins are collection failures on Python 3.11:

.pyomo.contrib.mindtpy.tests.test_mindtpy
.pyomo.contrib.mindtpy.tests.test_mindtpy_ECP

exception:

pyomo/pyomo/contrib/mindtpy/tests/test_mindtpy.py:60: in <module>
    if all(SolverFactory(s).available() for s in required_solvers):
pyomo/pyomo/contrib/mindtpy/tests/test_mindtpy.py:60: in <genexpr>
    if all(SolverFactory(s).available() for s in required_solvers):
pyomo/pyomo/contrib/appsi/base.py:1633: in available
    raise ApplicationError(f'Solver {self.__class__} is not available ({ans}).')
E   pyomo.common.errors.ApplicationError: Solver <class 'pyomo.contrib.appsi.base.SolverFactoryClass.register.<locals>.decorator.<locals>.LegacySolver'> is not available (NotFound).

...both appear to be because an APPSI solver is not available.

bernalde · 2023-07-05T19:37:24Z

We need to address all the open comments and manage the test failures when APPSI is not available, as reported on Jenkins @ZedongPeng

ZedongPeng · 2023-08-22T04:55:35Z

In MINLP3_simple.py, if X equals -1, then log(0) will happen in const1 and result in the math domain error.
Therefore, I change the lower bound of X to -0.9.

PS: This will only happen with Python 3.7 or 3.8. If we use Python 3.10, math domain error will not be reported. Since this error happens inside the Gurobi callback function, I think gurobipy in Python 3.10 might have some special handling with the math domain error.

pyomo/pyomo/contrib/mindtpy/tests/MINLP3_simple.py

Lines 65 to 74 in c569b78

    
           Y = m.Y = Var(J, domain=Binary, initialize=initY) 
        
           # CONTINUOUS VARIABLES 
        
           X = m.X = Var(I, domain=Reals, initialize=initX, bounds=(-1, 50)) 
        
           """Constraint definitions""" 
        
           # CONSTRAINTS 
        
           m.const1 = Constraint(expr=-X[2] + 5 * log(X[1] + 1) + 3 * Y[1] >= 0) 
        
           m.const2 = Constraint(expr=-X[2] + X[1] ** 2 - Y[1] <= 1) 
        
           m.const3 = Constraint(expr=X[1] + X[2] + 20 * Y[1] <= 24) 
        
           m.const4 = Constraint(expr=2 * X[2] + 3 * X[1] <= 10)

mrmundt

This looks pretty good; I found a few places where the docstrings aren't correct.

pyomo/contrib/mindtpy/algorithm_base_class.py

pyomo/contrib/mindtpy/extended_cutting_plane.py

mrmundt · 2023-08-22T17:54:34Z

pyomo/contrib/mindtpy/tests/test_mindtpy_feas_pump.py

@@ -17,7 +17,8 @@
 from pyomo.contrib.mindtpy.tests.feasibility_pump1 import Feasibility_Pump1
 from pyomo.contrib.mindtpy.tests.feasibility_pump2 import Feasibility_Pump2

-required_solvers = ('ipopt', 'glpk', 'cplex')
+required_solvers = ('ipopt', 'cplex')
+# TODO: 'appsi_highs' will fail here.


+1 on this; does this work now?

emma58

@ZedongPeng, it looks like there are a couple changes you missed, but otherwise this looks good.

pyomo/contrib/mindtpy/algorithm_base_class.py

emma58 · 2023-08-22T18:17:33Z

pyomo/contrib/mindtpy/algorithm_base_class.py

+                    "APPSI-CPLEX cannot get duals for mixed-integer problems"
+                    "mip_solver will be changed to CPLEX."


I'm still confused about this logging message: What duals are you getting for a MIP and why do you have to change to cplex?

pyomo/contrib/mindtpy/util.py

ZedongPeng · 2023-08-22T21:33:22Z

I resolved the latest comments and this PR is ready to be merged after all checks succeed. @bernalde @emma58 @mrmundt

mrmundt · 2023-08-22T22:00:35Z

@ZedongPeng - the osx doctest failure looks like a spurious one (something took a fraction of a second longer than expected), so feel free to ignore it.

mrmundt · 2023-08-22T22:13:51Z

@ZedongPeng - Jenkins failure is also not your fault. Something is wrong with the machine. I'll go fix it.

codecov · 2023-08-22T23:48:34Z

Codecov Report

Patch coverage: 62.89% and project coverage change: +0.24% 🎉

Comparison is base (c569b78) 87.84% compared to head (0d9430d) 88.09%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2887      +/-   ##
==========================================
+ Coverage   87.84%   88.09%   +0.24%     
==========================================
  Files         770      768       -2     
  Lines       89665    89211     -454     
==========================================
- Hits        78764    78587     -177     
+ Misses      10901    10624     -277

Flag	Coverage Δ
linux	`85.16% <62.26%> (+0.29%)`	⬆️
osx	`74.94% <8.17%> (+0.36%)`	⬆️
other	`85.34% <62.26%> (+0.29%)`	⬆️
win	`82.40% <62.26%> (+0.28%)`	⬆️

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

Files Changed	Coverage Δ
pyomo/contrib/mindtpy/algorithm_base_class.py	`81.81% <ø> (+0.61%)`	⬆️
pyomo/contrib/mindtpy/config_options.py	`100.00% <ø> (ø)`
pyomo/contrib/mindtpy/cut_generation.py	`54.19% <0.00%> (-1.53%)`	⬇️
pyomo/contrib/mindtpy/tabu_list.py	`25.00% <0.00%> (ø)`
...yomo/contrib/mindtpy/global_outer_approximation.py	`29.16% <50.00%> (-7.88%)`	⬇️
pyomo/contrib/mindtpy/util.py	`79.32% <50.00%> (+14.95%)`	⬆️
pyomo/contrib/mindtpy/extended_cutting_plane.py	`83.09% <70.58%> (-1.52%)`	⬇️
pyomo/contrib/mindtpy/single_tree.py	`58.58% <74.68%> (+7.64%)`	⬆️
pyomo/contrib/mindtpy/feasibility_pump.py	`100.00% <100.00%> (ø)`
pyomo/contrib/mindtpy/outer_approximation.py	`90.32% <100.00%> (-0.31%)`	⬇️

... and 6 files with indirect coverage changes

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

ZedongPeng added 16 commits February 20, 2023 23:48

update mindtpy docstring

a7fd6ee

replace setatter

18358f7

replace setattr

65f522c

generate var_list from var_set

80855fe

remove feasible_no_good_cuts

18e6f40

Merge branch 'mindtpy-rewrite' of github.com:ZedongPeng/pyomo into mi…

8e03c40

…ndtpy-rewrite

Merge branch 'main' into mindtpy-rewrite

0d4b126

add support for appsi_highs

4afc6ae

Merge branch 'main' into mindtpy-rewrite

15f4726

improve mindtpy subsolver definition and usage

596ddb7

improve single tree implementation

7ea1ea3

improve single-tree implementation

8ccf173

add check for subsolvers

1ffb3c0

format

9dc2847

change default MIP solver to appsi_highs

e9bfc46

fix typo

51e7d9d

bernalde suggested changes Jun 26, 2023

View reviewed changes

jsiirola added the AT: PRE-TEST INSPECTED label Jun 26, 2023

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Jun 26, 2023

ZedongPeng added 3 commits June 27, 2023 11:48

Merge branch 'main' into mindtpy-rewrite

67f1f08

fix bug

235611a

fix typo

03880e9

blnicho requested a review from emma58 June 27, 2023 18:42

ZedongPeng added 2 commits July 6, 2023 17:35

fix mindtpy singletree implementation bug

2110e81

add description for mip_start_lazy_oa_cuts

b6086bc

fix math domain error

2cb4c22

bernalde approved these changes Aug 22, 2023

View reviewed changes

mrmundt added the AT: PRE-TEST INSPECTED label Aug 22, 2023

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Aug 22, 2023

mrmundt requested a review from emma58 August 22, 2023 17:15

mrmundt requested changes Aug 22, 2023

View reviewed changes

emma58 requested changes Aug 22, 2023

View reviewed changes

ZedongPeng added 3 commits August 22, 2023 15:29

fix docstring

c0d8ad9

change GUROBI to Gurobi

fa810fd

remove appsi_highs support

8014474

mrmundt approved these changes Aug 22, 2023

View reviewed changes

mrmundt added the AT: PRE-TEST INSPECTED label Aug 22, 2023

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Aug 22, 2023

ZedongPeng added 5 commits August 22, 2023 16:46

remove wrong calculate_dual_at_solution check

f26ae11

black format

3fe865d

add comments

59941ba

add comments for optimalitytarget

987b40a

black format

0d9430d

mrmundt added the AT: PRE-TEST INSPECTED label Aug 22, 2023

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Aug 22, 2023

mrmundt added the AT: RETEST label Aug 22, 2023

pyomo-autotest removed the AT: RETEST label Aug 22, 2023

bernalde approved these changes Aug 23, 2023

View reviewed changes

emma58 approved these changes Aug 23, 2023

View reviewed changes

mrmundt merged commit 91a31d0 into Pyomo:main Aug 23, 2023
28 of 30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mindtpy rewrite #2887

Mindtpy rewrite #2887

ZedongPeng commented Jun 26, 2023 •

edited

Loading

bernalde left a comment

bernalde Jun 26, 2023

ZedongPeng Jun 27, 2023

bernalde Jul 7, 2023

emma58 Aug 9, 2023

mrmundt Aug 22, 2023

ZedongPeng Aug 22, 2023

ZedongPeng commented Jun 26, 2023 •

edited

Loading

bernalde commented Jun 26, 2023

ZedongPeng commented Jun 26, 2023

jsiirola commented Jun 26, 2023

bernalde commented Jul 5, 2023

ZedongPeng commented Aug 22, 2023

mrmundt left a comment

mrmundt Aug 22, 2023

emma58 left a comment

emma58 Aug 22, 2023

ZedongPeng commented Aug 22, 2023

mrmundt commented Aug 22, 2023

mrmundt commented Aug 22, 2023

codecov bot commented Aug 22, 2023 •

edited

Loading

		"APPSI-CPLEX cannot get duals for mixed-integer problems"
		"mip_solver will be changed to CPLEX."

Mindtpy rewrite #2887

Mindtpy rewrite #2887

Conversation

ZedongPeng commented Jun 26, 2023 • edited Loading

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

bernalde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZedongPeng commented Jun 26, 2023 • edited Loading

bernalde commented Jun 26, 2023

ZedongPeng commented Jun 26, 2023

jsiirola commented Jun 26, 2023

bernalde commented Jul 5, 2023

ZedongPeng commented Aug 22, 2023

mrmundt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emma58 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZedongPeng commented Aug 22, 2023

mrmundt commented Aug 22, 2023

mrmundt commented Aug 22, 2023

codecov bot commented Aug 22, 2023 • edited Loading

Codecov Report

ZedongPeng commented Jun 26, 2023 •

edited

Loading

ZedongPeng commented Jun 26, 2023 •

edited

Loading

codecov bot commented Aug 22, 2023 •

edited

Loading