[CP-SAT] remove multiple inheritance in the python layer as this slow…

…s downs the code a lot; improve test coverage

ortools/sat/python/cp_model_test.py

@@ -173,6 +173,28 @@ def testCreateIntegerVariable(self) -> None:
         cst = model.new_constant(5)
         self.assertEqual("5", str(cst))
 
+    def testLiteral(self) -> None:
+        print("testLiteral")
+        model = cp_model.CpModel()
+        x = model.new_bool_var("x")
+        self.assertEqual("x", str(x))
+        self.assertEqual("not(x)", str(~x))
+        self.assertEqual("not(x)", str(x.negated()))
+        self.assertEqual(x.negated().negated(), x)
+        self.assertEqual(x.negated().negated().index, x.index)
+        y = model.new_int_var(0, 1, "y")
+        self.assertEqual("y", str(y))
+        self.assertEqual("not(y)", str(~y))
+        zero = model.new_constant(0)
+        self.assertEqual("0", str(zero))
+        self.assertEqual("not(0)", str(~zero))
+        one = model.new_constant(1)
+        self.assertEqual("1", str(one))
+        self.assertEqual("not(1)", str(~one))
+        z = model.new_int_var(0, 2, "z")
+        self.assertRaises(TypeError, z.negated)
+        self.assertRaises(TypeError, z.__invert__)
+
     def testNegation(self) -> None:
         print("testNegation")
         model = cp_model.CpModel()
@@ -519,6 +541,13 @@ def testSumWithApi(self) -> None:
         print("testSumWithApi")
         model = cp_model.CpModel()
         x = [model.new_int_var(0, 2, "x%i" % i) for i in range(100)]
+        self.assertEqual(cp_model.LinearExpr.sum([x[0]]), x[0])
+        self.assertEqual(cp_model.LinearExpr.sum([x[0], 0]), x[0])
+        self.assertEqual(cp_model.LinearExpr.sum([x[0], 0.0]), x[0])
+        self.assertEqual(
+            repr(cp_model.LinearExpr.sum([x[0], 2])),
+            repr(cp_model.LinearExpr.affine(x[0], 1, 2)),
+        )
         model.add(cp_model.LinearExpr.sum(x) <= 1)
         model.maximize(x[99])
         solver = cp_model.CpSolver()
@@ -1599,12 +1628,18 @@ def testIntVarSeries(self) -> None:
         self.assertEqual(cp_model.OPTIMAL, solver.solve(model))
         solution = solver.values(x)
         self.assertTrue((solution.values == [0, 5, 0]).all())
+        self.assertRaises(TypeError, x.apply, lambda x: ~x)
 
     def testBoolVarSeries(self) -> None:
         print("testBoolVarSeries")
         df = pd.DataFrame([1, -1, 1], columns=["coeffs"])
         model = cp_model.CpModel()
         x = model.new_bool_var_series(name="x", index=df.index)
+        _ = x.apply(lambda x: ~x)
+        y = model.new_int_var_series(
+            name="y", index=df.index, lower_bounds=0, upper_bounds=1
+        )
+        _ = y.apply(lambda x: ~x)
         model.minimize(df.coeffs.dot(x))
         solver = cp_model.CpSolver()
         self.assertEqual(cp_model.OPTIMAL, solver.solve(model))

ortools/sat/python/linear_expr.h

@@ -431,15 +431,15 @@ class IntConstant : public LinearExpr {
 };
 
 // A Boolean literal (a Boolean variable or its negation).
-class Literal {
+class Literal : public LinearExpr {
  public:
-  virtual ~Literal() = default;
+  ~Literal() override = default;
   virtual int index() const = 0;
   virtual Literal* negated() const = 0;
 };
 
 // A class to hold a variable index.
-class BaseIntVar : public LinearExpr, public Literal {
+class BaseIntVar : public Literal {
  public:
   explicit BaseIntVar(int index) : index_(index), negated_(nullptr) {
     DCHECK_GE(index, 0);
@@ -493,7 +493,7 @@ H AbslHashValue(H h, const BaseIntVar* i) {
 }
 
 // A class to hold a negated variable index.
-class NotBooleanVariable : public LinearExpr, public Literal {
+class NotBooleanVariable : public Literal {
  public:
   explicit NotBooleanVariable(BaseIntVar* var) : var_(var) {}
   ~NotBooleanVariable() override = default;

ortools/sat/python/swig_helper.cc

@@ -481,7 +481,7 @@ PYBIND11_MODULE(swig_helper, m) {
           [](LinearExpr* lhs, int64_t rhs) {
             if (rhs == std::numeric_limits<int64_t>::max() ||
                 rhs == std::numeric_limits<int64_t>::min()) {
-              throw_error(PyExc_ValueError,
+              throw_error(PyExc_ArithmeticError,
                           "== INT_MIN or INT_MAX is not supported");
             }
             return CheckBoundedLinearExpression(lhs->EqCst(rhs), lhs);
@@ -660,7 +660,7 @@ PYBIND11_MODULE(swig_helper, m) {
       .def_property_readonly("coefficient", &IntAffine::coefficient)
       .def_property_readonly("offset", &IntAffine::offset);
 
-  py::class_<Literal>(m, "Literal", kLiteralClassDoc)
+  py::class_<Literal, LinearExpr>(m, "Literal", kLiteralClassDoc)
       .def_property_readonly("index", &Literal::index,
                              "The index of the literal in the model.")
       .def("negated", &Literal::negated,
@@ -693,7 +693,7 @@ PYBIND11_MODULE(swig_helper, m) {
   //   object. That means memory of the negated variable is onwed by the C++
   //   layer, but a reference is kept in python to link the lifetime of the
   //   negated variable to the base variable.
-  py::class_<BaseIntVar, PyBaseIntVar, LinearExpr, Literal>(m, "BaseIntVar")
+  py::class_<BaseIntVar, PyBaseIntVar, Literal>(m, "BaseIntVar")
       .def(py::init<int>())        // Integer variable.
       .def(py::init<int, bool>())  // Potential Boolean variable.
       .def_property_readonly("index", &BaseIntVar::index,
@@ -717,7 +717,7 @@ PYBIND11_MODULE(swig_helper, m) {
           "__invert__",
           [](BaseIntVar* self) {
             if (!self->is_boolean()) {
-              throw_error(PyExc_ValueError,
+              throw_error(PyExc_TypeError,
                           "negated() is only supported for Boolean variables.");
             }
             return self->negated();
@@ -729,7 +729,7 @@ PYBIND11_MODULE(swig_helper, m) {
           "Not",
           [](BaseIntVar* self) {
             if (!self->is_boolean()) {
-              throw_error(PyExc_ValueError,
+              throw_error(PyExc_TypeError,
                           "negated() is only supported for Boolean variables.");
             }
             return self->negated();
@@ -739,7 +739,7 @@ PYBIND11_MODULE(swig_helper, m) {
   // Memory management:
   // - Do we need a reference_internal (that add a py::keep_alive<1, 0>() rule)
   //   or just a reference ?
-  py::class_<NotBooleanVariable, LinearExpr, Literal>(m, "NotBooleanVariable")
+  py::class_<NotBooleanVariable, Literal>(m, "NotBooleanVariable")
       .def(py::init<BaseIntVar*>())
       .def_property_readonly("index", &NotBooleanVariable::index,
                              "The index of the variable in the model.")