Skip to content

Commit b29f407

Browse files
committed
[CP-SAT] use precedences in completion time cuts; improve glue clause sharing
1 parent aac608e commit b29f407

13 files changed

Lines changed: 549 additions & 572 deletions

ortools/sat/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2893,6 +2893,7 @@ cc_library(
28932893
":linear_constraint",
28942894
":linear_constraint_manager",
28952895
":model",
2896+
":precedences",
28962897
":sat_base",
28972898
":sat_solver",
28982899
":scheduling_helpers",
@@ -3548,6 +3549,7 @@ cc_library(
35483549
"//ortools/util:strong_integers",
35493550
"//ortools/util:time_limit",
35503551
"@abseil-cpp//absl/container:flat_hash_set",
3552+
"@abseil-cpp//absl/container:inlined_vector",
35513553
"@abseil-cpp//absl/log",
35523554
"@abseil-cpp//absl/log:check",
35533555
"@abseil-cpp//absl/log:vlog_is_on",

ortools/sat/cp_model_solver.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1127,7 +1127,9 @@ class FullProblemSolver : public SubSolver {
11271127
// Note that this is done after the loading, so we will never export
11281128
// problem clauses.
11291129
if (shared_->clauses != nullptr) {
1130-
const int id = shared_->clauses->RegisterNewId();
1130+
const int id = shared_->clauses->RegisterNewId(
1131+
/*may_terminate_early=*/stop_at_first_solution_ &&
1132+
local_model_.GetOrCreate<CpModelProto>()->has_objective());
11311133
shared_->clauses->SetWorkerNameForId(id, local_model_.Name());
11321134

11331135
RegisterClausesLevelZeroImport(id, shared_->clauses.get(),

ortools/sat/cp_model_solver_helpers.cc

Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -953,27 +953,31 @@ void RegisterClausesExport(int id, SharedClausesManager* shared_clauses_manager,
953953
if (!model->GetOrCreate<SatParameters>()->share_glue_clauses()) {
954954
return;
955955
}
956-
auto* clause_stream = shared_clauses_manager->GetClauseStream(id);
957-
const int max_lbd =
958-
model->GetOrCreate<SatParameters>()->clause_cleanup_lbd_bound();
959-
// Note that this callback takes no global locks, everything operates on this
960-
// worker's own clause stream, whose lock is only used by this worker, and
961-
// briefly when generating a batch in SharedClausesManager::Synchronize().
962-
auto share_clause = [mapping, clause_stream, max_lbd,
963-
clause = std::vector<int>()](
956+
const double share_interval =
957+
model->GetOrCreate<SatParameters>()->share_glue_clauses_dtime();
958+
auto* clause_stream = model->GetOrCreate<UniqueClauseStream>();
959+
auto* time_limit = model->GetOrCreate<TimeLimit>();
960+
auto share_clause = [mapping, clause_stream, time_limit, id,
961+
shared_clauses_manager, share_interval,
962+
next_batch_dtime = -1.0, clause = std::vector<int>()](
964963
int lbd, absl::Span<const Literal> literals) mutable {
965-
if (lbd <= 0 || lbd > max_lbd ||
966-
!clause_stream->CanAccept(literals.size(), lbd)) {
967-
return;
964+
if (literals.size() >= UniqueClauseStream::kMinClauseSize &&
965+
literals.size() <= UniqueClauseStream::kMaxClauseSize) {
966+
clause.clear();
967+
for (const Literal& lit : literals) {
968+
const int var =
969+
mapping->GetProtoVariableFromBooleanVariable(lit.Variable());
970+
if (var == -1) return;
971+
clause.push_back(lit.IsPositive() ? var : NegatedRef(var));
972+
}
973+
clause_stream->Add(clause, lbd);
968974
}
969-
clause.clear();
970-
for (const Literal& lit : literals) {
971-
const int var =
972-
mapping->GetProtoVariableFromBooleanVariable(lit.Variable());
973-
if (var == -1) return;
974-
clause.push_back(lit.IsPositive() ? var : NegatedRef(var));
975+
const double elapsed_dtime = time_limit->GetElapsedDeterministicTime();
976+
if (next_batch_dtime < 0) next_batch_dtime = elapsed_dtime + share_interval;
977+
if (elapsed_dtime >= next_batch_dtime) {
978+
shared_clauses_manager->AddBatch(id, clause_stream->NextBatch());
979+
next_batch_dtime = elapsed_dtime + share_interval;
975980
}
976-
clause_stream->Add(clause);
977981
};
978982
model->GetOrCreate<ClauseManager>()->SetAddClauseCallback(
979983
std::move(share_clause));
@@ -994,16 +998,16 @@ int RegisterClausesLevelZeroImport(int id,
994998
auto* implications = model->GetOrCreate<BinaryImplicationGraph>();
995999
const bool share_glue_clauses =
9961000
model->GetOrCreate<SatParameters>()->share_glue_clauses();
1001+
auto* clause_stream =
1002+
share_glue_clauses ? model->GetOrCreate<UniqueClauseStream>() : nullptr;
9971003
const bool minimize_shared_clauses =
9981004
model->GetOrCreate<SatParameters>()->minimize_shared_clauses();
999-
auto* clause_stream = share_glue_clauses
1000-
? shared_clauses_manager->GetClauseStream(id)
1001-
: nullptr;
10021005
auto* clause_manager = model->GetOrCreate<ClauseManager>();
10031006
const auto& import_level_zero_clauses = [shared_clauses_manager, id, mapping,
10041007
sat_solver, implications,
1005-
clause_stream, clause_manager,
1006-
minimize_shared_clauses]() {
1008+
minimize_shared_clauses,
1009+
clause_stream,
1010+
clause_manager]() mutable {
10071011
std::vector<std::pair<int, int>> new_binary_clauses;
10081012
shared_clauses_manager->GetUnseenBinaryClauses(id, &new_binary_clauses);
10091013
implications->EnableSharing(false);
@@ -1020,28 +1024,27 @@ int RegisterClausesLevelZeroImport(int id,
10201024
int new_clauses = 0;
10211025
std::array<Literal, UniqueClauseStream::kMaxClauseSize> local_clause;
10221026
sat_solver->EnsureNewClauseIndexInitialized();
1023-
// Temporarily disable clause sharing so we don't immediately re-export the
1024-
// clauses we just imported.
1027+
// Temporarily disable clause sharing.
10251028
auto callback = clause_manager->TakeAddClauseCallback();
1026-
for (const absl::Span<const int> shared_clause :
1027-
shared_clauses_manager->GetUnseenClauses(id)) {
1028-
// Check this clause was not already learned by this worker.
1029-
// We can delete the fingerprint because we should not learn an identical
1030-
// clause, and the global stream will not emit the same clause while any
1031-
// worker hasn't consumed this clause (and thus also shouldn't relearn the
1032-
// clause).
1033-
if (clause_stream->Delete(shared_clause)) continue;
1034-
for (int i = 0; i < shared_clause.size(); ++i) {
1035-
local_clause[i] = mapping->Literal(shared_clause[i]);
1036-
}
1037-
if (!sat_solver->AddProblemClause(
1038-
absl::MakeSpan(local_clause).subspan(0, shared_clause.size()))) {
1039-
return false;
1029+
while (true) {
1030+
auto batch = shared_clauses_manager->GetUnseenClauses(id);
1031+
if (batch.empty()) break;
1032+
for (int clause_index = 0; clause_index < batch.size(); ++clause_index) {
1033+
const absl::Span<const int>& shared_clause = batch[clause_index];
1034+
// Check this clause was not already learned by this worker.
1035+
if (!clause_stream->BlockClause(shared_clause)) continue;
1036+
++new_clauses;
1037+
for (int i = 0; i < shared_clause.size(); ++i) {
1038+
local_clause[i] = mapping->Literal(shared_clause[i]);
1039+
}
1040+
if (!sat_solver->AddProblemClause(
1041+
absl::MakeSpan(local_clause)
1042+
.subspan(0, shared_clause.size()))) {
1043+
return false;
1044+
}
10401045
}
1041-
++new_clauses;
10421046
}
10431047
clause_manager->SetAddClauseCallback(std::move(callback));
1044-
clause_stream->RemoveWorstClauses();
10451048
if (minimize_shared_clauses && new_clauses > 0) {
10461049
// The new clauses may be subsumed, so try to minimize them to reduce
10471050
// overhead of sharing.
@@ -2110,8 +2113,7 @@ SharedClasses::SharedClasses(const CpModelProto* proto, Model* global_model)
21102113
!params.interleave_search() || params.num_workers() <= 1;
21112114
response->SetSynchronizationMode(always_synchronize);
21122115
if (params.share_binary_clauses() && params.num_workers() > 1) {
2113-
clauses = std::make_unique<SharedClausesManager>(always_synchronize,
2114-
absl::Seconds(1));
2116+
clauses = std::make_unique<SharedClausesManager>(always_synchronize);
21152117
}
21162118
}
21172119

ortools/sat/cp_model_solver_test.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ TEST(LoadCpModelTest, PureSatProblem) {
8787
TEST(LoadCpModelTest, PureSatProblemWithLimit) {
8888
const CpModelProto model_proto = Random3SatProblem(500);
8989
LOG(INFO) << CpModelStats(model_proto);
90-
Model model;
91-
model.Add(NewSatParameters("max_deterministic_time:0.00001"));
92-
const CpSolverResponse response = SolveCpModel(model_proto, &model);
90+
SatParameters params;
91+
params.set_max_deterministic_time(0.00001);
92+
const CpSolverResponse response = SolveWithParameters(model_proto, params);
9393
EXPECT_EQ(response.status(), CpSolverStatus::UNKNOWN);
9494
LOG(INFO) << CpSolverResponseStats(response);
9595
}
@@ -193,7 +193,8 @@ TEST(LoadCpModelTest, SimpleCumulative) {
193193
}
194194

195195
TEST(SolverCpModelTest, EmptyModel) {
196-
const CpModelProto cp_model = ParseTestProto("solution_hint {}");
196+
CpModelProto cp_model;
197+
cp_model.mutable_solution_hint();
197198

198199
SatParameters params;
199200
params.set_debug_crash_if_presolve_breaks_hint(true);
@@ -329,6 +330,7 @@ TEST(SolveCpModelTest, TrivialModelWithCore) {
329330
response.solution().end())));
330331
}
331332

333+
#if !defined(__EMBEDDED_PLATFORM__)
332334
TEST(SolveCpModelTest, TrivialLinearTranslatedModel) {
333335
const CpModelProto model_proto = ParseTestProto(R"pb(
334336
variables { domain: -10 domain: 10 }
@@ -4803,6 +4805,7 @@ TEST(PresolveCpModelTest, CumulativeBug4) {
48034805
response = SolveWithParameters(cp_model, params);
48044806
EXPECT_EQ(response.status(), CpSolverStatus::OPTIMAL);
48054807
}
4808+
#endif // !defined(__EMBEDDED_PLATFORM__)
48064809

48074810
} // namespace
48084811
} // namespace sat

ortools/sat/diffn.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
#include "absl/log/vlog_is_on.h"
3333
#include "absl/numeric/bits.h"
3434
#include "absl/types/span.h"
35-
#include "ortools/base/stl_util.h"
35+
// #include "ortools/base/stl_util.h"
3636
#include "ortools/sat/2d_mandatory_overlap_propagator.h"
3737
#include "ortools/sat/2d_orthogonal_packing.h"
3838
#include "ortools/sat/2d_try_edge_propagator.h"

ortools/sat/parameters_validation.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ std::string ValidateParameters(const SatParameters& params) {
8787
TEST_IS_FINITE(relative_gap_limit);
8888
TEST_IS_FINITE(restart_dl_average_ratio);
8989
TEST_IS_FINITE(restart_lbd_average_ratio);
90+
TEST_IS_FINITE(share_glue_clauses_dtime);
9091
TEST_IS_FINITE(shared_tree_open_leaves_per_worker);
9192
TEST_IS_FINITE(shaving_deterministic_time_in_probing_search);
9293
TEST_IS_FINITE(shaving_search_deterministic_time);
@@ -156,6 +157,7 @@ std::string ValidateParameters(const SatParameters& params) {
156157
TEST_NON_NEGATIVE(presolve_probing_deterministic_time_limit);
157158
TEST_NON_NEGATIVE(probing_deterministic_time_limit);
158159
TEST_NON_NEGATIVE(symmetry_detection_deterministic_time_limit);
160+
TEST_POSITIVE(share_glue_clauses_dtime);
159161

160162
if (params.enumerate_all_solutions() &&
161163
(params.num_search_workers() > 1 || params.num_workers() > 1)) {

ortools/sat/sat_parameters.proto

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ option java_multiple_files = true;
2424
// Contains the definitions for all the sat algorithm parameters and their
2525
// default values.
2626
//
27-
// NEXT TAG: 322
27+
// NEXT TAG: 323
2828
message SatParameters {
2929
// In some context, like in a portfolio of search, it makes sense to name a
3030
// given parameters set for logging purpose.
@@ -705,6 +705,9 @@ message SatParameters {
705705
// are imported.
706706
optional bool minimize_shared_clauses = 300 [default = true];
707707

708+
// The amount of dtime between each export of shared glue clauses.
709+
optional double share_glue_clauses_dtime = 322 [default = 1.0];
710+
708711
// ==========================================================================
709712
// Debugging parameters
710713
// ==========================================================================

0 commit comments

Comments
 (0)