Add the support of GreyBox in MindtPy #2988

ZedongPeng · 2023-09-08T04:14:17Z

Fixes #2767

Summary/Motivation:

This Pull Request adds the support of the Greybox in MindtPy.

Changes proposed in this PR:

Generates OA cuts using the information from the GreyBox model.
Fixes a bug in pyomo_grey_box_nlp.py

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.

bernalde

Good work, Zedong! I request some changes and also some revisions from our friends that motivated this development! @adowling2 !

bernalde · 2023-09-09T01:41:54Z

pyomo/contrib/mindtpy/algorithm_base_class.py

+            tmp=True,
+            ignore_infeasible=False,
+            tolerance=config.constraint_tolerance,
+        )


The use of this transformation is new. Is it motivated by our new tests? Is the transformation well-supported? @emma58

After MindtPy rewrite, we repeatedly use the fixed-nlp subproblem. Therefore, we will call 'contrib.deactivate_trivial_constraints' transformation and revert it after solving. I missed the feasibility subproblem and this might be a bug.

bernalde · 2023-09-09T01:42:32Z

pyomo/contrib/mindtpy/cut_generation.py

+                - (slack_var if config.add_slack else 0)
+                <= 0
+            )
+    # TODO: gurobi_persistent currently does not support greybox model.


Should this become an issue?

Gurobi will fail even when we deactivate the greybox block. Therefore, we cannot use Gurobi as the mip solver.

Is this a bug in gurobi_persistent? Why does it behave differently?

I will submit an issue related to this.

@emma58 Could you provide me a reference example?

from pyomo.environ import * m = ConcreteModel() m.b = Block() m.b.x = Var(bounds=(2, 5)) m.y = Var(domain=NonNegativeReals) m.c = Constraint(expr=m.b.x - m.y <= 4) m.obj = Objective(expr=m.b.x + m.y) m.b.deactivate() m.somewhere_safe_to_keep_things = Block() m.somewhere_safe_to_keep_things.x = Reference(m.b.x) opt = SolverFactory('gurobi_persistent') opt.set_instance(m) opt.solve(m, tee=True)

This is the main idea. You'll want to figure out how to do your own accounting in terms of finding the Vars you need to reference and naming the references, etc.

#3000 Issue opened

@emma58 . I don't think Reference is an elegant way to resolve this issue. I will comment the code of adding Gurobi lazy constraints and only support greybox in multi-tree implementation this time.
Btw, since persistent solver will be totally replaced by appsi_solver, does appsi_solver support both CPLEX and Gurobi callbacks as persistent solver now? @michaelbynum

appsi_gurobi does support callbacks. appsi_cplex will, but does not currently.

pyomo/contrib/mindtpy/cut_generation.py

pyomo/contrib/pynumero/interfaces/pyomo_grey_box_nlp.py

bernalde · 2023-09-09T01:48:56Z

The tests are failing because:
FAILED pyomo/contrib/mindtpy/tests/test_mindtpy.py::TestMindtPy::test_OA_APPSI_ipopt - RuntimeError: A feasible solution was not found, so no solution can be loaded.Please set opt.config.load_solution=False and check results.termination_condition and results.best_feasible_objective before loading a solution.
This has to do with the new option passed. Let's solve this first

ZedongPeng · 2023-09-19T23:42:39Z

This PR is ready for review. @emma58

bernalde · 2023-09-20T01:42:08Z

I don't think we're ready yet, Zedong.
Some errors from the tests:

=========================== short test summary info ============================
FAILED pyomo/contrib/mindtpy/tests/test_mindtpy.py::TestMindtPy::test_OA_APPSI_ipopt - RuntimeError: A feasible solution was not found, so no solution can be loaded.Please set opt.config.load_solution=False and check results.termination_condition and results.best_feasible_objective before loading a solution.
FAILED pyomo/environ/tests/test_environ.py::TestPyomoEnviron::test_tpl_import_time - AssertionError: Items in the first set but not the second:
'scipy' : Unexpected module found in 5 slowest-loading TPL modules
==== 2 failed, 12120 passed, 5009 skipped, 29 xfailed in 954.63s (0:15:54) =====

We are responsible for the first, and scipy taking so much longer seems to be a new thing. Let's aim to fix the first error and then we can keep trying.

jsiirola · 2023-09-20T14:23:31Z

The scipy error needs to be resolved, too: it indicates that something is causing scipy to be imported when we import pyomo.environ. This is likely because something in mindpy is doing import scipy instead of from pyomo.common.dependencies import scipy.

