Merge branch 'maingo_interface' of https://github.com/MAiNGO-github/pyomo into maingo_interface

Author: Clara Witte

.github/workflows/test_branches.yml

@@ -92,7 +92,7 @@ jobs:
           skip_doctest: 1
           TARGET: linux
           PYENV: conda
-          PACKAGES: mpi4py
+          PACKAGES: mpi4py==3.1.5
 
         - os: ubuntu-latest
           python: '3.10'

.github/workflows/test_pr_and_main.yml

@@ -93,7 +93,7 @@ jobs:
           skip_doctest: 1
           TARGET: linux
           PYENV: conda
-          PACKAGES: mpi4py
+          PACKAGES: mpi4py==3.1.5
 
         - os: ubuntu-latest
           python: '3.11'

doc/OnlineDocs/developer_reference/solvers.rst

@@ -84,6 +84,37 @@ be used with other Pyomo tools / capabilities.
    ...
    3 Declarations: x y obj
 
+In keeping with our commitment to backwards compatibility, both the legacy and
+future methods of specifying solver options are supported:
+
+.. testcode::
+   :skipif: not ipopt_available
+
+   import pyomo.environ as pyo
+
+   model = pyo.ConcreteModel()
+   model.x = pyo.Var(initialize=1.5)
+   model.y = pyo.Var(initialize=1.5)
+
+   def rosenbrock(model):
+       return (1.0 - model.x) ** 2 + 100.0 * (model.y - model.x**2) ** 2
+
+   model.obj = pyo.Objective(rule=rosenbrock, sense=pyo.minimize)
+
+   # Backwards compatible
+   status = pyo.SolverFactory('ipopt_v2').solve(model, options={'max_iter' : 6})
+   # Forwards compatible
+   status = pyo.SolverFactory('ipopt_v2').solve(model, solver_options={'max_iter' : 6})
+   model.pprint()
+
+.. testoutput::
+   :skipif: not ipopt_available
+   :hide:
+
+   2 Var Declarations
+   ...
+   3 Declarations: x y obj
+
 Using the new interfaces directly
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

pyomo/contrib/appsi/solvers/maingo.py

@@ -420,7 +420,7 @@ def _postsolve(self, timer: HierarchicalTimer):
             if status == maingopy.FEASIBLE_POINT:
                 results.globally_optimal = False
                 logger.warning(
-                    "MAiNGO did only find a feasible solution but did not prove its global optimality."
+                    "MAiNGO found a feasible solution but did not prove its global optimality."
                 )
         elif status == maingopy.INFEASIBLE:
             results.termination_condition = TerminationCondition.infeasible

pyomo/contrib/gdpopt/tests/test_LBB.py

@@ -59,6 +59,7 @@ def test_infeasible_GDP(self):
         self.assertIsNone(m.d.disjuncts[0].indicator_var.value)
         self.assertIsNone(m.d.disjuncts[1].indicator_var.value)
 
+    @unittest.skipUnless(z3_available, "Z3 SAT solver is not available")
     def test_infeasible_GDP_check_sat(self):
         """Test for infeasible GDP with check_sat option True."""
         m = ConcreteModel()

pyomo/contrib/gdpopt/tests/test_gdpopt.py

@@ -22,7 +22,6 @@
 from pyomo.common.collections import Bunch
 from pyomo.common.config import ConfigDict, ConfigValue
 from pyomo.common.fileutils import import_file, PYOMO_ROOT_DIR
-from pyomo.contrib.appsi.solvers.gurobi import Gurobi
 from pyomo.contrib.gdpopt.create_oa_subproblems import (
     add_util_block,
     add_disjunct_list,
@@ -767,6 +766,9 @@ def test_time_limit(self):
             results.solver.termination_condition, TerminationCondition.maxTimeLimit
         )
 
+    @unittest.skipUnless(
+        license_available, "No BARON license--8PP logical problem exceeds demo size"
+    )
     def test_LOA_8PP_logical_default_init(self):
         """Test logic-based outer approximation with 8PP."""
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_logical.py'))
@@ -870,6 +872,9 @@ def test_LOA_8PP_maxBinary(self):
         )
         ct.check_8PP_solution(self, eight_process, results)
 
