From 1571e79439734fa2585ca1f6d7e83b7401184bb2 Mon Sep 17 00:00:00 2001 From: Laurent Perron Date: Mon, 13 Jan 2025 14:17:07 +0100 Subject: [PATCH] more unit testing --- ortools/linear_solver/python/model_builder.py | 10 +-- .../python/model_builder_helper.cc | 16 ++-- .../python/model_builder_test.py | 81 ++++++++++++------- .../wrappers/model_builder_helper.cc | 34 ++++---- .../wrappers/model_builder_helper.h | 14 ++-- 5 files changed, 88 insertions(+), 67 deletions(-) diff --git a/ortools/linear_solver/python/model_builder.py b/ortools/linear_solver/python/model_builder.py index 59ebed151f..ee67ca2c17 100644 --- a/ortools/linear_solver/python/model_builder.py +++ b/ortools/linear_solver/python/model_builder.py @@ -510,7 +510,7 @@ class Model: """ return _attribute_series( # pylint: disable=g-long-lambda - func=lambda c: mbh.FlatExpression( + func=lambda c: mbh.FlatExpr( # pylint: disable=g-complex-comprehension [ Variable(self.__helper, var_id) @@ -862,7 +862,7 @@ class Model: self.__helper.set_constraint_lower_bound(ct.index, lb - linear_expr) self.__helper.set_constraint_upper_bound(ct.index, ub - linear_expr) elif isinstance(linear_expr, LinearExpr): - flat_expr = mbh.FlatExpression(linear_expr) + flat_expr = mbh.FlatExpr(linear_expr) # pylint: disable=protected-access self.__helper.set_constraint_lower_bound(ct.index, lb - flat_expr.offset) self.__helper.set_constraint_upper_bound(ct.index, ub - flat_expr.offset) @@ -944,7 +944,7 @@ class Model: self.__helper.set_constraint_lower_bound(ct.index, lb - linear_expr) self.__helper.set_constraint_upper_bound(ct.index, ub - linear_expr) elif isinstance(linear_expr, LinearExpr): - flat_expr = mbh.FlatExpression(linear_expr) + flat_expr = mbh.FlatExpr(linear_expr) # pylint: disable=protected-access self.__helper.set_constraint_lower_bound(ct.index, lb - flat_expr.offset) self.__helper.set_constraint_upper_bound(ct.index, ub - flat_expr.offset) @@ -1044,7 +1044,7 @@ class Model: elif isinstance(linear_expr, Variable): self.helper.set_var_objective_coefficient(linear_expr.index, 1.0) elif isinstance(linear_expr, LinearExpr): - flat_expr = mbh.FlatExpression(linear_expr) + flat_expr = mbh.FlatExpr(linear_expr) # pylint: disable=protected-access self.helper.set_objective_offset(flat_expr.offset) var_indices = [var.index for var in flat_expr.vars] @@ -1070,7 +1070,7 @@ class Model: if coeff != 0.0: variables.append(variable) coefficients.append(coeff) - return mbh.FlatExpression( + return mbh.FlatExpr( variables, coefficients, self.__helper.objective_offset() ) diff --git a/ortools/linear_solver/python/model_builder_helper.cc b/ortools/linear_solver/python/model_builder_helper.cc index 025c4dad3a..0d65d87266 100644 --- a/ortools/linear_solver/python/model_builder_helper.cc +++ b/ortools/linear_solver/python/model_builder_helper.cc @@ -53,7 +53,7 @@ using ::operations_research::MPVariableProto; using ::operations_research::mb::AffineExpr; using ::operations_research::mb::BoundedLinearExpression; using ::operations_research::mb::FixedValue; -using ::operations_research::mb::FlatExpression; +using ::operations_research::mb::FlatExpr; using ::operations_research::mb::LinearExpr; using ::operations_research::mb::ModelBuilderHelper; using ::operations_research::mb::ModelSolverHelper; @@ -232,9 +232,7 @@ LinearExpr* SumArguments(py::args args, const py::kwargs& kwargs) { } }; - if (args.size() == 0) { - return new FixedValue(0.0); - } else if (args.size() == 1 && py::isinstance(args[0])) { + if (args.size() == 1 && py::isinstance(args[0])) { // Normal list or tuple argument. py::sequence elements = args[0].cast(); linear_exprs.reserve(elements.size()); @@ -463,17 +461,17 @@ PYBIND11_MODULE(model_builder_helper, m) { }); // Expose Internal classes, mostly for testing. - py::class_(m, "FlatExpression") + py::class_(m, "FlatExpr") .def(py::init()) .def(py::init()) .def(py::init&, const std::vector&, double>(), py::keep_alive<1, 2>()) .def(py::init()) - .def_property_readonly("vars", &FlatExpression::vars) - .def("variable_indices", &FlatExpression::VarIndices) - .def_property_readonly("coeffs", &FlatExpression::coeffs) - .def_property_readonly("offset", &FlatExpression::offset); + .def_property_readonly("vars", &FlatExpr::vars) + .def("variable_indices", &FlatExpr::VarIndices) + .def_property_readonly("coeffs", &FlatExpr::coeffs) + .def_property_readonly("offset", &FlatExpr::offset); py::class_(m, "AffineExpr") .def(py::init()) diff --git a/ortools/linear_solver/python/model_builder_test.py b/ortools/linear_solver/python/model_builder_test.py index 6aa4d37a1e..ee6e74f2b3 100644 --- a/ortools/linear_solver/python/model_builder_test.py +++ b/ortools/linear_solver/python/model_builder_test.py @@ -31,7 +31,7 @@ from ortools.linear_solver.python import model_builder_helper as mbh def build_dict(expr: mb.LinearExprT) -> Dict[mbh.Variable, float]: res = {} - flat_expr = mbh.FlatExpression(expr) + 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) @@ -209,7 +209,7 @@ ENDATA t = model.new_int_var(3, 10, "t") e1 = mb.LinearExpr.sum([x, y, z]) - flat_e1 = mbh.FlatExpression(e1) + flat_e1 = mbh.FlatExpr(e1) expected_vars = np.array([0, 1, 2], dtype=np.int32) np_testing.assert_array_equal(expected_vars, flat_e1.variable_indices()) np_testing.assert_array_equal( @@ -219,7 +219,7 @@ ENDATA self.assertEqual(e1.__str__(), "(x + y + z)") e2 = mb.LinearExpr.sum([e1, 4.0]) - flat_e2 = mbh.FlatExpression(e2) + flat_e2 = mbh.FlatExpr(e2) np_testing.assert_array_equal(expected_vars, flat_e2.variable_indices()) np_testing.assert_array_equal( np.array([1, 1, 1], dtype=np.double), flat_e2.coeffs @@ -229,7 +229,7 @@ ENDATA self.assertEqual(flat_e2.__str__(), "x + y + z + 4") e3 = mb.LinearExpr.term(e2, 2) - flat_e3 = mbh.FlatExpression(e3) + flat_e3 = mbh.FlatExpr(e3) np_testing.assert_array_equal(expected_vars, flat_e3.variable_indices()) np_testing.assert_array_equal( np.array([2, 2, 2], dtype=np.double), flat_e3.coeffs @@ -239,7 +239,7 @@ ENDATA self.assertEqual(flat_e3.__str__(), "2 * x + 2 * y + 2 * z + 8") e4 = mb.LinearExpr.weighted_sum([x, t], [-1, 1], constant=2) - flat_e4 = mbh.FlatExpression(e4) + flat_e4 = mbh.FlatExpr(e4) np_testing.assert_array_equal( np.array([0, 3], dtype=np.int32), flat_e4.variable_indices() ) @@ -250,7 +250,7 @@ ENDATA self.assertEqual(e4.__str__(), "(-x + t + 2)") e4b = mb.LinearExpr.weighted_sum([e4 * 3], [1]) - flat_e4b = mbh.FlatExpression(e4b) + flat_e4b = mbh.FlatExpr(e4b) np_testing.assert_array_equal( np.array([0, 3], dtype=np.int32), flat_e4b.variable_indices() ) @@ -261,7 +261,7 @@ ENDATA self.assertEqual(e4b.__str__(), "(3 * (-x + t + 2))") e5 = mb.LinearExpr.sum([e1, -3, e4]) - flat_e5 = mbh.FlatExpression(e5) + flat_e5 = mbh.FlatExpr(e5) np_testing.assert_array_equal( np.array([1, 2, 3], dtype=np.int32), flat_e5.variable_indices() ) @@ -272,7 +272,7 @@ ENDATA self.assertEqual(flat_e5.__str__(), "y + z + t - 1") e6 = mb.LinearExpr.term(x, 2.0, constant=1.0) - flat_e6 = mbh.FlatExpression(e6) + flat_e6 = mbh.FlatExpr(e6) np_testing.assert_array_equal( np.array([0], dtype=np.int32), flat_e6.variable_indices() ) @@ -288,10 +288,36 @@ ENDATA e9 = mb.LinearExpr.term(x * 2 + 3, 1, constant=0) e10 = mb.LinearExpr.term(x, 2, constant=3) self.assertEqual( - str(mbh.FlatExpression(e9)), - str(mbh.FlatExpression(e10)), + str(mbh.FlatExpr(e9)), + str(mbh.FlatExpr(e10)), ) + e10 = mb.LinearExpr.sum() + self.assertEqual(str(e10), "0") + + e11 = mb.LinearExpr.sum(x) + self.assertIsInstance(e11, mb.Variable) + self.assertEqual(x.index, e11.index) + + e12 = mb.LinearExpr.sum(-1.0, x, 1.0) + self.assertIsInstance(e12, mb.Variable) + self.assertEqual(x.index, e12.index) + + e13 = mb.LinearExpr.sum(-1.0, x, constant=1.0) + self.assertIsInstance(e13, mb.Variable) + self.assertEqual(x.index, e13.index) + + e14 = mb.LinearExpr.weighted_sum([x, t, 1.2], [1, -1, -1.0], constant=2) + flat_e14 = mbh.FlatExpr(e14) + np_testing.assert_array_equal( + np.array([0, 3], dtype=np.int32), flat_e14.variable_indices() + ) + np_testing.assert_array_equal( + np.array([1, -1], dtype=np.double), flat_e14.coeffs + ) + self.assertEqual(flat_e14.offset, 0.8) + self.assertEqual(e14.__str__(), "(x - t + 0.8)") + def test_variables(self): model = mb.Model() x = model.new_int_var(0.0, 4.0, "x") @@ -580,27 +606,27 @@ class LinearBaseTest(parameterized.TestCase): ), # LinearExpression dict( - testcase_name="FlatExpression(x.sum())", - expr=lambda x, y: mbh.FlatExpression(x.sum()), + testcase_name="FlatExpr(x.sum())", + expr=lambda x, y: mbh.FlatExpr(x.sum()), expected_str="x[0] + x[1] + x[2]", ), dict( - testcase_name="FlatExpression(FlatExpression(x.sum()))", + testcase_name="FlatExpr(FlatExpr(x.sum()))", # pylint: disable=g-long-lambda - expr=lambda x, y: mbh.FlatExpression(mbh.FlatExpression(x.sum())), + expr=lambda x, y: mbh.FlatExpr(mbh.FlatExpr(x.sum())), expected_str="x[0] + x[1] + x[2]", ), dict( - testcase_name="""FlatExpression(sum([ - FlatExpression(x.sum()), - FlatExpression(x.sum()), + testcase_name="""FlatExpr(sum([ + FlatExpr(x.sum()), + FlatExpr(x.sum()), ]))""", # pylint: disable=g-long-lambda - expr=lambda x, y: mbh.FlatExpression( + expr=lambda x, y: mbh.FlatExpr( sum( [ - mbh.FlatExpression(x.sum()), - mbh.FlatExpression(x.sum()), + mbh.FlatExpr(x.sum()), + mbh.FlatExpr(x.sum()), ] ) ), @@ -610,7 +636,7 @@ class LinearBaseTest(parameterized.TestCase): def test_str(self, expr, expected_str): x = self.x y = self.y - self.assertEqual(str(mbh.FlatExpression(expr(x, y))), expected_str) + self.assertEqual(str(mbh.FlatExpr(expr(x, y))), expected_str) class LinearBaseErrorsTest(absltest.TestCase): @@ -623,7 +649,7 @@ class LinearBaseErrorsTest(absltest.TestCase): def __init__(self): mb.LinearExpr.__init__(self) - mbh.FlatExpression(UnknownLinearType()) + mbh.FlatExpr(UnknownLinearType()) def test_division_by_zero(self): with self.assertRaises(ZeroDivisionError): @@ -1483,8 +1509,8 @@ class ModelBuilderObjectiveTest(parameterized.TestCase): expr2: mbh.LinearExpr, ) -> None: """Test that the two linear expressions are almost equal.""" - flat_expr1 = mbh.FlatExpression(expr1) - flat_expr2 = mbh.FlatExpression(expr2) + flat_expr1 = mbh.FlatExpr(expr1) + flat_expr2 = mbh.FlatExpr(expr2) self.assertEqual(len(flat_expr1.vars), len(flat_expr2.vars)) if len(flat_expr1.vars) > 0: # pylint: disable=g-explicit-length-test self.assertSequenceEqual(flat_expr1.vars, flat_expr2.vars) @@ -1511,7 +1537,6 @@ class ModelBuilderObjectiveTest(parameterized.TestCase): x = model.new_var_series(name="x", index=variable_indices) y = model.new_var_series(name="y", index=variable_indices) objective_expression = expression(x, y) - print(f"objective_expression: {objective_expression}") if is_maximize: model.maximize(objective_expression) else: @@ -1529,7 +1554,7 @@ class ModelBuilderObjectiveTest(parameterized.TestCase): # Set and check for old objective. model.maximize(old_objective_expression) - flat_got_objective_expression = mbh.FlatExpression(model.objective_expression()) + flat_got_objective_expression = mbh.FlatExpr(model.objective_expression()) self.assertEmpty(flat_got_objective_expression.vars) self.assertEmpty(flat_got_objective_expression.coeffs) self.assertAlmostEqual(flat_got_objective_expression.offset, 1) @@ -1553,7 +1578,7 @@ class ModelBuilderObjectiveTest(parameterized.TestCase): model = mb.Model() x = model.new_var_series(name="x", index=variable_indices) y = model.new_var_series(name="y", index=variable_indices) - objective_expression = mbh.FlatExpression(expression(x, y)) + objective_expression = mbh.FlatExpr(expression(x, y)) model.minimize(objective_expression) got_objective_expression = model.objective_expression() self.assertLinearExpressionAlmostEqual( @@ -1572,7 +1597,7 @@ class ModelBuilderObjectiveTest(parameterized.TestCase): model = mb.Model() x = model.new_var_series(name="x", index=variable_indices) y = model.new_var_series(name="y", index=variable_indices) - objective_expression = mbh.FlatExpression(expression(x, y)) + objective_expression = mbh.FlatExpr(expression(x, y)) model.maximize(objective_expression) got_objective_expression = model.objective_expression() self.assertLinearExpressionAlmostEqual( diff --git a/ortools/linear_solver/wrappers/model_builder_helper.cc b/ortools/linear_solver/wrappers/model_builder_helper.cc index 0d1d6d1a22..1d26c09942 100644 --- a/ortools/linear_solver/wrappers/model_builder_helper.cc +++ b/ortools/linear_solver/wrappers/model_builder_helper.cc @@ -880,26 +880,26 @@ double ExprEvaluator::Evaluate() { return offset_; } -FlatExpression::FlatExpression(const LinearExpr* expr) { +FlatExpr::FlatExpr(const LinearExpr* expr) { ExprFlattener lin; lin.AddToProcess(expr, 1.0); offset_ = lin.Flatten(&vars_, &coeffs_); } -FlatExpression::FlatExpression(const LinearExpr* pos, const LinearExpr* neg) { +FlatExpr::FlatExpr(const LinearExpr* pos, const LinearExpr* neg) { ExprFlattener lin; lin.AddToProcess(pos, 1.0); lin.AddToProcess(neg, -1.0); offset_ = lin.Flatten(&vars_, &coeffs_); } -FlatExpression::FlatExpression(const std::vector& vars, - const std::vector& coeffs, double offset) +FlatExpr::FlatExpr(const std::vector& vars, + const std::vector& coeffs, double offset) : vars_(vars), coeffs_(coeffs), offset_(offset) {} -FlatExpression::FlatExpression(double offset) : offset_(offset) {} +FlatExpr::FlatExpr(double offset) : offset_(offset) {} -std::vector FlatExpression::VarIndices() const { +std::vector FlatExpr::VarIndices() const { std::vector var_indices; var_indices.reserve(vars_.size()); for (const Variable* var : vars_) { @@ -908,23 +908,21 @@ std::vector FlatExpression::VarIndices() const { return var_indices; } -void FlatExpression::Visit(ExprVisitor& lin, double c) const { +void FlatExpr::Visit(ExprVisitor& lin, double c) const { for (int i = 0; i < vars_.size(); ++i) { lin.AddVarCoeff(vars_[i], coeffs_[i] * c); } - if (offset_ != 0.0) { - lin.AddConstant(offset_ * c); - } + lin.AddConstant(offset_ * c); } -std::string FlatExpression::ToString() const { +std::string FlatExpr::ToString() const { if (vars_.empty()) { return absl::StrCat(offset_); } std::string s; int num_printed = 0; for (int i = 0; i < vars_.size(); ++i) { - if (coeffs_[i] == 0.0) continue; + DCHECK_NE(coeffs_[i], 0.0); ++num_printed; if (num_printed > 5) { absl::StrAppend(&s, " + ..."); @@ -966,9 +964,9 @@ std::string FlatExpression::ToString() const { return s; } -std::string FlatExpression::DebugString() const { +std::string FlatExpr::DebugString() const { std::string s = absl::StrCat( - "FlatExpression(", + "FlatExpr(", absl::StrJoin(vars_, ", ", [](std::string* out, const Variable* expr) { absl::StrAppend(out, expr->DebugString()); })); @@ -1217,7 +1215,7 @@ void Variable::SetObjectiveCoefficient(double coeff) { BoundedLinearExpression::BoundedLinearExpression(const LinearExpr* expr, double lower_bound, double upper_bound) { - FlatExpression flat_expr(expr); + FlatExpr flat_expr(expr); vars_ = flat_expr.vars(); coeffs_ = flat_expr.coeffs(); lower_bound_ = lower_bound - flat_expr.offset(); @@ -1228,7 +1226,7 @@ BoundedLinearExpression::BoundedLinearExpression(const LinearExpr* pos, const LinearExpr* neg, double lower_bound, double upper_bound) { - FlatExpression flat_expr(pos, neg); + FlatExpr flat_expr(pos, neg); vars_ = flat_expr.vars(); coeffs_ = flat_expr.coeffs(); lower_bound_ = lower_bound - flat_expr.offset(); @@ -1238,7 +1236,7 @@ BoundedLinearExpression::BoundedLinearExpression(const LinearExpr* pos, BoundedLinearExpression::BoundedLinearExpression(const LinearExpr* expr, int64_t lower_bound, int64_t upper_bound) { - FlatExpression flat_expr(expr); + FlatExpr flat_expr(expr); vars_ = flat_expr.vars(); coeffs_ = flat_expr.coeffs(); lower_bound_ = lower_bound - flat_expr.offset(); @@ -1249,7 +1247,7 @@ BoundedLinearExpression::BoundedLinearExpression(const LinearExpr* pos, const LinearExpr* neg, int64_t lower_bound, int64_t upper_bound) { - FlatExpression flat_expr(pos, neg); + FlatExpr flat_expr(pos, neg); vars_ = flat_expr.vars(); coeffs_ = flat_expr.coeffs(); lower_bound_ = lower_bound - flat_expr.offset(); diff --git a/ortools/linear_solver/wrappers/model_builder_helper.h b/ortools/linear_solver/wrappers/model_builder_helper.h index 0b5e7003c9..5b42e97fb8 100644 --- a/ortools/linear_solver/wrappers/model_builder_helper.h +++ b/ortools/linear_solver/wrappers/model_builder_helper.h @@ -36,7 +36,7 @@ namespace mb { // Base implementation of linear expressions. class BoundedLinearExpression; -class FlatExpression; +class FlatExpr; class ExprVisitor; class LinearExpr; class ModelBuilderHelper; @@ -114,14 +114,14 @@ class ExprEvaluator : public ExprVisitor { }; // A flat linear expression sum(vars[i] * coeffs[i]) + offset -class FlatExpression : public LinearExpr { +class FlatExpr : public LinearExpr { public: - explicit FlatExpression(const LinearExpr* expr); + explicit FlatExpr(const LinearExpr* expr); // Flatten pos - neg. - FlatExpression(const LinearExpr* pos, const LinearExpr* neg); - FlatExpression(const std::vector&, - const std::vector&, double); - explicit FlatExpression(double offset); + FlatExpr(const LinearExpr* pos, const LinearExpr* neg); + FlatExpr(const std::vector&, const std::vector&, + double); + explicit FlatExpr(double offset); const std::vector& vars() const { return vars_; } std::vector VarIndices() const; const std::vector& coeffs() const { return coeffs_; }