From 9860b59bd97be68dde5773c5a69e1bb20a6ee9af Mon Sep 17 00:00:00 2001 From: Laurent Perron Date: Thu, 24 Oct 2024 11:25:59 +0200 Subject: [PATCH] add tests -t CP-SAT table code; reindent, fix includes of highs proto solver; fix missing call in CP local search; fix link issue in CP-SAT test --- ortools/constraint_solver/local_search.cc | 17 ++- .../proto_solver/highs_proto_solver.cc | 6 +- ortools/sat/BUILD.bazel | 3 + ortools/sat/cp_model_presolve.cc | 2 + ortools/sat/cp_model_table_test.cc | 142 +++++++++++++++++- ortools/sat/linear_programming_constraint.h | 1 - 6 files changed, 163 insertions(+), 8 deletions(-) diff --git a/ortools/constraint_solver/local_search.cc b/ortools/constraint_solver/local_search.cc index 41d1f4614d..50e5abe321 100644 --- a/ortools/constraint_solver/local_search.cc +++ b/ortools/constraint_solver/local_search.cc @@ -562,8 +562,9 @@ bool PathOperator::IncrementPosition() { base_sibling_alternatives_[i] = 0; base_nodes_[i] = OldNext(base_nodes_[i]); if (iteration_parameters_.accept_path_end_base || - !IsPathEnd(base_nodes_[i])) + !IsPathEnd(base_nodes_[i])) { break; + } } calls_per_base_node_[i] = 0; base_alternatives_[i] = 0; @@ -4314,6 +4315,7 @@ class FindOneNeighbor : public DecisionBuilder { const RegularLimit* limit, LocalSearchFilterManager* filter_manager); ~FindOneNeighbor() override {} + void EnterSearch(); Decision* Next(Solver* solver) override; std::string DebugString() const override { return "FindOneNeighbor"; } @@ -4396,6 +4398,13 @@ FindOneNeighbor::FindOneNeighbor(Assignment* const assignment, } } +void FindOneNeighbor::EnterSearch() { + // Reset neighbor_found_ to false to ensure everything is properly + // synchronized at the beginning of the search. + neighbor_found_ = false; + last_synchronized_assignment_.reset(); +} + Decision* FindOneNeighbor::Next(Solver* const solver) { CHECK(nullptr != solver); @@ -4853,6 +4862,7 @@ class LocalSearch : public DecisionBuilder { LocalSearchOperator* const ls_operator_; DecisionBuilder* const first_solution_sub_decision_builder_; DecisionBuilder* const sub_decision_builder_; + FindOneNeighbor* find_neighbors_db_; std::vector nested_decisions_; int nested_decision_index_; RegularLimit* const limit_; @@ -5008,6 +5018,7 @@ Decision* LocalSearch::Next(Solver* const solver) { if (!has_started_) { nested_decision_index_ = 0; solver->SaveAndSetValue(&has_started_, true); + find_neighbors_db_->EnterSearch(); } else if (nested_decision_index_ < 0) { solver->Fail(); } @@ -5072,11 +5083,11 @@ void LocalSearch::PushFirstSolutionDecision(DecisionBuilder* first_solution) { void LocalSearch::PushLocalSearchDecision() { Solver* const solver = assignment_->solver(); - DecisionBuilder* find_neighbors = solver->RevAlloc( + find_neighbors_db_ = solver->RevAlloc( new FindOneNeighbor(assignment_, objective_, pool_, ls_operator_, sub_decision_builder_, limit_, filter_manager_)); nested_decisions_.push_back( - solver->RevAlloc(new NestedSolveDecision(find_neighbors, false))); + solver->RevAlloc(new NestedSolveDecision(find_neighbors_db_, false))); } class DefaultSolutionPool : public SolutionPool { diff --git a/ortools/linear_solver/proto_solver/highs_proto_solver.cc b/ortools/linear_solver/proto_solver/highs_proto_solver.cc index 5122a5652a..58d06edbcc 100644 --- a/ortools/linear_solver/proto_solver/highs_proto_solver.cc +++ b/ortools/linear_solver/proto_solver/highs_proto_solver.cc @@ -32,11 +32,13 @@ #include "absl/types/optional.h" #include "google/protobuf/repeated_field.h" #include "lp_data/HConst.h" +#include "lp_data/HighsInfo.h" #include "lp_data/HighsStatus.h" #include "ortools/base/timer.h" #include "ortools/linear_solver/linear_solver.pb.h" #include "ortools/linear_solver/model_validator.h" #include "ortools/util/lazy_mutable_copy.h" +#include "util/HighsInt.h" namespace operations_research { @@ -120,7 +122,7 @@ absl::StatusOr HighsSolveProto( const MPVariableProto& variable = model.variable(v); std::string varname_str = ""; if (!variable.name().empty()) { - varname_str = variable.name().c_str(); + varname_str = variable.name(); highs.passColName(v, varname_str); } } @@ -188,7 +190,7 @@ absl::StatusOr HighsSolveProto( const MPConstraintProto& constraint = model.constraint(c); std::string constraint_name_str = ""; if (!constraint.name().empty()) { - constraint_name_str = constraint.name().c_str(); + constraint_name_str = constraint.name(); highs.passRowName(c, constraint_name_str); } } diff --git a/ortools/sat/BUILD.bazel b/ortools/sat/BUILD.bazel index 30ef248596..851f3f2bbe 100644 --- a/ortools/sat/BUILD.bazel +++ b/ortools/sat/BUILD.bazel @@ -833,8 +833,11 @@ cc_test( deps = [ ":cp_model_cc_proto", ":cp_model_table", + ":model", + ":presolve_context", ":sat_parameters_cc_proto", "//ortools/base:gmock_main", + "//ortools/base:parse_test_proto", "@com_google_absl//absl/container:inlined_vector", "@com_google_absl//absl/types:span", ], diff --git a/ortools/sat/cp_model_presolve.cc b/ortools/sat/cp_model_presolve.cc index 3a1744f26b..cf1cdc0c05 100644 --- a/ortools/sat/cp_model_presolve.cc +++ b/ortools/sat/cp_model_presolve.cc @@ -11360,12 +11360,14 @@ void CpModelPresolver::ProcessVariableOnlyUsedInEncoding(int var) { const int encoding_lit = context_->GetOrCreateVarValueEncoding(var, v); const auto eq_it = value_to_equal_literals.find(v); if (eq_it != value_to_equal_literals.end()) { + absl::c_sort(eq_it->second); for (const int lit : eq_it->second) { context_->AddImplication(lit, encoding_lit); } } const auto neq_it = value_to_not_equal_literals.find(v); if (neq_it != value_to_not_equal_literals.end()) { + absl::c_sort(neq_it->second); for (const int lit : neq_it->second) { context_->AddImplication(lit, NegatedRef(encoding_lit)); } diff --git a/ortools/sat/cp_model_table_test.cc b/ortools/sat/cp_model_table_test.cc index 72c8ba0d37..0eb72f6825 100644 --- a/ortools/sat/cp_model_table_test.cc +++ b/ortools/sat/cp_model_table_test.cc @@ -20,13 +20,151 @@ #include "absl/types/span.h" #include "gtest/gtest.h" #include "ortools/base/gmock.h" +#include "ortools/base/parse_test_proto.h" #include "ortools/sat/cp_model.pb.h" +#include "ortools/sat/model.h" +#include "ortools/sat/presolve_context.h" #include "ortools/sat/sat_parameters.pb.h" namespace operations_research { namespace sat { namespace { +using ::google::protobuf::contrib::parse_proto::ParseTestProto; + +TEST(TableTest, DuplicateVariablesInTable) { + CpModelProto model_proto = ParseTestProto(R"pb( + variables { domain: [ 0, 2 ] } + variables { domain: [ 0, 2 ] } + constraints { + table { + exprs { vars: 0 coeffs: 1 } + exprs { vars: 0 coeffs: -1 } + exprs { vars: 1 coeffs: 1 } + exprs { vars: 1 coeffs: 1 } + values: [ 0, 0, 0, 0 ] + values: [ 1, -1, 0, 0 ] + values: [ 1, 0, 0, 0 ] + values: [ 1, -1, 1, 1 ] + values: [ 2, -2, 2, 2 ] + } + } + )pb"); + + Model model; + PresolveContext context(&model, &model_proto, nullptr); + context.InitializeNewDomains(); + + CanonicalizeTable(&context, model_proto.mutable_constraints(0)); + + const CpModelProto expected_presolved_model = ParseTestProto(R"pb( + variables { domain: [ 0, 2 ] } + variables { domain: [ 0, 2 ] } + constraints { + table { + exprs { vars: 0 coeffs: 1 } + exprs { vars: 1 coeffs: 1 } + values: [ 0, 0 ] + values: [ 1, 0 ] + values: [ 1, 1 ] + values: [ 2, 2 ] + } + } + )pb"); + EXPECT_THAT(expected_presolved_model, testing::EqualsProto(model_proto)); +} + +TEST(TableTest, RemoveFixedColumnsFromTable) { + CpModelProto model_proto = ParseTestProto(R"pb( + variables { domain: [ 0, 2 ] } + variables { domain: [ 1, 1 ] } + variables { domain: [ 0, 2 ] } + constraints { + table { + exprs { vars: 0 coeffs: 1 } + exprs { vars: 1 coeffs: 1 } + exprs { vars: 2 coeffs: 1 } + values: [ 0, 0, 0 ] + values: [ 0, 1, 0 ] + values: [ 1, 1, 1 ] + values: [ 1, 1, 2 ] + values: [ 2, 1, 2 ] + } + } + )pb"); + + Model model; + PresolveContext context(&model, &model_proto, nullptr); + context.InitializeNewDomains(); + + CanonicalizeTable(&context, model_proto.mutable_constraints(0)); + RemoveFixedColumnsFromTable(&context, model_proto.mutable_constraints(0)); + + const CpModelProto expected_presolved_model = ParseTestProto(R"pb( + variables { domain: [ 0, 2 ] } + variables { domain: [ 1, 1 ] } + variables { domain: [ 0, 2 ] } + constraints { + table { + exprs { vars: 0 coeffs: 1 } + exprs { vars: 2 coeffs: 1 } + values: [ 0, 0 ] + values: [ 1, 1 ] + values: [ 1, 2 ] + values: [ 2, 2 ] + } + } + )pb"); + EXPECT_THAT(expected_presolved_model, testing::EqualsProto(model_proto)); +} + +TEST(TableTest, NormalizeNoRemove) { + CpModelProto model_proto = ParseTestProto(R"pb( + variables { domain: [ 0, 2 ] } + variables { domain: [ 1, 2 ] } + variables { domain: [ 0, 2 ] } + constraints { + table { + exprs { vars: 0 coeffs: 1 } + exprs { vars: 1 coeffs: 1 } + exprs { vars: 2 coeffs: 1 } + values: [ 0, 0, 0 ] + values: [ 0, 1, 0 ] + values: [ 1, 1, 1 ] + values: [ 1, 1, 2 ] + values: [ 2, 1, 2 ] + } + } + )pb"); + + Model model; + PresolveContext context(&model, &model_proto, nullptr); + context.InitializeNewDomains(); + + CanonicalizeTable(&context, model_proto.mutable_constraints(0)); + + const CpModelProto expected_presolved_model = ParseTestProto(R"pb( + variables { domain: [ 0, 2 ] } + variables { domain: [ 1, 2 ] } + variables { domain: [ 0, 2 ] } + constraints { + table { + exprs { vars: 0 coeffs: 1 } + exprs { vars: 1 coeffs: 1 } + exprs { vars: 2 coeffs: 1 } + values: [ 0, 1, 0 ] + values: [ 1, 1, 1 ] + values: [ 1, 1, 2 ] + values: [ 2, 1, 2 ] + } + } + )pb"); + EXPECT_THAT(expected_presolved_model, testing::EqualsProto(model_proto)); + + RemoveFixedColumnsFromTable(&context, model_proto.mutable_constraints(0)); + EXPECT_THAT(expected_presolved_model, testing::EqualsProto(model_proto)); +} + TEST(CompressTuplesTest, OneAny) { const std::vector domain_sizes = {2, 2, 2, 4}; std::vector> tuples = { @@ -75,7 +213,7 @@ TEST(FullyCompressTuplesTest, BasicTest) { EXPECT_EQ(result, expected); } -TEST(CompressTuplesTest, BasicTest2) { +TEST(FullyCompressTuplesTest, BasicTest2) { const std::vector domain_sizes = {4, 4, 4, 4}; std::vector> tuples = { {0, 0, 0, 0}, @@ -89,7 +227,7 @@ TEST(CompressTuplesTest, BasicTest2) { EXPECT_EQ(result, expected); } -TEST(CompressTuplesTest, BasicTest3) { +TEST(FullyCompressTuplesTest, BasicTest3) { const std::vector domain_sizes = {4, 4, 4, 4}; std::vector> tuples = { {0, 0, 0, 0}, {0, 1, 0, 0}, {1, 0, 0, 0}, {1, 1, 0, 0}, diff --git a/ortools/sat/linear_programming_constraint.h b/ortools/sat/linear_programming_constraint.h index 5364b871c2..093e68de9a 100644 --- a/ortools/sat/linear_programming_constraint.h +++ b/ortools/sat/linear_programming_constraint.h @@ -115,7 +115,6 @@ class ScatteredIntegerVector { util_intops::StrongVector dense_vector_; }; - // A SAT constraint that enforces a set of linear inequality constraints on // integer variables using an LP solver. //