ZedongPeng · 2023-09-20T18:52:32Z

Hi @jsiirola . scipy cannot be imported from Pyomo dependency. How can I fix this?

Import code
from pyomo.common.dependencies.scipy.sparse import coo_matrix

Output

  File "/home/runner/work/pyomo/pyomo/pyomo/contrib/pynumero/interfaces/external_grey_box.py", line 15, in <module>
    from pyomo.common.dependencies.scipy.sparse import coo_matrix
  File "/home/runner/work/pyomo/pyomo/pyomo/common/dependencies.py", line 168, in __getattr__
    return getattr(_mod, attr)
  File "/home/runner/work/pyomo/pyomo/pyomo/common/dependencies.py", line 75, in __getattr__
    raise DeferredImportError(self._moduleunavailable_message())
pyomo.common.errors.DeferredImportError: The scipy.sparse module (an optional Pyomo dependency) failed to import: ModuleNotFoundError: No module named 'scipy'

jsiirola · 2023-09-20T19:15:36Z

@ZedongPeng, you want to do something like:

import pyomo.common.dependencies.scipy.sparse as scipy_sparse

# then later...
return scipy_sparse.coo_matrix((data, (row, col)), shape=(1, 5))

ZedongPeng · 2023-09-21T21:37:03Z

The error is not inside MindtPy now. How can we deal with it? @jsiirola

=========================== short test summary info ============================
FAILED pyomo/environ/tests/test_environ.py::TestPyomoEnviron::test_tpl_import_time - AssertionError: Items in the first set but not the second:
'scipy' : Unexpected module found in 5 slowest-loading TPL modules
==== 1 failed, 12121 passed, 5009 skipped, 29 xfailed in 1001.61s (0:16:41) ====

jsiirola · 2023-09-21T21:51:22Z

Ahh - the issue is that mindtpy is importing PyNumero (in pyomo/contrib/mindtpy/algorithm_base_class.py), which is importing scipy.

You can see this by running python pyomo/environ/tests/test_environ.py. You will see (in a lot of other output):

TPLS:
                            __future__:    179  (from pyomo.core.expr.visitor)
[....]
                          scipy.sparse:  12384  (from pyomo.contrib.pynumero.interfaces.external_grey_box)
                           scipy.stats: 134511  (from pyomo.contrib.pynumero.dependencies)

You can resolve things by deferring the import of pynumero using the attempt_import mechanism. Replace the pynumero import with:

from pyomo.common.dependencies import attempt_import

egb = attempt_import('pyomo.contrib.pynumero.interfaces.external_grey_box')[0]

and then replace instances of ExternalGreyBoxBlock with egb.ExternalGreyBoxBlock.

bernalde

There seem to be some test failing still.
E pyomo.common.errors.DeferredImportError: The pyomo.contrib.pynumero.interfaces.external_grey_box module (an optional Pyomo dependency) failed to import: ModuleNotFoundError: No module named 'scipy' ------- generated xml file: /home/runner/work/pyomo/pyomo/TEST-pyomo.xml ------- =========================== short test summary info ============================ ERROR pyomo/contrib/mindtpy/tests/test_mindtpy.py ERROR pyomo/contrib/mindtpy/tests/test_mindtpy_ECP.py ERROR pyomo/contrib/mindtpy/tests/test_mindtpy_feas_pump.py ERROR pyomo/contrib/mindtpy/tests/test_mindtpy_grey_box.py ERROR pyomo/contrib/mindtpy/tests/test_mindtpy_lp_nlp.py !!!!!!!!!!!!!!!!!!! Interrupted: 5 errors during collection !!!!!!!!!!!!!!!!!!!! ================== 30 skipped, 5 errors in 100.53s (0:01:40) ===================
These tests fail on the slim verrsion and pypy versions (those without numpy). We are importing numpy instead of attempting that import

emma58

This looks pretty good. A few questions/comments, and you still need to get the tests passing, which will just involve making sure you're never accidentally importing numpy or scipy via pynumero when they're not actually available.

emma58 · 2023-09-25T16:28:42Z

pyomo/contrib/mindtpy/algorithm_base_class.py

