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

@@ -546,8 +546,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;
@@ -4768,6 +4769,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"; }
 
@@ -4850,6 +4852,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);
 
@@ -5307,6 +5316,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<NestedSolveDecision*> nested_decisions_;
   int nested_decision_index_;
   RegularLimit* const limit_;
@@ -5462,6 +5472,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();
   }
@@ -5526,11 +5537,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 {

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 {
 
@@ -111,8 +113,29 @@ absl::StatusOr<MPSolutionResponse> HighsSolveProto(
       highs.changeColCost(column, obj_coeffs[column]);
     }
 
-    // TODO(user): Support variable names.
-    // TODO(user): Support hints.
+    // Variable names.
+    for (int v = 0; v < variable_size; ++v) {
+      const MPVariableProto& variable = model.variable(v);
+      std::string varname_str = "";
+      if (!variable.name().empty()) {
+        varname_str = variable.name();
+        highs.passColName(v, varname_str);
+      }
+    }
+
+    // Hints.
+    int num_hints = model.solution_hint().var_index_size();
+    if (num_hints > 0) {
+      std::vector<HighsInt> hint_index(0, num_hints);
+      std::vector<double> hint_value(0, num_hints);
+      for (int i = 0; i < num_hints; ++i) {
+        hint_index[i] = model.solution_hint().var_index(i);
+        hint_value[i] = model.solution_hint().var_value(i);
+      }
+      const int* hint_indices = &hint_index[0];
+      const double* hint_values = &hint_value[0];
+      highs.setSolution((HighsInt)num_hints, hint_indices, hint_values);
+    }
   }
 
   {
@@ -157,7 +180,16 @@ absl::StatusOr<MPSolutionResponse> HighsSolveProto(
           return response;
         }
       }
-      // TODO(user): Support constraint names.
+
+      // Constraint names.
+      for (int c = 0; c < model.constraint_size(); ++c) {
+        const MPConstraintProto& constraint = model.constraint(c);
+        std::string constraint_name_str = "";
+        if (!constraint.name().empty()) {
+          constraint_name_str = constraint.name();
+          highs.passRowName(c, constraint_name_str);
+        }
+      }
     }
 
     if (!model.general_constraint().empty()) {

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",
     ],

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));
       }

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<int64_t> domain_sizes = {2, 2, 2, 4};
   std::vector<std::vector<int64_t>> tuples = {
@@ -75,7 +213,7 @@ TEST(FullyCompressTuplesTest, BasicTest) {
   EXPECT_EQ(result, expected);
 }
 
-TEST(CompressTuplesTest, BasicTest2) {
+TEST(FullyCompressTuplesTest, BasicTest2) {
   const std::vector<int64_t> domain_sizes = {4, 4, 4, 4};
   std::vector<std::vector<int64_t>> 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<int64_t> domain_sizes = {4, 4, 4, 4};
   std::vector<std::vector<int64_t>> tuples = {
       {0, 0, 0, 0}, {0, 1, 0, 0}, {1, 0, 0, 0}, {1, 1, 0, 0},

ortools/sat/linear_programming_constraint.h

@@ -115,7 +115,6 @@ class ScatteredIntegerVector {
   util_intops::StrongVector<glop::ColIndex, IntegerValue> dense_vector_;
 };
 
-
 // A SAT constraint that enforces a set of linear inequality constraints on
 // integer variables using an LP solver.
 //