+    @unittest.skipUnless(
+        license_available, "No BARON license--8PP logical problem exceeds demo size"
+    )
     def test_LOA_8PP_logical_maxBinary(self):
         """Test logic-based OA with max_binary initialization."""
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_logical.py'))
@@ -1050,7 +1055,11 @@ def assert_correct_disjuncts_active(
 
         self.assertTrue(fabs(value(eight_process.profit.expr) - 68) <= 1e-2)
 
-    @unittest.skipUnless(Gurobi().available(), "APPSI Gurobi solver is not available")
+    @unittest.skipUnless(
+        SolverFactory('appsi_gurobi').available(exception_flag=False)
+        and SolverFactory('appsi_gurobi').license_is_valid(),
+        "Legacy APPSI Gurobi solver is not available",
+    )
     def test_auto_persistent_solver(self):
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_model.py'))
         m = exfile.build_eight_process_flowsheet()
@@ -1126,6 +1135,9 @@ def test_RIC_8PP_default_init(self):
         )
         ct.check_8PP_solution(self, eight_process, results)
 
+    @unittest.skipUnless(
+        license_available, "No BARON license--8PP logical problem exceeds demo size"
+    )
     def test_RIC_8PP_logical_default_init(self):
         """Test logic-based outer approximation with 8PP."""
         exfile = import_file(join(exdir, 'eight_process', 'eight_proc_logical.py'))

pyomo/contrib/gdpopt/util.py

@@ -553,6 +553,13 @@ def _add_bigm_constraint_to_transformed_model(m, constraint, block):
     # making a Reference to the ComponentData so that it will look like an
     # indexed component for now. If I redesign bigm at some point, then this
     # could be prettier.
-    bigm._transform_constraint(Reference(constraint), parent_disjunct, None, [], [])
+    bigm._transform_constraint(
+        Reference(constraint),
+        parent_disjunct,
+        None,
+        [],
+        [],
+        1 - parent_disjunct.binary_indicator_var,
+    )
     # Now get rid of it because this is a class attribute!
     del bigm._config

pyomo/contrib/incidence_analysis/interface.py

@@ -28,7 +28,7 @@
     scipy as sp,
     plotly,
 )
-from pyomo.common.deprecation import deprecated
+from pyomo.common.deprecation import deprecated, deprecation_warning
 from pyomo.contrib.incidence_analysis.config import get_config_from_kwds
 from pyomo.contrib.incidence_analysis.matching import maximum_matching
 from pyomo.contrib.incidence_analysis.connected import get_independent_submatrices
@@ -453,11 +453,29 @@ def _validate_input(self, variables, constraints):
                 raise ValueError("Neither variables nor a model have been provided.")
             else:
                 variables = self.variables
+        elif self._incidence_graph is not None:
+            # If variables were provided and an incidence graph is cached,
+            # make sure the provided variables exist in the graph.
+            for var in variables:
+                if var not in self._var_index_map:
+                    raise KeyError(
+                        f"Variable {var} does not exist in the cached"
+                        " incidence graph."
+                    )
         if constraints is None:
             if self._incidence_graph is None:
                 raise ValueError("Neither constraints nor a model have been provided.")
             else:
                 constraints = self.constraints
+        elif self._incidence_graph is not None:
+            # If constraints were provided and an incidence graph is cached,
+            # make sure the provided constraints exist in the graph.
+            for con in constraints:
+                if con not in self._con_index_map:
+                    raise KeyError(
+                        f"Constraint {con} does not exist in the cached"
+                        " incidence graph."
+                    )
 
         _check_unindexed(variables + constraints)
         return variables, constraints
@@ -854,7 +872,7 @@ def dulmage_mendelsohn(self, variables=None, constraints=None):
         # Hopefully this does not get too confusing...
         return var_partition, con_partition
 
-    def remove_nodes(self, nodes, constraints=None):
+    def remove_nodes(self, variables=None, constraints=None):
         """Removes the specified variables and constraints (columns and
         rows) from the cached incidence matrix.
 
@@ -866,35 +884,76 @@ def remove_nodes(self, nodes, constraints=None):
 
         Parameters
         ----------
-        nodes: list
-            VarData or ConData objects whose columns or rows will be
-            removed from the incidence matrix.
+        variables: list
+            VarData objects whose nodes will be removed from the incidence graph
         constraints: list
-            VarData or ConData objects whose columns or rows will be
-            removed from the incidence matrix.
+            ConData objects whose nodes will be removed from the incidence graph
+
+        .. note::
+
+           **Deprecation in Pyomo v6.7.2.dev0**
+
+           The pre-6.7.2.dev0 implementation of ``remove_nodes`` allowed variables and
+           constraints to remove to be specified in a single list. This made
+           error checking difficult, and indeed, if invalid components were
+           provided, we carried on silently instead of throwing an error or
+           warning. As part of a fix to raise an error if an invalid component
+           (one that is not part of the incidence graph) is provided, we now require
+           variables and constraints to be specified separately.
 
         """
         if constraints is None:
             constraints = []
