From b1e8fffc750fadefac54833d3acdf100ce2fbe02 Mon Sep 17 00:00:00 2001 From: Laurent Perron Date: Mon, 6 Jan 2025 21:51:04 +0100 Subject: [PATCH] [CP-SAT] fix more fuzzer bugs; polish python code --- ortools/sat/cp_model_presolve.cc | 6 +- ortools/sat/cp_model_table.cc | 2 +- ortools/sat/cp_model_table_test.cc | 12 ++ ortools/sat/diffn.cc | 8 +- ortools/sat/linear_constraint.cc | 1 + ortools/sat/linear_programming_constraint.cc | 20 +- ortools/sat/linear_relaxation.cc | 10 +- ortools/sat/python/BUILD.bazel | 1 + ortools/sat/python/cp_model.py | 33 ++- ortools/sat/python/cp_model_test.py | 150 ++++++-------- ortools/sat/python/linear_expr.cc | 92 ++++----- ortools/sat/python/linear_expr.h | 201 +++++++++++-------- ortools/sat/python/swig_helper.cc | 31 ++- 13 files changed, 305 insertions(+), 262 deletions(-) diff --git a/ortools/sat/cp_model_presolve.cc b/ortools/sat/cp_model_presolve.cc index 35e0a2a9dc..4ea89c6080 100644 --- a/ortools/sat/cp_model_presolve.cc +++ b/ortools/sat/cp_model_presolve.cc @@ -608,7 +608,7 @@ bool CpModelPresolver::PresolveAtMostOrExactlyOne(ConstraintProto* ct) { // By domination argument, we can fix to false everything but the minimum. if (singleton_literal_with_cost.size() > 1) { - std::sort( + std::stable_sort( singleton_literal_with_cost.begin(), singleton_literal_with_cost.end(), [](const std::pair& a, @@ -1629,8 +1629,8 @@ bool CpModelPresolver::PresolveIntProd(ConstraintProto* ct) { PossibleIntegerOverflow(*context_->working_model, lin->vars(), lin->coeffs(), lin->domain(0))) { context_->working_model->mutable_constraints()->RemoveLast(); - // Re-add a new term with the constant factor. - ct->mutable_int_prod()->add_exprs()->set_offset(constant_factor); + // The constant factor will be handled by the creation of an affine + // relation below. } else { // Replace with a linear equation. context_->UpdateNewConstraintsVariableUsage(); context_->UpdateRuleStats("int_prod: linearize product by constant."); diff --git a/ortools/sat/cp_model_table.cc b/ortools/sat/cp_model_table.cc index 39256c555a..d10ae8cd91 100644 --- a/ortools/sat/cp_model_table.cc +++ b/ortools/sat/cp_model_table.cc @@ -191,7 +191,7 @@ void CompressTuples(absl::Span domain_sizes, std::vector to_remove; std::vector tuple_minus_var_i(num_vars - 1); for (int i = 0; i < num_vars; ++i) { - const int domain_size = domain_sizes[i]; + const int64_t domain_size = domain_sizes[i]; if (domain_size == 1) continue; absl::flat_hash_map, std::vector> masked_tuples_to_indices; diff --git a/ortools/sat/cp_model_table_test.cc b/ortools/sat/cp_model_table_test.cc index 63a758a6d2..92ab451caa 100644 --- a/ortools/sat/cp_model_table_test.cc +++ b/ortools/sat/cp_model_table_test.cc @@ -196,6 +196,18 @@ TEST(CompressTuplesTest, NotPerfect) { EXPECT_EQ(tuples, expected_tuples); } +TEST(CompressTuplesTest, BigInteger) { + const std::vector domain_sizes = {576460752303423490}; + const std::vector> original_tuples = { + {1}, + {2}, + }; + std::vector> tuples = original_tuples; + CompressTuples(domain_sizes, &tuples); + + EXPECT_EQ(tuples, original_tuples); +} + TEST(FullyCompressTuplesTest, BasicTest) { const std::vector domain_sizes = {4, 4}; std::vector> tuples = { diff --git a/ortools/sat/diffn.cc b/ortools/sat/diffn.cc index a81853c2b2..0975351e26 100644 --- a/ortools/sat/diffn.cc +++ b/ortools/sat/diffn.cc @@ -106,8 +106,14 @@ void AddDiffnCumulativeRelationOnX(SchedulingConstraintHelper* x, const IntegerVariable max_end_var = CreateVariableAtOrBelowMaxOf(y->Ends(), model); - // (max_end - min_start) >= capacity. auto* integer_trail = model->GetOrCreate(); + if (integer_trail->UpperBound(max_end_var) < + integer_trail->LowerBound(min_start_var)) { + // Trivial infeasible case, will be handled by the linear constraint + // from the interval. + return; + } + // (max_end - min_start) >= capacity. const AffineExpression capacity(model->Add(NewIntegerVariable( 0, CapSub(integer_trail->UpperBound(max_end_var).value(), integer_trail->LowerBound(min_start_var).value())))); diff --git a/ortools/sat/linear_constraint.cc b/ortools/sat/linear_constraint.cc index 9fcf7fafac..52688cf1b3 100644 --- a/ortools/sat/linear_constraint.cc +++ b/ortools/sat/linear_constraint.cc @@ -61,6 +61,7 @@ void LinearConstraintBuilder::AddTerm(AffineExpression expr, terms_.push_back({NegationOf(expr.var), -coeff * expr.coeff}); } } + DCHECK(!ProdOverflow(coeff, expr.constant)); offset_ += coeff * expr.constant; } diff --git a/ortools/sat/linear_programming_constraint.cc b/ortools/sat/linear_programming_constraint.cc index 29066e5ce9..171ef99012 100644 --- a/ortools/sat/linear_programming_constraint.cc +++ b/ortools/sat/linear_programming_constraint.cc @@ -359,12 +359,13 @@ LinearProgrammingConstraint::LinearProgrammingConstraint( lp_reduced_cost_.assign(vars.size(), 0.0); if (!vars.empty()) { - const int max_index = NegationOf(vars.back()).value(); + const IntegerVariable max_index = std::max( + NegationOf(vars.back()), integer_trail_->NumIntegerVariables() - 1); if (max_index >= expanded_lp_solution_.size()) { - expanded_lp_solution_.assign(max_index + 1, 0.0); + expanded_lp_solution_.assign(max_index.value() + 1, 0.0); } if (max_index >= expanded_reduced_costs_.size()) { - expanded_reduced_costs_.assign(max_index + 1, 0.0); + expanded_reduced_costs_.assign(max_index.value() + 1, 0.0); } } } @@ -805,6 +806,17 @@ void LinearProgrammingConstraint::SetLevel(int level) { lp_solution_is_set_ = true; lp_solution_ = level_zero_lp_solution_; lp_solution_level_ = 0; + // Add the fixed variables. They might have been skipped when we did the + // linear relaxation of the model, but cut generators expect all variables + // to have an LP value. + if (expanded_lp_solution_.size() < integer_trail_->NumIntegerVariables()) { + expanded_lp_solution_.resize(integer_trail_->NumIntegerVariables()); + } + for (IntegerVariable i(0); i < integer_trail_->NumIntegerVariables(); ++i) { + if (integer_trail_->IsFixed(i)) { + expanded_lp_solution_[i] = ToDouble(integer_trail_->LowerBound(i)); + } + } for (int i = 0; i < lp_solution_.size(); i++) { const IntegerVariable var = extended_integer_variables_[i]; expanded_lp_solution_[var] = lp_solution_[i]; @@ -1175,7 +1187,7 @@ bool LinearProgrammingConstraint::AnalyzeLp() { // linear expression == rhs, we can use this to propagate more! // // TODO(user): Also propagate on -cut ? in practice we already do that in many -// places were we try to generate the cut on -cut... But we coould do it sooner +// places were we try to generate the cut on -cut... But we could do it sooner // and more cleanly here. bool LinearProgrammingConstraint::PreprocessCut(IntegerVariable first_slack, CutData* cut) { diff --git a/ortools/sat/linear_relaxation.cc b/ortools/sat/linear_relaxation.cc index d891f7f013..a33e98e7fc 100644 --- a/ortools/sat/linear_relaxation.cc +++ b/ortools/sat/linear_relaxation.cc @@ -1302,7 +1302,10 @@ void AppendLinearConstraintRelaxation(const ConstraintProto& ct, const IntegerVariable int_var = mapping->Integer(ref); lc.AddTerm(int_var, coeff); } - relaxation->linear_constraints.push_back(lc.Build()); + LinearConstraint built_ct = lc.Build(); + if (!PossibleOverflow(*integer_trail, built_ct)) { + relaxation->linear_constraints.push_back(std::move(built_ct)); + } } if (rhs_domain_max < max_activity) { // And(ei) => terms <= rhs_domain_max @@ -1319,7 +1322,10 @@ void AppendLinearConstraintRelaxation(const ConstraintProto& ct, const IntegerVariable int_var = mapping->Integer(ref); lc.AddTerm(int_var, coeff); } - relaxation->linear_constraints.push_back(lc.Build()); + LinearConstraint built_ct = lc.Build(); + if (!PossibleOverflow(*integer_trail, built_ct)) { + relaxation->linear_constraints.push_back(std::move(built_ct)); + } } } diff --git a/ortools/sat/python/BUILD.bazel b/ortools/sat/python/BUILD.bazel index 8d8da9a951..83c85d3a46 100644 --- a/ortools/sat/python/BUILD.bazel +++ b/ortools/sat/python/BUILD.bazel @@ -23,6 +23,7 @@ cc_library( hdrs = ["linear_expr.h"], deps = [ "//ortools/sat:cp_model_cc_proto", + "//ortools/util:fp_roundtrip_conv", "//ortools/util:sorted_interval_list", "@com_google_absl//absl/container:btree", "@com_google_absl//absl/container:fixed_array", diff --git a/ortools/sat/python/cp_model.py b/ortools/sat/python/cp_model.py index f16e93996d..795790c21f 100644 --- a/ortools/sat/python/cp_model.py +++ b/ortools/sat/python/cp_model.py @@ -721,7 +721,7 @@ class CpModel: if not isinstance(index, pd.Index): raise TypeError("Non-index object is used as index") if not name.isidentifier(): - raise ValueError(f"name={name} is not a valid identifier") + raise ValueError(f"name={name!r} is not a valid identifier") if ( isinstance(lower_bounds, IntegralTypes) and isinstance(upper_bounds, IntegralTypes) @@ -775,7 +775,7 @@ class CpModel: if not isinstance(index, pd.Index): raise TypeError("Non-index object is used as index") if not name.isidentifier(): - raise ValueError(f"name={name} is not a valid identifier") + raise ValueError(f"name={name!r} is not a valid identifier") return pd.Series( index=index, data=[ @@ -824,17 +824,9 @@ class CpModel: else: return self.add_bool_and([]) # Evaluate to true. raise TypeError( - "not supported: CpModel.add_linear_expression_in_domain(" - + str(linear_expr) - + " " - + str(type(linear_expr)) - + " " - + str(linear_expr.is_integer()) - + " " - + str(domain) - + " " - + str(type(domain)) - + ")" + f"not supported: CpModel.add_linear_expression_in_domain({linear_expr} " + f" {type(linear_expr)} {linear_expr.is_integer()} {domain} " + f"{type(domain)}" ) def add(self, ct: Union[BoundedLinearExpression, bool, np.bool_]) -> Constraint: @@ -1642,7 +1634,7 @@ class CpModel: if not isinstance(index, pd.Index): raise TypeError("Non-index object is used as index") if not name.isidentifier(): - raise ValueError(f"name={name} is not a valid identifier") + raise ValueError(f"name={name!r} is not a valid identifier") starts = _convert_to_linear_expr_series_and_validate_index(starts, index) sizes = _convert_to_linear_expr_series_and_validate_index(sizes, index) @@ -1715,7 +1707,7 @@ class CpModel: if not isinstance(index, pd.Index): raise TypeError("Non-index object is used as index") if not name.isidentifier(): - raise ValueError(f"name={name} is not a valid identifier") + raise ValueError(f"name={name!r} is not a valid identifier") starts = _convert_to_linear_expr_series_and_validate_index(starts, index) sizes = _convert_to_integral_series_and_validate_index(sizes, index) @@ -1819,7 +1811,7 @@ class CpModel: if not isinstance(index, pd.Index): raise TypeError("Non-index object is used as index") if not name.isidentifier(): - raise ValueError(f"name={name} is not a valid identifier") + raise ValueError(f"name={name!r} is not a valid identifier") starts = _convert_to_linear_expr_series_and_validate_index(starts, index) sizes = _convert_to_linear_expr_series_and_validate_index(sizes, index) @@ -1913,7 +1905,7 @@ class CpModel: if not isinstance(index, pd.Index): raise TypeError("Non-index object is used as index") if not name.isidentifier(): - raise ValueError(f"name={name} is not a valid identifier") + raise ValueError(f"name={name!r} is not a valid identifier") starts = _convert_to_linear_expr_series_and_validate_index(starts, index) sizes = _convert_to_integral_series_and_validate_index(sizes, index) @@ -2857,7 +2849,8 @@ class ObjectiveSolutionPrinter(CpSolverSolutionCallback): obj = self.objective_value print( f"Solution {self.__solution_count}, time =" - f" {current_time - self.__start_time:0.2f} s, objective = {obj}" + f" {current_time - self.__start_time:0.2f} s, objective = {obj}", + flush=True, ) self.__solution_count += 1 @@ -2885,7 +2878,7 @@ class VarArrayAndObjectiveSolutionPrinter(CpSolverSolutionCallback): ) for v in self.__variables: print(f" {v} = {self.value(v)}", end=" ") - print() + print(flush=True) self.__solution_count += 1 @property @@ -2912,7 +2905,7 @@ class VarArraySolutionPrinter(CpSolverSolutionCallback): ) for v in self.__variables: print(f" {v} = {self.value(v)}", end=" ") - print() + print(flush=True) self.__solution_count += 1 @property diff --git a/ortools/sat/python/cp_model_test.py b/ortools/sat/python/cp_model_test.py index 4510611a6d..25842a5a51 100644 --- a/ortools/sat/python/cp_model_test.py +++ b/ortools/sat/python/cp_model_test.py @@ -570,42 +570,19 @@ class CpModelTest(absltest.TestCase): self.assertEqual(1.0, solver.objective_value) for i in range(100): self.assertEqual(solver.value(x[i]), 1 if i == 99 else 0) - self.assertRaises( - ValueError, - cp_model.LinearExpr.weighted_sum, - [x[0]], - [1, 2], - ) - self.assertRaises( - ValueError, - cp_model.LinearExpr.weighted_sum, - [x[0]], - [1.1, 2.2], - ) - self.assertRaises( - ValueError, - cp_model.LinearExpr.weighted_sum, - [x[0], 3, 5], - [1, 2], - ) - self.assertRaises( - ValueError, - cp_model.LinearExpr.weighted_sum, - [x[0], 2.2, 3], - [1.1, 2.2], - ) - self.assertRaises( - ValueError, - cp_model.LinearExpr.WeightedSum, - [x[0]], - [1, 2], - ) - self.assertRaises( - ValueError, - cp_model.LinearExpr.WeightedSum, - [x[0]], - [1.1, 2.2], - ) + + with self.assertRaises(ValueError): + cp_model.LinearExpr.weighted_sum([x[0]], [1, 2]) + with self.assertRaises(ValueError): + cp_model.LinearExpr.weighted_sum([x[0]], [1.1, 2.2]) + with self.assertRaises(ValueError): + cp_model.LinearExpr.weighted_sum([x[0], 3, 5], [1, 2]) + with self.assertRaises(ValueError): + cp_model.LinearExpr.weighted_sum([x[0], 2.2, 3], [1.1, 2.2]) + with self.assertRaises(ValueError): + cp_model.LinearExpr.WeightedSum([x[0]], [1, 2]) + with self.assertRaises(ValueError): + cp_model.LinearExpr.WeightedSum([x[0]], [1.1, 2.2]) def testAllDifferent(self) -> None: print("testAllDifferent") @@ -643,7 +620,8 @@ class CpModelTest(absltest.TestCase): self.assertLen(model.proto.constraints[0].element.exprs, 4) self.assertEqual(0, model.proto.constraints[0].element.linear_index.vars[0]) self.assertEqual(4, model.proto.constraints[0].element.linear_target.vars[0]) - self.assertRaises(ValueError, model.add_element, x[0], [], x[4]) + with self.assertRaises(ValueError): + model.add_element(x[0], [], x[4]) def testFixedElement(self) -> None: print("testFixedElement") @@ -691,7 +669,8 @@ class CpModelTest(absltest.TestCase): self.assertLen(model.proto.constraints[0].circuit.heads, 5) self.assertLen(model.proto.constraints[0].circuit.tails, 5) self.assertLen(model.proto.constraints[0].circuit.literals, 5) - self.assertRaises(ValueError, model.add_circuit, []) + with self.assertRaises(ValueError): + model.add_circuit([]) def testMultipleCircuit(self) -> None: print("testMultipleCircuit") @@ -706,7 +685,8 @@ class CpModelTest(absltest.TestCase): self.assertLen(model.proto.constraints[0].routes.heads, 5) self.assertLen(model.proto.constraints[0].routes.tails, 5) self.assertLen(model.proto.constraints[0].routes.literals, 5) - self.assertRaises(ValueError, model.add_multiple_circuit, []) + with self.assertRaises(ValueError): + model.add_multiple_circuit([]) def testAllowedAssignments(self) -> None: print("testAllowedAssignments") @@ -719,18 +699,16 @@ class CpModelTest(absltest.TestCase): self.assertLen(model.proto.constraints, 1) self.assertLen(model.proto.constraints[0].table.exprs, 5) self.assertLen(model.proto.constraints[0].table.values, 15) - self.assertRaises( - TypeError, - model.add_allowed_assignments, - x, - [(0, 1, 2, 3, 4), (4, 3, 2, 1, 1), (0, 0, 0, 0)], - ) - self.assertRaises( - ValueError, - model.add_allowed_assignments, - [], - [(0, 1, 2, 3, 4), (4, 3, 2, 1, 1), (0, 0, 0, 0)], - ) + with self.assertRaises(TypeError): + model.add_allowed_assignments( + x, + [(0, 1, 2, 3, 4), (4, 3, 2, 1, 1), (0, 0, 0, 0)], + ) + with self.assertRaises(ValueError): + model.add_allowed_assignments( + [], + [(0, 1, 2, 3, 4), (4, 3, 2, 1, 1), (0, 0, 0, 0)], + ) def testForbiddenAssignments(self) -> None: print("testForbiddenAssignments") @@ -770,31 +748,29 @@ class CpModelTest(absltest.TestCase): self.assertLen(model.proto.constraints[0].automaton.transition_label, 4) self.assertLen(model.proto.constraints[0].automaton.final_states, 2) self.assertEqual(0, model.proto.constraints[0].automaton.starting_state) - self.assertRaises( - TypeError, - model.add_automaton, - x, - 0, - [2, 3], - [(0, 0, 0), (0, 1, 1), (2, 2), (2, 3, 3)], - ) - self.assertRaises( - ValueError, - model.add_automaton, - [], - 0, - [2, 3], - [(0, 0, 0), (0, 1, 1), (2, 3, 3)], - ) - self.assertRaises( - ValueError, - model.add_automaton, - x, - 0, - [], - [(0, 0, 0), (0, 1, 1), (2, 3, 3)], - ) - self.assertRaises(ValueError, model.add_automaton, x, 0, [2, 3], []) + with self.assertRaises(TypeError): + model.add_automaton( + x, + 0, + [2, 3], + [(0, 0, 0), (0, 1, 1), (2, 2), (2, 3, 3)], + ) + with self.assertRaises(ValueError): + model.add_automaton( + [], + 0, + [2, 3], + [(0, 0, 0), (0, 1, 1), (2, 3, 3)], + ) + with self.assertRaises(ValueError): + model.add_automaton( + x, + 0, + [], + [(0, 0, 0), (0, 1, 1), (2, 3, 3)], + ) + with self.assertRaises(ValueError): + model.add_automaton(x, 0, [2, 3], []) def testInverse(self) -> None: print("testInverse") @@ -995,9 +971,11 @@ class CpModelTest(absltest.TestCase): self.assertLen(model.proto.constraints[0].bool_or.literals, 5) model.add_bool_or([x[0], x[1], False]) self.assertLen(model.proto.variables, 6) - self.assertRaises(TypeError, model.add_bool_or, [x[2], 2]) + with self.assertRaises(TypeError): + model.add_bool_or([x[2], 2]) y = model.new_int_var(0, 4, "y") - self.assertRaises(TypeError, model.add_bool_or, [y, False]) + with self.assertRaises(TypeError): + model.add_bool_or([y, False]) def testBoolOrListOrGet(self) -> None: print("testBoolOrListOrGet") @@ -1153,13 +1131,12 @@ class CpModelTest(absltest.TestCase): self.assertEqual(1, j.index) self.assertEqual(2, k.index) self.assertEqual(3, l.index) - self.assertRaises(TypeError, model.new_optional_interval_var, 1, 2, 3, x, "x") - self.assertRaises( - TypeError, model.new_optional_interval_var, b + x, 2, 3, b, "x" - ) - self.assertRaises( - TypeError, model.new_optional_interval_var, 1, 2, 3, b + 1, "x" - ) + with self.assertRaises(TypeError): + model.new_optional_interval_var(1, 2, 3, x, "x") + with self.assertRaises(TypeError): + model.new_optional_interval_var(b + x, 2, 3, b, "x") + with self.assertRaises(TypeError): + model.new_optional_interval_var(1, 2, 3, b + 1, "x") def testNoOverlap(self) -> None: print("testNoOverlap") @@ -1209,7 +1186,8 @@ class CpModelTest(absltest.TestCase): ct = model.add_cumulative(intervals, demands, capacity) self.assertEqual(10, ct.index) self.assertLen(ct.proto.cumulative.intervals, 10) - self.assertRaises(TypeError, model.add_cumulative, [intervals[0], 3], [2, 3], 3) + with self.assertRaises(TypeError): + model.add_cumulative([intervals[0], 3], [2, 3], 3) def testGetOrMakeIndexFromConstant(self) -> None: print("testGetOrMakeIndexFromConstant") diff --git a/ortools/sat/python/linear_expr.cc b/ortools/sat/python/linear_expr.cc index 21eddf2c7b..2ec285a31c 100644 --- a/ortools/sat/python/linear_expr.cc +++ b/ortools/sat/python/linear_expr.cc @@ -22,13 +22,14 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" +#include "ortools/util/fp_roundtrip_conv.h" #include "ortools/util/sorted_interval_list.h" namespace operations_research { namespace sat { namespace python { -bool LinearExpr::IsInteger() { +bool LinearExpr::IsInteger() const { IntExprVisitor lin; lin.AddToProcess(this, 1); return lin.ProcessAll(); @@ -240,21 +241,21 @@ LinearExpr* LinearExpr::MulFloat(double cst) { LinearExpr* LinearExpr::Neg() { return new IntAffine(this, -1, 0); } -void FloatExprVisitor::AddToProcess(LinearExpr* expr, double coeff) { +void FloatExprVisitor::AddToProcess(const LinearExpr* expr, double coeff) { to_process_.push_back(std::make_pair(expr, coeff)); } void FloatExprVisitor::AddConstant(double constant) { offset_ += constant; } -void FloatExprVisitor::AddVarCoeff(BaseIntVar* var, double coeff) { +void FloatExprVisitor::AddVarCoeff(const BaseIntVar* var, double coeff) { canonical_terms_[var] += coeff; } -double FloatExprVisitor::Process(LinearExpr* expr, - std::vector* vars, +double FloatExprVisitor::Process(const LinearExpr* expr, + std::vector* vars, std::vector* coeffs) { AddToProcess(expr, 1.0); while (!to_process_.empty()) { const auto [expr, coeff] = to_process_.back(); to_process_.pop_back(); - expr->VisitAsFloat(this, coeff); + expr->VisitAsFloat(*this, coeff); } vars->clear(); @@ -273,20 +274,20 @@ CanonicalFloatExpression::CanonicalFloatExpression(LinearExpr* expr) { offset_ = lin.Process(expr, &vars_, &coeffs_); } -CanonicalIntExpression::CanonicalIntExpression(LinearExpr* expr) : ok_(true) { +CanonicalIntExpression::CanonicalIntExpression(LinearExpr* expr) { IntExprVisitor lin; lin.AddToProcess(expr, 1); ok_ = lin.Process(&vars_, &coeffs_, &offset_); } -void FloatConstant::VisitAsFloat(FloatExprVisitor* lin, double c) { - lin->AddConstant(value_ * c); +void FloatConstant::VisitAsFloat(FloatExprVisitor& lin, double c) const { + lin.AddConstant(value_ * c); } std::string FloatConstant::ToString() const { return absl::StrCat(value_); } std::string FloatConstant::DebugString() const { - return absl::StrCat("FloatConstant(", value_, ")"); + return absl::StrCat("FloatConstant(", RoundTripDoubleFormat(value_), ")"); } FloatWeightedSum::FloatWeightedSum(const std::vector& exprs, @@ -302,11 +303,11 @@ FloatWeightedSum::FloatWeightedSum(const std::vector& exprs, coeffs_(coeffs.begin(), coeffs.end()), offset_(offset) {} -void FloatWeightedSum::VisitAsFloat(FloatExprVisitor* lin, double c) { +void FloatWeightedSum::VisitAsFloat(FloatExprVisitor& lin, double c) const { for (int i = 0; i < exprs_.size(); ++i) { - lin->AddToProcess(exprs_[i], coeffs_[i] * c); + lin.AddToProcess(exprs_[i], coeffs_[i] * c); } - lin->AddConstant(offset_ * c); + lin.AddConstant(offset_ * c); } std::string FloatWeightedSum::ToString() const { @@ -423,9 +424,9 @@ std::string IntWeightedSum::DebugString() const { FloatAffine::FloatAffine(LinearExpr* expr, double coeff, double offset) : expr_(expr), coeff_(coeff), offset_(offset) {} -void FloatAffine::VisitAsFloat(FloatExprVisitor* lin, double c) { - lin->AddToProcess(expr_, c * coeff_); - lin->AddConstant(offset_ * c); +void FloatAffine::VisitAsFloat(FloatExprVisitor& lin, double c) const { + lin.AddToProcess(expr_, c * coeff_); + lin.AddConstant(offset_ * c); } std::string FloatAffine::ToString() const { @@ -454,7 +455,7 @@ BoundedLinearExpression* LinearExpr::Eq(LinearExpr* rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); lin.AddToProcess(rhs, -1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -464,7 +465,7 @@ BoundedLinearExpression* LinearExpr::Eq(LinearExpr* rhs) { BoundedLinearExpression* LinearExpr::EqCst(int64_t rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -475,7 +476,7 @@ BoundedLinearExpression* LinearExpr::Ne(LinearExpr* rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); lin.AddToProcess(rhs, -1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -486,7 +487,7 @@ BoundedLinearExpression* LinearExpr::Ne(LinearExpr* rhs) { BoundedLinearExpression* LinearExpr::NeCst(int64_t rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -498,7 +499,7 @@ BoundedLinearExpression* LinearExpr::Le(LinearExpr* rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); lin.AddToProcess(rhs, -1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -509,7 +510,7 @@ BoundedLinearExpression* LinearExpr::Le(LinearExpr* rhs) { BoundedLinearExpression* LinearExpr::LeCst(int64_t rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -521,7 +522,7 @@ BoundedLinearExpression* LinearExpr::Lt(LinearExpr* rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); lin.AddToProcess(rhs, -1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -532,7 +533,7 @@ BoundedLinearExpression* LinearExpr::Lt(LinearExpr* rhs) { BoundedLinearExpression* LinearExpr::LtCst(int64_t rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -545,7 +546,7 @@ BoundedLinearExpression* LinearExpr::Ge(LinearExpr* rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); lin.AddToProcess(rhs, -1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -556,7 +557,7 @@ BoundedLinearExpression* LinearExpr::Ge(LinearExpr* rhs) { BoundedLinearExpression* LinearExpr::GeCst(int64_t rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -568,7 +569,7 @@ BoundedLinearExpression* LinearExpr::Gt(LinearExpr* rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); lin.AddToProcess(rhs, -1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -579,7 +580,7 @@ BoundedLinearExpression* LinearExpr::Gt(LinearExpr* rhs) { BoundedLinearExpression* LinearExpr::GtCst(int64_t rhs) { IntExprVisitor lin; lin.AddToProcess(this, 1); - std::vector vars; + std::vector vars; std::vector coeffs; int64_t offset; if (!lin.Process(&vars, &coeffs, &offset)) return nullptr; @@ -588,13 +589,13 @@ BoundedLinearExpression* LinearExpr::GtCst(int64_t rhs) { Domain(rhs + 1, std::numeric_limits::max())); } -void IntExprVisitor::AddToProcess(LinearExpr* expr, int64_t coeff) { +void IntExprVisitor::AddToProcess(const LinearExpr* expr, int64_t coeff) { to_process_.push_back(std::make_pair(expr, coeff)); } void IntExprVisitor::AddConstant(int64_t constant) { offset_ += constant; } -void IntExprVisitor::AddVarCoeff(BaseIntVar* var, int64_t coeff) { +void IntExprVisitor::AddVarCoeff(const BaseIntVar* var, int64_t coeff) { canonical_terms_[var] += coeff; } @@ -602,12 +603,12 @@ bool IntExprVisitor::ProcessAll() { while (!to_process_.empty()) { const auto [expr, coeff] = to_process_.back(); to_process_.pop_back(); - if (!expr->VisitAsInt(this, coeff)) return false; + if (!expr->VisitAsInt(*this, coeff)) return false; } return true; } -bool IntExprVisitor::Process(std::vector* vars, +bool IntExprVisitor::Process(std::vector* vars, std::vector* coeffs, int64_t* offset) { if (!ProcessAll()) return false; vars->clear(); @@ -621,7 +622,7 @@ bool IntExprVisitor::Process(std::vector* vars, return true; } -bool IntExprVisitor::Evaluate(LinearExpr* expr, +bool IntExprVisitor::Evaluate(const LinearExpr* expr, const CpSolverResponse& solution, int64_t* value) { AddToProcess(expr, 1); @@ -644,14 +645,13 @@ BaseIntVar::BaseIntVar(int index, bool is_boolean) : index_(index), negated_(is_boolean ? new NotBooleanVariable(this) : nullptr) {} -BoundedLinearExpression::BoundedLinearExpression(std::vector vars, - std::vector coeffs, - int64_t offset, - const Domain& bounds) +BoundedLinearExpression::BoundedLinearExpression( + const std::vector& vars, + const std::vector& coeffs, int64_t offset, const Domain& bounds) : vars_(vars), coeffs_(coeffs), offset_(offset), bounds_(bounds) {} const Domain& BoundedLinearExpression::bounds() const { return bounds_; } -const std::vector& BoundedLinearExpression::vars() const { +const std::vector& BoundedLinearExpression::vars() const { return vars_; } const std::vector& BoundedLinearExpression::coeffs() const { @@ -731,14 +731,14 @@ std::string BoundedLinearExpression::ToString() const { } std::string BoundedLinearExpression::DebugString() const { - return absl::StrCat("BoundedLinearExpression(vars=[", - absl::StrJoin(vars_, ", ", - [](std::string* out, BaseIntVar* var) { - absl::StrAppend(out, var->DebugString()); - }), - "], coeffs=[", absl::StrJoin(coeffs_, ", "), - "], offset=", offset_, ", bounds=", bounds_.ToString(), - ")"); + return absl::StrCat( + "BoundedLinearExpression(vars=[", + absl::StrJoin(vars_, ", ", + [](std::string* out, const BaseIntVar* var) { + absl::StrAppend(out, var->DebugString()); + }), + "], coeffs=[", absl::StrJoin(coeffs_, ", "), "], offset=", offset_, + ", bounds=", bounds_.ToString(), ")"); } bool BoundedLinearExpression::CastToBool(bool* result) const { diff --git a/ortools/sat/python/linear_expr.h b/ortools/sat/python/linear_expr.h index 0f9ee99e47..e6349b3738 100644 --- a/ortools/sat/python/linear_expr.h +++ b/ortools/sat/python/linear_expr.h @@ -25,11 +25,10 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" #include "ortools/sat/cp_model.pb.h" +#include "ortools/util/fp_roundtrip_conv.h" #include "ortools/util/sorted_interval_list.h" -namespace operations_research { -namespace sat { -namespace python { +namespace operations_research::sat::python { class BoundedLinearExpression; class CanonicalFloatExpression; @@ -40,7 +39,7 @@ class LinearExpr; class BaseIntVar; class NotBooleanVariable; -// A class to hold an linear expression or a constant. +// A class to hold a pointer to a linear expression or a constant. struct ExprOrValue { explicit ExprOrValue(LinearExpr* e) : expr(e) {} explicit ExprOrValue(double v) : double_value(v) {} @@ -51,56 +50,93 @@ struct ExprOrValue { int64_t int_value = 0; }; -// A linear expression that can be either integer or floating point. +// Interface for a linear expression that can be either integer or floating +// point. class LinearExpr { public: virtual ~LinearExpr() = default; - virtual void VisitAsFloat(FloatExprVisitor* /*lin*/, double /*c*/) = 0; - virtual bool VisitAsInt(IntExprVisitor* /*lin*/, int64_t /*c*/) = 0; - bool IsInteger(); - virtual std::string ToString() const { return "LinearExpr"; } - virtual std::string DebugString() const { return "LinearExpr()"; } + virtual void VisitAsFloat(FloatExprVisitor& /*lin*/, double /*c*/) const = 0; + virtual bool VisitAsInt(IntExprVisitor& /*lin*/, int64_t /*c*/) const = 0; + bool IsInteger() const; + virtual std::string ToString() const = 0; + virtual std::string DebugString() const = 0; + // Returns a new LinearExpr that is the sum of the given expressions. static LinearExpr* Sum(const std::vector& exprs); + // Returns a new LinearExpr that is the sum of the given expressions or + // constants. static LinearExpr* MixedSum(const std::vector& exprs); + // Returns the sum(exprs[i] * coeffs[i]). static LinearExpr* WeightedSumInt(const std::vector& exprs, const std::vector& coeffs); + // Returns the sum(exprs[i] * coeffs[i]). static LinearExpr* WeightedSumFloat(const std::vector& exprs, const std::vector& coeffs); + // Returns the sum(exprs[i] * coeffs[i]). static LinearExpr* MixedWeightedSumInt(const std::vector& exprs, const std::vector& coeffs); + // Returns the sum(exprs[i] * coeffs[i]). static LinearExpr* MixedWeightedSumFloat( const std::vector& exprs, const std::vector& coeffs); + // returns expr * coeff. static LinearExpr* TermInt(LinearExpr* expr, int64_t coeff); + // returns expr * coeff. static LinearExpr* TermFloat(LinearExpr* expr, double coeff); + // returns expr * coeff + offset. static LinearExpr* AffineInt(LinearExpr* expr, int64_t coeff, int64_t offset); + // returns expr * coeff + offset. static LinearExpr* AffineFloat(LinearExpr* expr, double coeff, double offset); + // Returns a new LinearExpr that is the given constant. static LinearExpr* ConstantInt(int64_t value); + // Returns a new LinearExpr that is the given constant. static LinearExpr* ConstantFloat(double value); + // return (this) + (expr). LinearExpr* Add(LinearExpr* expr); + // return (this) + (cst). LinearExpr* AddInt(int64_t cst); + // return (this) + (cst). LinearExpr* AddFloat(double cst); + // return (this) - (expr). LinearExpr* Sub(LinearExpr* expr); + // return (this) - (cst). LinearExpr* SubInt(int64_t cst); + // return (this) - (cst). LinearExpr* SubFloat(double cst); + // return (cst) - (this). LinearExpr* RSubInt(int64_t cst); + // return (cst) - (this). LinearExpr* RSubFloat(double cst); + // return (this) * (cst). LinearExpr* MulInt(int64_t cst); + // return (this) * (cst). LinearExpr* MulFloat(double cst); + // return -(this). LinearExpr* Neg(); + // returns (this) == (rhs). BoundedLinearExpression* Eq(LinearExpr* rhs); + // returns (this) == (rhs). BoundedLinearExpression* EqCst(int64_t rhs); + // returns (this) != (rhs). BoundedLinearExpression* Ne(LinearExpr* rhs); + // returns (this) != (rhs). BoundedLinearExpression* NeCst(int64_t rhs); + // returns (this) >= (rhs). BoundedLinearExpression* Ge(LinearExpr* rhs); + // returns (this) >= (rhs). BoundedLinearExpression* GeCst(int64_t rhs); + // returns (this) <= (rhs). BoundedLinearExpression* Le(LinearExpr* rhs); + // returns (this) <= (rhs). BoundedLinearExpression* LeCst(int64_t rhs); + // returns (this) < (rhs). BoundedLinearExpression* Lt(LinearExpr* rhs); + // returns (this) < (rhs). BoundedLinearExpression* LtCst(int64_t rhs); + // returns (this) > (rhs). BoundedLinearExpression* Gt(LinearExpr* rhs); + // returns (this) > (rhs). BoundedLinearExpression* GtCst(int64_t rhs); }; @@ -112,15 +148,16 @@ struct BaseIntVarComparator { // A visitor class to process a floating point linear expression. class FloatExprVisitor { public: - void AddToProcess(LinearExpr* expr, double coeff); + void AddToProcess(const LinearExpr* expr, double coeff); void AddConstant(double constant); - void AddVarCoeff(BaseIntVar* var, double coeff); - double Process(LinearExpr* expr, std::vector* vars, + void AddVarCoeff(const BaseIntVar* var, double coeff); + double Process(const LinearExpr* expr, std::vector* vars, std::vector* coeffs); private: - std::vector> to_process_; - absl::btree_map canonical_terms_; + std::vector> to_process_; + absl::btree_map + canonical_terms_; double offset_ = 0; }; @@ -128,31 +165,32 @@ class FloatExprVisitor { class CanonicalFloatExpression { public: explicit CanonicalFloatExpression(LinearExpr* expr); - const std::vector& vars() const { return vars_; } + const std::vector& vars() const { return vars_; } const std::vector& coeffs() const { return coeffs_; } double offset() const { return offset_; } private: - std::vector vars_; + std::vector vars_; std::vector coeffs_; - double offset_; + double offset_ = 0; }; // A visitor class to process an integer linear expression. class IntExprVisitor { public: - void AddToProcess(LinearExpr* expr, int64_t coeff); + void AddToProcess(const LinearExpr* expr, int64_t coeff); void AddConstant(int64_t constant); - void AddVarCoeff(BaseIntVar* var, int64_t coeff); + void AddVarCoeff(const BaseIntVar* var, int64_t coeff); bool ProcessAll(); - bool Process(std::vector* vars, std::vector* coeffs, - int64_t* offset); - bool Evaluate(LinearExpr* expr, const CpSolverResponse& solution, + bool Process(std::vector* vars, + std::vector* coeffs, int64_t* offset); + bool Evaluate(const LinearExpr* expr, const CpSolverResponse& solution, int64_t* value); private: - std::vector> to_process_; - absl::btree_map canonical_terms_; + std::vector> to_process_; + absl::btree_map + canonical_terms_; int64_t offset_ = 0; }; @@ -160,53 +198,55 @@ class IntExprVisitor { class CanonicalIntExpression { public: explicit CanonicalIntExpression(LinearExpr* expr); - const std::vector& vars() const { return vars_; } + const std::vector& vars() const { return vars_; } const std::vector& coeffs() const { return coeffs_; } int64_t offset() const { return offset_; } bool ok() const { return ok_; } private: - std::vector vars_; + std::vector vars_; std::vector coeffs_; - int64_t offset_; - bool ok_; + int64_t offset_ = 0; + bool ok_ = true; }; // A class to hold a sum of linear expressions, and optional integer and -// double offsets. +// double offsets (at most one of them can be non-zero, this is DCHECKed). class SumArray : public LinearExpr { public: explicit SumArray(const std::vector& exprs, int64_t int_offset = 0, double double_offset = 0.0) : exprs_(exprs.begin(), exprs.end()), int_offset_(int_offset), - double_offset_(double_offset) {} + double_offset_(double_offset) { + DCHECK(int_offset_ == 0 || double_offset_ == 0.0); + } ~SumArray() override = default; - bool VisitAsInt(IntExprVisitor* lin, int64_t c) override { + bool VisitAsInt(IntExprVisitor& lin, int64_t c) const override { if (double_offset_ != 0.0) return false; for (int i = 0; i < exprs_.size(); ++i) { - lin->AddToProcess(exprs_[i], c); + lin.AddToProcess(exprs_[i], c); } - lin->AddConstant(int_offset_ * c); + lin.AddConstant(int_offset_ * c); return true; } - void VisitAsFloat(FloatExprVisitor* lin, double c) override { + void VisitAsFloat(FloatExprVisitor& lin, double c) const override { for (int i = 0; i < exprs_.size(); ++i) { - lin->AddToProcess(exprs_[i], c); + lin.AddToProcess(exprs_[i], c); } if (int_offset_ != 0) { - lin->AddConstant(int_offset_ * c); + lin.AddConstant(int_offset_ * c); } else if (double_offset_ != 0.0) { - lin->AddConstant(double_offset_ * c); + lin.AddConstant(double_offset_ * c); } } std::string ToString() const override { if (exprs_.empty()) { if (double_offset_ != 0.0) { - return absl::StrCat(double_offset_); + return absl::StrCat(RoundTripDoubleFormat(double_offset_)); } else { return absl::StrCat(int_offset_); } @@ -246,7 +286,8 @@ class SumArray : public LinearExpr { absl::StrAppend(&s, ", int_offset=", int_offset_); } if (double_offset_ != 0.0) { - absl::StrAppend(&s, ", double_offset=", double_offset_); + absl::StrAppend( + &s, ", double_offset=", RoundTripDoubleFormat(double_offset_)); } absl::StrAppend(&s, ")"); return s; @@ -266,11 +307,11 @@ class FloatWeightedSum : public LinearExpr { const std::vector& coeffs, double offset); ~FloatWeightedSum() override = default; - void VisitAsFloat(FloatExprVisitor* lin, double c) override; + void VisitAsFloat(FloatExprVisitor& lin, double c) const override; std::string ToString() const override; std::string DebugString() const override; - bool VisitAsInt(IntExprVisitor* /*lin*/, int64_t /*c*/) override { + bool VisitAsInt(IntExprVisitor& /*lin*/, int64_t /*c*/) const override { return false; } @@ -290,18 +331,18 @@ class IntWeightedSum : public LinearExpr { offset_(offset) {} ~IntWeightedSum() override = default; - void VisitAsFloat(FloatExprVisitor* lin, double c) override { + void VisitAsFloat(FloatExprVisitor& lin, double c) const override { for (int i = 0; i < exprs_.size(); ++i) { - lin->AddToProcess(exprs_[i], coeffs_[i] * c); + lin.AddToProcess(exprs_[i], coeffs_[i] * c); } - lin->AddConstant(offset_ * c); + lin.AddConstant(offset_ * c); } - bool VisitAsInt(IntExprVisitor* lin, int64_t c) override { + bool VisitAsInt(IntExprVisitor& lin, int64_t c) const override { for (int i = 0; i < exprs_.size(); ++i) { - lin->AddToProcess(exprs_[i], coeffs_[i] * c); + lin.AddToProcess(exprs_[i], coeffs_[i] * c); } - lin->AddConstant(offset_ * c); + lin.AddConstant(offset_ * c); return true; } @@ -320,8 +361,8 @@ class FloatAffine : public LinearExpr { FloatAffine(LinearExpr* expr, double coeff, double offset); ~FloatAffine() override = default; - void VisitAsFloat(FloatExprVisitor* lin, double c) override; - bool VisitAsInt(IntExprVisitor* /*lin*/, int64_t /*c*/) override { + void VisitAsFloat(FloatExprVisitor& lin, double c) const override; + bool VisitAsInt(IntExprVisitor& /*lin*/, int64_t /*c*/) const override { return false; } std::string ToString() const override; @@ -344,15 +385,15 @@ class IntAffine : public LinearExpr { : expr_(expr), coeff_(coeff), offset_(offset) {} ~IntAffine() override = default; - bool VisitAsInt(IntExprVisitor* lin, int64_t c) override { - lin->AddToProcess(expr_, c * coeff_); - lin->AddConstant(offset_ * c); + bool VisitAsInt(IntExprVisitor& lin, int64_t c) const override { + lin.AddToProcess(expr_, c * coeff_); + lin.AddConstant(offset_ * c); return true; } - void VisitAsFloat(FloatExprVisitor* lin, double c) override { - lin->AddToProcess(expr_, c * coeff_); - lin->AddConstant(offset_ * c); + void VisitAsFloat(FloatExprVisitor& lin, double c) const override { + lin.AddToProcess(expr_, c * coeff_); + lin.AddConstant(offset_ * c); } std::string ToString() const override { @@ -394,8 +435,8 @@ class FloatConstant : public LinearExpr { explicit FloatConstant(double value) : value_(value) {} ~FloatConstant() override = default; - void VisitAsFloat(FloatExprVisitor* lin, double c) override; - bool VisitAsInt(IntExprVisitor* /*lin*/, int64_t /*c*/) override { + void VisitAsFloat(FloatExprVisitor& lin, double c) const override; + bool VisitAsInt(IntExprVisitor& /*lin*/, int64_t /*c*/) const override { return false; } std::string ToString() const override; @@ -410,13 +451,13 @@ class IntConstant : public LinearExpr { public: explicit IntConstant(int64_t value) : value_(value) {} ~IntConstant() override = default; - bool VisitAsInt(IntExprVisitor* lin, int64_t c) override { - lin->AddConstant(value_ * c); + bool VisitAsInt(IntExprVisitor& lin, int64_t c) const override { + lin.AddConstant(value_ * c); return true; } - void VisitAsFloat(FloatExprVisitor* lin, double c) override { - lin->AddConstant(value_ * c); + void VisitAsFloat(FloatExprVisitor& lin, double c) const override { + lin.AddConstant(value_ * c); } std::string ToString() const override { return absl::StrCat(value_); } @@ -451,13 +492,13 @@ class BaseIntVar : public Literal { int index() const override { return index_; } - bool VisitAsInt(IntExprVisitor* lin, int64_t c) override { - lin->AddVarCoeff(this, c); + bool VisitAsInt(IntExprVisitor& lin, int64_t c) const override { + lin.AddVarCoeff(this, c); return true; } - void VisitAsFloat(FloatExprVisitor* lin, double c) override { - lin->AddVarCoeff(this, c); + void VisitAsFloat(FloatExprVisitor& lin, double c) const override { + lin.AddVarCoeff(this, c); } std::string ToString() const override { @@ -499,15 +540,15 @@ class NotBooleanVariable : public Literal { int index() const override { return -var_->index() - 1; } - bool VisitAsInt(IntExprVisitor* lin, int64_t c) override { - lin->AddVarCoeff(var_, -c); - lin->AddConstant(c); + bool VisitAsInt(IntExprVisitor& lin, int64_t c) const override { + lin.AddVarCoeff(var_, -c); + lin.AddConstant(c); return true; } - void VisitAsFloat(FloatExprVisitor* lin, double c) override { - lin->AddVarCoeff(var_, -c); - lin->AddConstant(c); + void VisitAsFloat(FloatExprVisitor& lin, double c) const override { + lin.AddVarCoeff(var_, -c); + lin.AddConstant(c); } std::string ToString() const override { @@ -521,20 +562,20 @@ class NotBooleanVariable : public Literal { } private: - BaseIntVar* var_; + BaseIntVar* const var_; }; // A class to hold a linear expression with bounds. class BoundedLinearExpression { public: - BoundedLinearExpression(std::vector vars, - std::vector coeffs, int64_t offset, + BoundedLinearExpression(const std::vector& vars, + const std::vector& coeffs, int64_t offset, const Domain& bounds); ~BoundedLinearExpression() = default; const Domain& bounds() const; - const std::vector& vars() const; + const std::vector& vars() const; const std::vector& coeffs() const; int64_t offset() const; std::string ToString() const; @@ -542,14 +583,12 @@ class BoundedLinearExpression { bool CastToBool(bool* result) const; private: - std::vector vars_; - std::vector coeffs_; + const std::vector vars_; + const std::vector coeffs_; int64_t offset_; const Domain bounds_; }; -} // namespace python -} // namespace sat -} // namespace operations_research +} // namespace operations_research::sat::python #endif // OR_TOOLS_SAT_PYTHON_LINEAR_EXPR_H_ diff --git a/ortools/sat/python/swig_helper.cc b/ortools/sat/python/swig_helper.cc index a56242517f..129a5417cb 100644 --- a/ortools/sat/python/swig_helper.cc +++ b/ortools/sat/python/swig_helper.cc @@ -35,9 +35,7 @@ namespace py = pybind11; -namespace operations_research { -namespace sat { -namespace python { +namespace operations_research::sat::python { using ::py::arg; @@ -70,18 +68,18 @@ class PyBaseIntVar : public BaseIntVar { using BaseIntVar::BaseIntVar; /* Inherit constructors */ std::string ToString() const override { - PYBIND11_OVERRIDE_NAME(std::string, // Return type (ret_type) - BaseIntVar, // Parent class (cname) - "__str__", // Name of method in Python (name) - ToString, // Name of function in C++ (fn) + PYBIND11_OVERRIDE_PURE_NAME(std::string, // Return type (ret_type) + BaseIntVar, // Parent class (cname) + "__str__", // Name of method in Python (name) + ToString, // Name of function in C++ (fn) ); } std::string DebugString() const override { - PYBIND11_OVERRIDE_NAME(std::string, // Return type (ret_type) - BaseIntVar, // Parent class (cname) - "__repr__", // Name of method in Python (name) - DebugString, // Name of function in C++ (fn) + PYBIND11_OVERRIDE_PURE_NAME(std::string, // Return type (ret_type) + BaseIntVar, // Parent class (cname) + "__repr__", // Name of method in Python (name) + DebugString, // Name of function in C++ (fn) ); } }; @@ -163,8 +161,7 @@ class ResponseWrapper { const CpSolverResponse response_; }; -const char* kLinearExprClassDoc = R"doc( - Holds an integer linear expression. +const char* kLinearExprClassDoc = R"doc(Holds an integer linear expression. A linear expression is built from integer constants and variables. For example, `x + 2 * (y - z + 1)`. @@ -810,8 +807,8 @@ PYBIND11_MODULE(swig_helper, m) { py::return_value_policy::reference_internal); py::class_(m, "BoundedLinearExpression") - .def(py::init, std::vector, int64_t, - Domain>()) + .def(py::init, std::vector, + int64_t, Domain>()) .def_property_readonly("bounds", &BoundedLinearExpression::bounds) .def_property_readonly("vars", &BoundedLinearExpression::vars) .def_property_readonly("coeffs", &BoundedLinearExpression::coeffs) @@ -831,6 +828,4 @@ PYBIND11_MODULE(swig_helper, m) { }); } // NOLINT(readability/fn_size) -} // namespace python -} // namespace sat -} // namespace operations_research +} // namespace operations_research::sat::python