@@ -802,18 +810,21 @@ def init_rNLP(self, add_oa_cuts=True):
            MindtPy unable to handle the termination condition of the relaxed NLP.
        """
        config = self.config
-        m = self.working_model.clone()
+        self.rnlp = self.working_model.clone()


Do you really want to store this publicly on the solver object? If your users can find it and it doesn't start with underscore, they might start to rely on it. That's not necessarily bad--just something to think through.

emma58 · 2023-09-25T16:33:10Z

pyomo/contrib/mindtpy/algorithm_base_class.py

+        # related to https://github.com/Pyomo/pyomo/issues/2363
+        if (
+            'appsi' in config.mip_solver
+            or 'appsi' in config.nlp_solver
+            or (
+                config.mip_regularization_solver is not None
+                and 'appsi' in config.mip_regularization_solver
+            )
+        ):
+            config.load_solutions = False


This makes me think that load_solutions shouldn't be up to the user then, since there are cases where you need to force it to be False.

Yes. This option should not be up to the users. Is there a way to define an option (not open to users) instead of config (open to users) in Pyomo?

It's not really an option at all, just something you calculate based on some of the config options, right? You could probably store it privately on the algorithm class. Just make sure to restore state afterwards if you do keep it on the class.

Is it a good choice to change load_solutions to the attribute of the Solver Object?

That sounds fine to me. You should probably make it private as well. The only risk of doing this is that you should be careful to restore the state after the solve.

emma58 · 2023-09-25T16:35:30Z

pyomo/contrib/mindtpy/config_options.py

+    CONFIG.declare(
+        'load_solutions',
+        ConfigValue(
+            default=True,
+            description='Whether to load solutions in solve() function',
+            domain=bool,
+        ),
+    )


See my comment above, but I'm not sure of why this should be up to the user. I think it would be fine (and perhaps easier to support in the future) to keep this detail locally, or maybe privately on the solver object, and manage it yourself. Especially since it looks users don't actually have a choice if they want to use appsi solvers.

pyomo/contrib/mindtpy/algorithm_base_class.py

emma58 · 2023-10-02T15:23:19Z

pyomo/contrib/mindtpy/cut_generation.py

+        for index, output in enumerate(target_model_grey_box.outputs.values()):
+            dual_value = jacobians_model.dual[jacobian_model_grey_box][
+                output.name.replace("outputs", "output_constraints")
+            ]
+            target_model.MindtPy_utils.cuts.oa_cuts.add(
+                expr=copysign(1, sign_adjust * dual_value)
+                * (
+                    sum(
+                        jacobian_matrix[index][var_index] * (var - value(var))
+                        for var_index, var in enumerate(
+                            target_model_grey_box.inputs.values()
+                        )
+                    )
+                )
+                - (output - value(output))
+                - (slack_var if config.add_slack else 0)
+                <= 0
+            )


Are you sure that this is always going to create a correct cut, even if greybox changes its behavior? The part that looks potentially scary to me is calling enumerate over getting the values() from dictionaries coming from greybox. You will get deterministic ordering from values() as long as the dictionary is constructed in the same order every time, but if that changes, is this still right? Or does greybox make a promise about consistency with that order and the jacobian? (This is me wondering out of ignorance, but it may be worth adding a comment even if it is fine.)

If we don't call enumerate over values, any suggestions about how to implement it? Directly enumerate over variable?

I don't really know--you would know better than me. It's completely possible that this is fine. My concern is that you will end up with the wrong coefficient (jacobian_matrix[index][var_index]) paired with the wrong var - value(var) term if the target_model_greyboox.inputs dictionary is every constructed in a different order that doesn't correspond to the indices in jacobian_matrix. I don't know greybox well at all, so maybe it makes a promise that what I just said will never happen, in which case your code is totally fine. I'm just asking if you know if that's the case or not. It's always worth thinking carefully when you are relying on the order things come up when iterating over a dictionary...

Currently, I cannot come up with a good way to improve this since the evaluate_jacobian_outputs returns a scipy.sparse._coo.coo_matrix.

Maybe just put in a comment that you are relying on the order in which items are added to the target_model_greybox.inputs dictionary, so that future you (or someone else) will have a hint if this ever doesn't work.

pyomo/contrib/mindtpy/tests/MINLP_simple_grey_box.py

pyomo/contrib/mindtpy/tests/test_mindtpy_grey_box.py

bernalde · 2023-10-31T22:19:52Z

Test failures point to import of scipy when it is not available. A conditional attempt import should resolve it.

ZedongPeng · 2023-11-13T15:46:32Z

Now all the tests passed except the Jenkins one. Could you show me the output of Jenkins? @emma58 Thanks.

emma58 · 2023-11-14T17:35:55Z

@ZedongPeng, I'm not convinced those failures are actually your fault... I just merged main, and we'll see what happens. It looks like you still have a few comments above to address though?

codecov · 2023-11-14T18:42:15Z

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (15a7bff) 87.96% compared to head (3cf0fc2) 87.96%.
Report is 11 commits behind head on main.

Files	Patch %	Lines
pyomo/contrib/mindtpy/cut_generation.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2988   +/-   ##
=======================================
  Coverage   87.96%   87.96%           
=======================================
  Files         770      770           
  Lines       90162    90187   +25     
=======================================
+ Hits        79310    79332   +22     
- Misses      10852    10855    +3

Flag	Coverage Δ
linux	`85.29% <96.96%> (+<0.01%)`	⬆️
osx	`75.09% <15.15%> (-0.02%)`	⬇️
other	`85.47% <90.90%> (+<0.01%)`	⬆️
win	`82.54% <90.90%> (+<0.01%)`	⬆️

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.

ZedongPeng · 2023-11-15T18:52:36Z

I think this PR is ready to merge if all the tests pass.

emma58 · 2023-11-15T19:04:18Z

This is ready for a second review. @jsiirola, @mrmundt, @blnicho, @michaelbynum, any volunteers?

ZedongPeng added 7 commits August 21, 2023 16:54

fix load_state_into_pyomo bug

bb03a18

Merge branch 'main' into add_grey_box

1d508e7

add the support of greybox in mindtpy

ca54afc

add grey box test in mindtpy

ec05a3d

Merge branch 'main' into add_grey_box

5048b42

black format

c2877a3

black format

ccff416

bernalde suggested changes Sep 9, 2023

View reviewed changes

ZedongPeng added 3 commits September 10, 2023 21:49

add comment

e461e93

remove the support of greybox in LP/NLP gurobi

13a7213

black format

20ccbdc

ZedongPeng added 6 commits September 20, 2023 11:54

update import source

4288e33

remove redundant import

8d7f6c5

add config check for load_solutions

807e4f5

recover numpy and scipy import

30771b3

Merge branch 'main' into add_grey_box

231abc6

change numpy and scipy import

72b142b

change scipy import

ff401df

ZedongPeng added 2 commits September 21, 2023 17:59

fix import error

d82dcde

black format

cb1c2a9

bernalde suggested changes Sep 21, 2023

View reviewed changes

update mindtpy import in pynumero

e80c6dc

emma58 reviewed Oct 2, 2023

View reviewed changes

ZedongPeng added 2 commits October 31, 2023 14:25

fix

46cf8a5

black format

ba135b6

ZedongPeng added 4 commits October 31, 2023 18:50

fix import bug

8eacf0b

fix import bug

bd12664

black format

ebe91a6

fix import bug

2b45756

blnicho added the AT: PRE-TEST INSPECTED label Nov 9, 2023

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Nov 9, 2023

Merge branch 'main' into add_grey_box

96efbd4

emma58 added the AT: PRE-TEST INSPECTED label Nov 14, 2023

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Nov 14, 2023

ZedongPeng added 5 commits November 14, 2023 15:23

change load_solutions to attribute

73f2e16

black format

a08fd36

remove load_solutions configuration

ae84558

add comments to enumerate over values()

7bf0268

black format

993290f

Merge branch 'main' into add_grey_box

3cf0fc2

emma58 added the AT: PRE-TEST INSPECTED label Nov 15, 2023

emma58 approved these changes Nov 15, 2023

View reviewed changes

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Nov 15, 2023

blnicho mentioned this pull request Nov 21, 2023

Fix MindtPy bugs #3034

Merged

blnicho approved these changes Nov 27, 2023

View reviewed changes

emma58 merged commit 90291b0 into Pyomo:main Nov 27, 2023
27 checks passed

Add the support of GreyBox in MindtPy #2988

Add the support of GreyBox in MindtPy #2988

Conversation

ZedongPeng commented Sep 8, 2023

Fixes #2767

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

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

bernalde commented Sep 9, 2023

ZedongPeng commented Sep 19, 2023

bernalde commented Sep 20, 2023

jsiirola commented Sep 20, 2023

ZedongPeng commented Sep 20, 2023

jsiirola commented Sep 20, 2023

ZedongPeng commented Sep 21, 2023

jsiirola commented Sep 21, 2023

bernalde left a 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

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

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

bernalde commented Oct 31, 2023

ZedongPeng commented Nov 13, 2023

emma58 commented Nov 14, 2023

codecov bot commented Nov 14, 2023 • edited Loading

Codecov Report

ZedongPeng commented Nov 15, 2023

emma58 commented Nov 15, 2023

codecov bot commented Nov 14, 2023 •

edited

Loading