Merge pull request #3678 from mrmundt/lpwriter_bug

Bugfix: lp_writer SOS constraints and row_order

Author: mrmundt

examples/pyomo/sos/sos2_piecewise.py

@@ -22,7 +22,7 @@
 
 model = pyo.ConcreteModel()
 
-model.index_set = pyo.Set(initialize=[1, 2])
+model.idx_set = pyo.Set(initialize=[1, 2])
 DOMAIN_PTS = {1: [1, 2, 3], 2: [1, 2, 3]}
 F = {1: [1, 4, 9], 2: [1, 4, 9]}
 # Note we can also implement this like below
@@ -37,18 +37,18 @@ def SOS_indices_init(model, t):
 
 
 model.SOS_indices = pyo.Set(
-    model.index_set, dimen=2, ordered=True, initialize=SOS_indices_init
+    model.idx_set, dimen=2, ordered=True, initialize=SOS_indices_init
 )
 
 
 def sos_var_indices_init(model):
-    return [(t, i) for t in model.index_set for i in range(len(DOMAIN_PTS[t]))]
+    return [(t, i) for t in model.idx_set for i in range(len(DOMAIN_PTS[t]))]
 
 
 model.sos_var_indices = pyo.Set(ordered=True, dimen=2, initialize=sos_var_indices_init)
 
-model.x = pyo.Var(model.index_set)  # domain variable
-model.Fx = pyo.Var(model.index_set)  # range variable
+model.x = pyo.Var(model.idx_set)  # domain variable
+model.Fx = pyo.Var(model.idx_set)  # range variable
 model.y = pyo.Var(model.sos_var_indices, within=pyo.NonNegativeReals)  # SOS2 variable
 
 model.obj = pyo.Objective(expr=pyo.sum_product(model.Fx), sense=pyo.maximize)
@@ -73,11 +73,11 @@ def constraint3_rule(model, t):
     return sum(model.y[t, j] for j in range(len(DOMAIN_PTS[t]))) == 1
 
 
-model.constraint1 = pyo.Constraint(model.index_set, rule=constraint1_rule)
-model.constraint2 = pyo.Constraint(model.index_set, rule=constraint2_rule)
-model.constraint3 = pyo.Constraint(model.index_set, rule=constraint3_rule)
+model.constraint1 = pyo.Constraint(model.idx_set, rule=constraint1_rule)
+model.constraint2 = pyo.Constraint(model.idx_set, rule=constraint2_rule)
+model.constraint3 = pyo.Constraint(model.idx_set, rule=constraint3_rule)
 model.SOS_set_constraint = pyo.SOSConstraint(
-    model.index_set, var=model.y, index=model.SOS_indices, sos=2
+    model.idx_set, var=model.y, index=model.SOS_indices, sos=2
 )
 
 # Fix the answer for testing purposes

pyomo/repn/plugins/lp_writer.py

@@ -19,6 +19,7 @@
     InEnum,
     document_kwargs_from_configdict,
 )
+from pyomo.common.deprecation import deprecation_warning
 from pyomo.common.gc_manager import PauseGC
 from pyomo.common.timing import TicTocTimer
 
@@ -30,12 +31,10 @@
     Param,
     Expression,
     SOSConstraint,
-    SortComponents,
     Suffix,
     SymbolMap,
     minimize,
 )
-from pyomo.core.base.component import ActiveComponent
 from pyomo.core.base.label import LPFileLabeler, NumericLabeler
 from pyomo.opt import WriterFactory
 from pyomo.repn.linear import LinearRepnVisitor
@@ -48,6 +47,7 @@
     initialize_var_map_from_column_order,
     int_float,
     ordered_active_constraints,
+    row_order2row_map,
 )
 
 ### FIXME: Remove the following as soon as non-active components no
@@ -555,18 +555,16 @@ def write(self, model):
                     )
                 )
             if self.config.row_order:
-                # sort() is stable (per Python docs), so we can let
-                # all unspecified rows have a row number one bigger than
-                # the number of rows specified by the user ordering.
-                _n = len(row_order)
-                sos.sort(key=lambda x: _row_getter(x, _n))
+                row_map = row_order2row_map(self.config)
+                _n = len(row_map)
+                sos.sort(key=lambda x: row_map.get(id(x), _n))
 
             ostream.write("\nSOS\n")
             for soscon in sos:
                 ostream.write(f'\n{getSymbol(soscon)}: S{soscon.level}::\n')
                 for v, w in getattr(soscon, 'get_items', soscon.items)():
                     if w.__class__ not in int_float:
-                        w = float(f)
+                        w = float(w)
                     ostream.write(f"  {getSymbol(v)}:{w!s}\n")
 
         ostream.write("\nend\n")

pyomo/repn/tests/cpxlp/test_lpv2.py

@@ -20,6 +20,34 @@
 from pyomo.repn.plugins.lp_writer import LPWriter
 
 
+def create_sos_model():
+    m = pyo.ConcreteModel()
+
+    m.IDX = pyo.Set(initialize=[1, 2])
+    m.SOS2_VARS = pyo.Set(m.IDX, initialize={1: [1, 2, 3], 2: [4, 5, 6]})
+    m.SOS3_VARS = pyo.Set(m.IDX, initialize={1: [1, 3, 5], 2: [2, 4, 6]})
+
+    m.x = pyo.Var(pyo.RangeSet(6))
+
+    m.sos1 = pyo.SOSConstraint(var=m.x, sos=2)
+    m.sos2 = pyo.SOSConstraint(m.IDX, var=m.x, index=m.SOS2_VARS, sos=1)
+    m.sos3 = pyo.SOSConstraint(m.IDX, var=m.x, index=m.SOS3_VARS, sos=2)
+
+    @m.Constraint()
+    def con1(m, i):
+        return sum(m.x[i] for i in m.x) == 0
+
+    @m.Constraint(m.IDX)
+    def con2(m, i):
+        return m.x[i] >= 20 + i
+
+    @m.Constraint(m.IDX)
+    def con3(m, i):
+        return m.x[i] >= 30 + i
+
+    return m
+
+
 class TestLPv2(unittest.TestCase):
     def test_warn_export_suffixes(self):
         m = pyo.ConcreteModel()
@@ -139,3 +167,181 @@ def test_deterministic_unordered_sets(self):
         self.assertEqual(LOG.getvalue(), "")
 
         self.assertEqual(ref, OUT.getvalue())