+        if variables is None:
+            variables = []
         if self._incidence_graph is None:
             raise RuntimeError(
                 "Attempting to remove variables and constraints from cached "
                 "incidence matrix,\nbut no incidence matrix has been cached."
             )
-        to_exclude = ComponentSet(nodes)
-        to_exclude.update(constraints)
-        vars_to_include = [v for v in self.variables if v not in to_exclude]
-        cons_to_include = [c for c in self.constraints if c not in to_exclude]
+
+        vars_to_validate = []
+        cons_to_validate = []
+        depr_msg = (
+            "In IncidenceGraphInterface.remove_nodes, passing variables and"
+            " constraints in the same list is deprecated. Please separate your"
+            " variables and constraints and pass them in the order variables,"
+            " constraints."
+        )
+        if any(var in self._con_index_map for var in variables) or any(
+            con in self._var_index_map for con in constraints
+        ):
+            deprecation_warning(depr_msg, version="6.7.2.dev0")
+        # If we received variables/constraints in the same list, sort them.
+        # Any unrecognized objects will be caught by _validate_input.
+        for var in variables:
+            if var in self._con_index_map:
+                cons_to_validate.append(var)
+            else:
+                vars_to_validate.append(var)
+        for con in constraints:
+            if con in self._var_index_map:
+                vars_to_validate.append(con)
+            else:
+                cons_to_validate.append(con)
+
+        variables, constraints = self._validate_input(
+            vars_to_validate, cons_to_validate
+        )
+        v_exclude = ComponentSet(variables)
+        c_exclude = ComponentSet(constraints)
+        vars_to_include = [v for v in self.variables if v not in v_exclude]
+        cons_to_include = [c for c in self.constraints if c not in c_exclude]
         incidence_graph = self._extract_subgraph(vars_to_include, cons_to_include)
         # update attributes
         self._variables = vars_to_include
         self._constraints = cons_to_include
         self._incidence_graph = incidence_graph
         self._var_index_map = ComponentMap(
-            (var, i) for i, var in enumerate(self.variables)
+            (var, i) for i, var in enumerate(vars_to_include)
         )
         self._con_index_map = ComponentMap(
-            (con, i) for i, con in enumerate(self._constraints)
+            (con, i) for i, con in enumerate(cons_to_include)
         )
 
     def plot(self, variables=None, constraints=None, title=None, show=True):

pyomo/contrib/incidence_analysis/tests/test_interface.py

@@ -634,17 +634,15 @@ def test_exception(self):
         nlp = PyomoNLP(model)
         igraph = IncidenceGraphInterface(nlp)
 
-        with self.assertRaises(RuntimeError) as exc:
+        with self.assertRaisesRegex(KeyError, "does not exist"):
             variables = [model.P]
             constraints = [model.ideal_gas]
             igraph.maximum_matching(variables, constraints)
-        self.assertIn("must be unindexed", str(exc.exception))
 
-        with self.assertRaises(RuntimeError) as exc:
+        with self.assertRaisesRegex(KeyError, "does not exist"):
             variables = [model.P]
             constraints = [model.ideal_gas]
             igraph.block_triangularize(variables, constraints)
-        self.assertIn("must be unindexed", str(exc.exception))
 
 
 @unittest.skipUnless(networkx_available, "networkx is not available.")
@@ -885,17 +883,15 @@ def test_exception(self):
         model = make_gas_expansion_model()
         igraph = IncidenceGraphInterface(model)
 
-        with self.assertRaises(RuntimeError) as exc:
+        with self.assertRaisesRegex(KeyError, "does not exist"):
             variables = [model.P]
             constraints = [model.ideal_gas]
             igraph.maximum_matching(variables, constraints)
-        self.assertIn("must be unindexed", str(exc.exception))
 
-        with self.assertRaises(RuntimeError) as exc:
+        with self.assertRaisesRegex(KeyError, "does not exist"):
             variables = [model.P]
             constraints = [model.ideal_gas]
             igraph.block_triangularize(variables, constraints)
-        self.assertIn("must be unindexed", str(exc.exception))
 
     @unittest.skipUnless(scipy_available, "scipy is not available.")
     def test_remove(self):
