improve model_builder code

Author: lperron

ortools/linear_solver/python/model_builder.py

@@ -53,7 +53,9 @@
 _VariableOrConstraint = Union["LinearConstraint", mbh.Variable]
 
 # Forward solve statuses.
+AffineExpr = mbh.AffineExpr
 BoundedLinearExpression = mbh.BoundedLinearExpression
+FlatExpr = mbh.FlatExpr
 LinearExpr = mbh.LinearExpr
 SolveStatus = mbh.SolveStatus
 Variable = mbh.Variable
@@ -107,7 +109,7 @@ def _add_linear_constraint_to_helper(
         if name is not None:
             helper.set_constraint_name(c.index, name)
         return c
-    raise TypeError("invalid type={}".format(type(bounded_expr)))
+    raise TypeError(f"invalid type={type(bounded_expr).__name__!r}")
 
 
 def _add_enforced_linear_constraint_to_helper(
@@ -171,7 +173,7 @@ def _add_enforced_linear_constraint_to_helper(
             helper.set_constraint_name(c.index, name)
         return c
 
-    raise TypeError("invalid type={}".format(type(bounded_expr)))
+    raise TypeError(f"invalid type={type(bounded_expr).__name__!r}")
 
 
 class LinearConstraint:
@@ -631,8 +633,10 @@ def new_var(
         Returns:
           a variable whose domain is [lb, ub].
         """
-
-        return Variable(self.__helper, lb, ub, is_integer, name)
+        if name:
+            return Variable(self.__helper, lb, ub, is_integer, name)
+        else:
+            return Variable(self.__helper, lb, ub, is_integer)
 
     def new_int_var(
         self, lb: NumberT, ub: NumberT, name: Optional[str] = None
@@ -712,16 +716,15 @@ def new_var_series(
         if not isinstance(index, pd.Index):
             raise TypeError("Non-index object is used as index")
         if not name.isidentifier():
-            raise ValueError("name={} is not a valid identifier".format(name))
+            raise ValueError(f"name={name!r} is not a valid identifier")
         if (
             mbn.is_a_number(lower_bounds)
             and mbn.is_a_number(upper_bounds)
             and lower_bounds > upper_bounds
         ):
             raise ValueError(
-                "lower_bound={} is greater than upper_bound={} for variable set={}".format(
-                    lower_bounds, upper_bounds, name
-                )
+                f"lower_bound={lower_bounds} is greater than"
+                f" upper_bound={upper_bounds} for variable set={name!r}"
             )
         if (
             isinstance(is_integral, bool)
@@ -733,10 +736,9 @@ def new_var_series(
             and math.ceil(lower_bounds) > math.floor(upper_bounds)
         ):
             raise ValueError(
-                "ceil(lower_bound={})={}".format(lower_bounds, math.ceil(lower_bounds))
-                + " is greater than floor("
-                + "upper_bound={})={}".format(upper_bounds, math.floor(upper_bounds))
-                + " for variable set={}".format(name)
+                f"ceil(lower_bound={lower_bounds})={math.ceil(lower_bounds)}"
+                f" is greater than floor({upper_bounds}) = {math.floor(upper_bounds)}"
+                f" for variable set={name!r}"
             )
         lower_bounds = _convert_to_series_and_validate_index(lower_bounds, index)
         upper_bounds = _convert_to_series_and_validate_index(upper_bounds, index)
@@ -871,8 +873,8 @@ def add_linear_constraint(  # pytype: disable=annotation-type-mismatch  # numpy-
             )
         else:
             raise TypeError(
-                f"Not supported: Model.add_linear_constraint({linear_expr})"
-                f" with type {type(linear_expr)}"
+                "Not supported:"
+                f" Model.add_linear_constraint({type(linear_expr).__name__!r})"
             )
         return ct
 
@@ -917,7 +919,7 @@ def add(
                 ],
             )
         else:
-            raise TypeError("Not supported: Model.add(" + str(ct) + ")")
+            raise TypeError(f"Not supported: Model.add({type(ct).__name__!r})")
 
     def linear_constraint_from_index(self, index: IntegerT) -> LinearConstraint:
         """Rebuilds a linear constraint object from the model and its index."""
@@ -954,8 +956,7 @@ def add_enforced_linear_constraint(  # pytype: disable=annotation-type-mismatch
         else:
             raise TypeError(
                 "Not supported:"
-                f" Model.add_enforced_linear_constraint({linear_expr}) with"
-                f" type {type(linear_expr)}"
+                f" Model.add_enforced_linear_constraint({type(linear_expr).__name__!r})"
             )
         return ct
 
@@ -1018,7 +1019,7 @@ def add_enforced(
                 ],
             )
         else:
-            raise TypeError("Not supported: Model.add_enforced(" + str(ct) + ")")
+            raise TypeError(f"Not supported: Model.add_enforced({type(ct).__name__!r}")
 
     def enforced_linear_constraint_from_index(
         self, index: IntegerT
@@ -1050,7 +1051,10 @@ def __optimize(self, linear_expr: LinearExprT, maximize: bool) -> None:
             var_indices = [var.index for var in flat_expr.vars]
             self.helper.set_objective_coefficients(var_indices, flat_expr.coeffs)
         else:
-            raise TypeError(f"Not supported: Model.minimize/maximize({linear_expr})")
+            raise TypeError(
+                "Not supported:"
+                f" Model.minimize/maximize({type(linear_expr).__name__!r})"
+            )
 
     @property
     def objective_offset(self) -> np.double:
@@ -1233,7 +1237,7 @@ def value(self, expr: LinearExprT) -> np.double:
         elif isinstance(expr, LinearExpr):
             return self.__solve_helper.expression_value(expr)
         else:
-            raise TypeError(f"Unknown expression {expr!r} of type {type(expr)}")
+            raise TypeError(f"Unknown expression {type(expr).__name__!r}")
 
     def values(self, variables: _IndexOrSeries) -> pd.Series:
         """Returns the values of the input variables.
@@ -1401,7 +1405,7 @@ def _convert_to_series_and_validate_index(
         else:
             raise ValueError("index does not match")
     else:
-        raise TypeError("invalid type={}".format(type(value_or_series)))
+        raise TypeError("invalid type={type(value_or_series).__name!r}")
     return result
 
 
@@ -1429,7 +1433,7 @@ def _convert_to_var_series_and_validate_index(
         else:
             raise ValueError("index does not match")
     else:
-        raise TypeError("invalid type={}".format(type(var_or_series)))
+        raise TypeError("invalid type={type(value_or_series).__name!r}")
     return result
 
 

ortools/linear_solver/python/model_builder_test.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import math
+import sys
 from typing import Any, Callable, Dict, Mapping, Union
 
 from absl.testing import absltest
@@ -32,9 +33,7 @@
 def build_dict(expr: mb.LinearExprT) -> Dict[mbh.Variable, float]:
     res = {}
     flat_expr = mbh.FlatExpr(expr)
-    print(f"expr = {expr} flat_expr = {flat_expr}", flush=True)
     for var, coeff in zip(flat_expr.vars, flat_expr.coeffs):
-        print(f"process {var} * {coeff}", flush=True)
         if not coeff:
             continue
         res[var] = coeff
@@ -46,6 +45,10 @@ class ModelBuilderTest(absltest.TestCase):
     # checking primal, dual, objective values and other values.
     NUM_PLACES = 5
 
+    def tearDown(self) -> None:
+        super().tearDown()
+        sys.stdout.flush()
+
     # pylint: disable=too-many-statements
     def run_minimal_linear_example(self, solver_name):
         """Minimal Linear Example."""
@@ -318,6 +321,49 @@ def test_class_api(self):
         self.assertEqual(flat_e14.offset, 0.8)
         self.assertEqual(e14.__str__(), "(x - t + 0.8)")
 
+        e15 = mb.LinearExpr.weighted_sum([1, x, 1], [1, 1, -1])
+        self.assertIsInstance(e15, mb.Variable)
+        self.assertEqual(x.index, e15.index)
+
+        e16 = mb.LinearExpr.affine(x, 1.0, 0.0)
+        self.assertIsInstance(e16, mb.Variable)
+        self.assertEqual(x.index, e16.index)
+
+        e17 = -x
+        self.assertIsInstance(e17, mb.AffineExpr)
+        self.assertEqual(str(e17), "(-x)")
+
+        e18 = mb.LinearExpr.affine(x, 1.0, -2.0)
+        self.assertIsInstance(e18, mb.AffineExpr)
+        self.assertEqual(str(e18), "(x - 2)")
+
+        e19 = mb.LinearExpr.weighted_sum([1, x, 1], [1, 1, -2])
+        self.assertIsInstance(e19, mb.AffineExpr)
+        self.assertEqual(str(e19), "(x - 1)")
+
+        e20 = mb.LinearExpr.affine(x, -2.0, 0.0)
+        self.assertIsInstance(e20, mb.AffineExpr)
+        self.assertEqual(str(e20), "(-2 * x)")
+
+        e21 = mb.LinearExpr.weighted_sum([1, x, 1], [1, 2, -1])
+        self.assertIsInstance(e21, mb.AffineExpr)
+        self.assertEqual(str(e21), "(2 * x)")
+
+        c1 = x == 2
+        self.assertEqual(str(c1), "x == 2")
+
+        c2 = -x == 3
+        self.assertEqual(str(c2), "-x == 3")
+
+        c3 = x + y == 3
+        self.assertEqual(str(c3), "(x + y) == 3")
+
+        c4 = -x + y == 3
+        self.assertEqual(str(c4), "(-x + y) == 3")
+
+        c5 = x - y == 3
+        self.assertEqual(str(c5), "(x - y) == 3")
+
     def test_variables(self):
         model = mb.Model()
         x = model.new_int_var(0.0, 4.0, "x")
@@ -330,6 +376,10 @@ def test_variables(self):
         self.assertEqual(1.0, x.lower_bound)
         self.assertEqual(3.0, x.upper_bound)
         self.assertTrue(x.is_integral)
+        n1 = model.new_int_var(0, 4)
+        self.assertEqual(n1.name, "variable#1")
+        n2 = model.new_int_var(0, 4, None)
+        self.assertEqual(n2.name, "variable#2")
 
         # Tests the equality operator.
         y = model.new_int_var(0.0, 4.0, "y")
@@ -449,6 +499,10 @@ def test_create_true_ct(self):
 
 class InternalHelperTest(absltest.TestCase):
 
+    def tearDown(self) -> None:
+        super().tearDown()
+        sys.stdout.flush()
+
     def test_anonymous_variables(self):
         helper = mb.Model().helper
         index = helper.add_var()
@@ -641,6 +695,10 @@ def test_str(self, expr, expected_str):
 
 class LinearBaseErrorsTest(absltest.TestCase):
 
+    def tearDown(self) -> None:
+        super().tearDown()
+        sys.stdout.flush()
+
     def test_unknown_linear_type(self):
         with self.assertRaises(TypeError):
 
@@ -758,6 +816,10 @@ def test_var_eq_var_as_bool(self):
 
 class BoundedLinearBaseErrorsTest(absltest.TestCase):
 
+    def tearDown(self) -> None:
+        super().tearDown()
+        sys.stdout.flush()
+
     def test_single_var_bounded_linear_expression_as_bool(self):
         with self.assertRaisesRegex(
             NotImplementedError, "Evaluating a BoundedLinearExpression"
@@ -775,6 +837,10 @@ def test_bounded_linear_expression_as_bool(self):
 
 class ModelBuilderErrorsTest(absltest.TestCase):
 
+    def tearDown(self) -> None:
+        super().tearDown()
+        sys.stdout.flush()
+
     def test_new_var_series_errors(self):
         with self.assertRaisesRegex(TypeError, r"Non-index object"):
             model = mb.Model()
@@ -1607,6 +1673,10 @@ def test_maximize(
 
 class ModelBuilderProtoTest(absltest.TestCase):
 
+    def tearDown(self) -> None:
+        super().tearDown()
+        sys.stdout.flush()
+
     def test_export_to_proto(self):
         expected = linear_solver_pb2.MPModelProto()
         text_format.Parse(
@@ -1970,6 +2040,11 @@ def test_get_objective_value(
 
 
 class ModelBuilderExamplesTest(absltest.TestCase):
+
+    def tearDown(self) -> None:
+        super().tearDown()
+        sys.stdout.flush()
+
     def test_simple_problem(self):
         # max 5x1 + 4x2 + 3x3
         # s.t 2x1 + 3x2 +  x3 <= 5

ortools/linear_solver/wrappers/model_builder_helper.cc

@@ -866,7 +866,6 @@ double ExprFlattener::Flatten(std::vector<const Variable*>* vars,
 }
 
 void ExprEvaluator::AddVarCoeff(const Variable* var, double coeff) {
-  if (coeff == 0.0) return;
   offset_ += coeff * helper_->variable_value(var->index());
 }