+
+    def test_SOS_ordering(self):
+        # This is in response to a bug that was identified with
+        # config.row_order with SOS constraints. If an SOS constraint
+        # was added and the `config.row_order` option selected,
+        # bug would ensue.
+
+        model = create_sos_model()
+
+        OUT = StringIO()
+        LPWriter().write(model, OUT, symbolic_solver_labels=True)
+        self.assertEqual(
+            r"""\* Source Pyomo model name=unknown *\
+
+min 
+ScalarObjective:
++1.0 ONE_VAR_CONSTANT
+
+s.t.
+
+c_e_con1_:
++1 x(1)
++1 x(2)
++1 x(3)
++1 x(4)
++1 x(5)
++1 x(6)
+= 0
+
+c_l_con2(1)_:
++1 x(1)
+>= 21
+
+c_l_con2(2)_:
++1 x(2)
+>= 22
+
+c_l_con3(1)_:
++1 x(1)
+>= 31
+
+c_l_con3(2)_:
++1 x(2)
+>= 32
+
+bounds
+   1 <= ONE_VAR_CONSTANT <= 1
+   -inf <= x(1) <= +inf
+   -inf <= x(2) <= +inf
+   -inf <= x(3) <= +inf
+   -inf <= x(4) <= +inf
+   -inf <= x(5) <= +inf
+   -inf <= x(6) <= +inf
+SOS
+
+sos1: S2::
+  x(1):1
+  x(2):2
+  x(3):3
+  x(4):4
+  x(5):5
+  x(6):6
+
+sos2(1): S1::
+  x(1):1
+  x(2):2
+  x(3):3
+
+sos2(2): S1::
+  x(4):1
+  x(5):2
+  x(6):3
+
+sos3(1): S2::
+  x(1):1
+  x(3):2
+  x(5):3
+
+sos3(2): S2::
+  x(2):1
+  x(4):2
+  x(6):3
+
+end
+""",
+            OUT.getvalue(),
+        )
+
+        OUT = StringIO()
+        LPWriter().write(
+            model,
+            OUT,
+            symbolic_solver_labels=True,
+            row_order=[
+                model.sos3[2],
+                model.con3[2],
+                model.con2,
+                model.sos2,
+                model.sos1,
+                model.con1,
+            ],
+        )
+        self.assertEqual(
+            r"""\* Source Pyomo model name=unknown *\
+
+min 
+ScalarObjective:
++1.0 ONE_VAR_CONSTANT
+
+s.t.
+
+c_l_con3(2)_:
++1 x(2)
+>= 32
+
+c_l_con2(1)_:
++1 x(1)
+>= 21
+
+c_l_con2(2)_:
++1 x(2)
+>= 22
+
+c_e_con1_:
++1 x(1)
++1 x(2)
++1 x(3)
++1 x(4)
++1 x(5)
++1 x(6)
+= 0
+
+c_l_con3(1)_:
++1 x(1)
+>= 31
+
+bounds
+   1 <= ONE_VAR_CONSTANT <= 1
+   -inf <= x(1) <= +inf
+   -inf <= x(2) <= +inf
+   -inf <= x(3) <= +inf
+   -inf <= x(4) <= +inf
+   -inf <= x(5) <= +inf
+   -inf <= x(6) <= +inf
+SOS
+
+sos3(2): S2::
+  x(2):1
+  x(4):2
+  x(6):3
+
+sos2(1): S1::
+  x(1):1
+  x(2):2
+  x(3):3
+
+sos2(2): S1::
+  x(4):1
+  x(5):2
+  x(6):3
+
+sos1: S2::
+  x(1):1
+  x(2):2
+  x(3):3
+  x(4):4
+  x(5):5
+  x(6):6
+
+sos3(1): S2::
+  x(1):1
+  x(3):2
+  x(5):3
+
+end
+""",
+            OUT.getvalue(),
+        )

pyomo/repn/util.py

@@ -748,17 +748,17 @@ def initialize_var_map_from_column_order(model, config, var_map):
     return var_map
 
 
-def ordered_active_constraints(model, config):
+def row_order2row_map(config):
+    """Convert a row_order (list or ComponentMap) into a dict mapping
+    constraint id -> row index. Returns an empty dict if no ordering."""
+    row_order = config.row_order
+    if row_order is None or isinstance(row_order, bool):
+        return {}
+
     sorter = FileDeterminism_to_SortComponents(config.file_determinism)
-    constraints = model.component_data_objects(Constraint, active=True, sort=sorter)
 
-    row_order = config.row_order
-    if row_order is None or row_order.__class__ is bool:
-        return constraints
-    elif isinstance(row_order, ComponentMap):
-        # The row order has historically also supported a ComponentMap of
-        # component to position in addition to the simple list of
-        # components.  Convert it to the simple list
+    if isinstance(row_order, ComponentMap):
+        # Convert ComponentMap to sorted list based on its values
         row_order = sorted(row_order, key=row_order.__getitem__)
 
     row_map = {}
@@ -768,10 +768,18 @@ def ordered_active_constraints(model, config):
                 row_map[id(c)] = c
         else:
             row_map[id(con)] = con
+
+    # Map the implicit dict ordering to an explicit 0..n ordering
+    return {_id: i for i, _id in enumerate(row_map)}
+
+
+def ordered_active_constraints(model, config):
+    sorter = FileDeterminism_to_SortComponents(config.file_determinism)
+    constraints = model.component_data_objects(Constraint, active=True, sort=sorter)
+
+    row_map = row_order2row_map(config)
     if not row_map:
         return constraints
-    # map the implicit dict ordering to an explicit 0..n ordering
-    row_map = {_id: i for i, _id in enumerate(row_map)}
     # sorted() is stable (per Python docs), so we can let all
     # unspecified rows have a row number one bigger than the
     # number of rows specified by the user ordering.