@@ -923,7 +919,7 @@ def test_remove(self):
         # Say we know that these variables and constraints should
         # be matched...
         vars_to_remove = [model.F[0], model.F[2]]
-        cons_to_remove = (model.mbal[1], model.mbal[2])
+        cons_to_remove = [model.mbal[1], model.mbal[2]]
         igraph.remove_nodes(vars_to_remove, cons_to_remove)
         variable_set = ComponentSet(igraph.variables)
         self.assertNotIn(model.F[0], variable_set)
@@ -1309,7 +1305,7 @@ def test_remove(self):
         # matrix.
         vars_to_remove = [m.flow_comp[1]]
         cons_to_remove = [m.flow_eqn[1]]
-        igraph.remove_nodes(vars_to_remove + cons_to_remove)
+        igraph.remove_nodes(vars_to_remove, cons_to_remove)
         var_dmp, con_dmp = igraph.dulmage_mendelsohn()
         var_con_set = ComponentSet(igraph.variables + igraph.constraints)
         underconstrained_set = ComponentSet(
@@ -1460,6 +1456,42 @@ def test_remove_no_matrix(self):
         with self.assertRaisesRegex(RuntimeError, "no incidence matrix"):
             igraph.remove_nodes([m.v1])
 
+    def test_remove_bad_node(self):
+        m = pyo.ConcreteModel()
+        m.x = pyo.Var([1, 2, 3])
+        m.eq = pyo.Constraint(pyo.PositiveIntegers)
+        m.eq[1] = m.x[1] * m.x[2] == m.x[3]
+        m.eq[2] = m.x[1] + 2 * m.x[2] == 3 * m.x[3]
+        igraph = IncidenceGraphInterface(m)
+        with self.assertRaisesRegex(KeyError, "does not exist"):
+            # Suppose we think something like this should work. We should get
+            # an error, and not silently do nothing.
+            igraph.remove_nodes([m.x], [m.eq[1]])
+
+        with self.assertRaisesRegex(KeyError, "does not exist"):
+            igraph.remove_nodes(None, [m.eq])
+
+        with self.assertRaisesRegex(KeyError, "does not exist"):
+            igraph.remove_nodes([[m.x[1], m.x[2]], [m.eq[1]]])
+
+    def test_remove_varcon_samelist_deprecated(self):
+        m = pyo.ConcreteModel()
+        m.x = pyo.Var([1, 2, 3])
+        m.eq = pyo.Constraint(pyo.PositiveIntegers)
+        m.eq[1] = m.x[1] * m.x[2] == m.x[3]
+        m.eq[2] = m.x[1] + 2 * m.x[2] == 3 * m.x[3]
+
+        igraph = IncidenceGraphInterface(m)
+        # This raises a deprecation warning. When the deprecated functionality
+        # is removed, this will fail, and this test should be updated accordingly.
+        igraph.remove_nodes([m.eq[1], m.x[1]])
+        self.assertEqual(len(igraph.variables), 2)
+        self.assertEqual(len(igraph.constraints), 1)
+
+        igraph.remove_nodes([], [m.eq[2], m.x[2]])
+        self.assertEqual(len(igraph.variables), 1)
+        self.assertEqual(len(igraph.constraints), 0)
+
 
 @unittest.skipUnless(networkx_available, "networkx is not available.")
 @unittest.skipUnless(scipy_available, "scipy is not available.")
@@ -1840,7 +1872,7 @@ def test_var_elim(self):
         for adj_con in igraph.get_adjacent_to(m.x[1]):
             for adj_var in igraph.get_adjacent_to(m.eq4):
                 igraph.add_edge(adj_var, adj_con)
-        igraph.remove_nodes([m.x[1], m.eq4])
+        igraph.remove_nodes([m.x[1]], [m.eq4])
 
         assert ComponentSet(igraph.variables) == ComponentSet([m.x[2], m.x[3], m.x[4]])
         assert ComponentSet(igraph.constraints) == ComponentSet([m.eq1, m.eq2, m.eq3])

pyomo/contrib/pynumero/algorithms/solvers/cyipopt_solver.py

@@ -319,7 +319,13 @@ def license_is_valid(self):
         return True
 
     def version(self):
-        return tuple(int(_) for _ in cyipopt.__version__.split("."))
+        def _int(x):
+            try:
+                return int(x)
+            except:
+                return x
+
+        return tuple(_int(_) for _ in cyipopt_interface.cyipopt.__version__.split("."))
 
     def solve(self, model, **kwds):
         config = self.config(kwds, preserve_implicit=True)