From 641a5ef2ead9fe2ea3e83ccbc77ad31832912643 Mon Sep 17 00:00:00 2001 From: Laurent Perron Date: Mon, 22 Apr 2024 16:26:46 +0200 Subject: [PATCH] [CP-SAT] remove randomization from linear propagation --- ortools/sat/linear_propagation.cc | 54 +++++++++++++------------- ortools/sat/linear_propagation.h | 63 ++++++++++++++++++++----------- 2 files changed, 68 insertions(+), 49 deletions(-) diff --git a/ortools/sat/linear_propagation.cc b/ortools/sat/linear_propagation.cc index 4b490d65e6..95dd45336c 100644 --- a/ortools/sat/linear_propagation.cc +++ b/ortools/sat/linear_propagation.cc @@ -435,11 +435,33 @@ void LinearPropagator::SetLevel(int level) { previous_level_ = level; } -void LinearPropagator::OnVariableChange(IntegerVariable var, IntegerValue lb) { +void LinearPropagator::SetPropagatedBy(IntegerVariable var, int id) { + int& ref_id = propagated_by_[var]; + if (ref_id == id) return; + + propagated_by_was_set_.Set(var); + + DCHECK_GE(var, 0); + DCHECK_LT(var, propagated_by_.size()); + if (ref_id != -1) { + DCHECK_GE(ref_id, 0); + DCHECK_LT(ref_id, id_to_propagation_count_.size()); + id_to_propagation_count_[ref_id]--; + } + ref_id = id; + if (id != -1) id_to_propagation_count_[id]++; +} + +void LinearPropagator::OnVariableChange(IntegerVariable var, IntegerValue lb, + int id) { + // If no constraint use this var, we just ignore it. + const int size = var_to_constraint_ids_[var].size(); + if (size == 0) return; + + SetPropagatedBy(var, id); order_.UpdateBound(var, lb); Bitset64::View in_queue = in_queue_.view(); - time_limit_->AdvanceDeterministicTime( - static_cast(var_to_constraint_ids_[var].size()) * 1e-9); + time_limit_->AdvanceDeterministicTime(static_cast(size) * 1e-9); for (const int id : var_to_constraint_ids_[var]) { if (in_queue[id]) continue; in_queue.Set(id); @@ -454,8 +476,7 @@ bool LinearPropagator::Propagate() { // is handled by PropagateOneConstraint(). for (const IntegerVariable var : modified_vars_.PositionsSetAtLeastOnce()) { if (var >= var_to_constraint_ids_.size()) continue; - SetPropagatedBy(var, -1); - OnVariableChange(var, integer_trail_->LowerBound(var)); + OnVariableChange(var, integer_trail_->LowerBound(var), -1); } // Cleanup. @@ -947,11 +968,9 @@ bool LinearPropagator::PropagateOneConstraint(int id) { const IntegerVariable next_var = NegationOf(var); if (actual_ub < new_ub) { // Was pushed further due to hole. We clear it. - SetPropagatedBy(next_var, -1); - OnVariableChange(next_var, -actual_ub); + OnVariableChange(next_var, -actual_ub, -1); } else if (actual_ub == new_ub) { - SetPropagatedBy(next_var, id); - OnVariableChange(next_var, -actual_ub); + OnVariableChange(next_var, -actual_ub, id); // We reorder them first. std::swap(vars[i], vars[num_pushed]); @@ -1258,23 +1277,6 @@ void LinearPropagator::AddToQueueIfNeeded(int id) { propagation_queue_.push_back(id); } -void LinearPropagator::SetPropagatedBy(IntegerVariable var, int id) { - int& ref_id = propagated_by_[var]; - if (ref_id == id) return; - - propagated_by_was_set_.Set(var); - - DCHECK_GE(var, 0); - DCHECK_LT(var, propagated_by_.size()); - if (ref_id != -1) { - DCHECK_GE(ref_id, 0); - DCHECK_LT(ref_id, id_to_propagation_count_.size()); - id_to_propagation_count_[ref_id]--; - } - ref_id = id; - if (id != -1) id_to_propagation_count_[id]++; -} - void LinearPropagator::ClearPropagatedBy() { // To be sparse, we use the fact that each node with a parent must be in // modified_vars_. diff --git a/ortools/sat/linear_propagation.h b/ortools/sat/linear_propagation.h index a806f9fad2..1932ab499c 100644 --- a/ortools/sat/linear_propagation.h +++ b/ortools/sat/linear_propagation.h @@ -142,7 +142,7 @@ class EnforcementPropagator : public SatPropagator { // Helper class to decide on the constraint propagation order. // -// Each constraint might push some variables which might in turn render other +// Each constraint might push some variables which might in turn make other // constraint tighter. In general, it seems better to make sure we push first // constraints that are not affected by other variables and delay the // propagation of constraint that we know will become tigher. @@ -179,11 +179,9 @@ class ConstraintPropagationOrder { var_to_id_[var] = id; if (!in_ids_[id]) { in_ids_.Set(id); - ids_.push_back(id); - // randomize starting pos? - const int random_pos = absl::Uniform(*random_, 0, ids_.size()); - std::swap(ids_[random_pos], ids_.back()); + // All new ids are likely not in a cycle, we want to test them first. + ids_.push_front(id); } } @@ -207,33 +205,51 @@ class ConstraintPropagationOrder { int NextId() { if (ids_.empty()) return -1; - int best_pos = 0; + int best_id = 0; + int best_num_vars = 0; int best_degree = std::numeric_limits::max(); const int size = ids_.size(); const auto var_has_entry = var_has_entry_.const_view(); for (int i = 0; i < size; ++i) { - const int pos = (i + start_) % size; - const int id = ids_[pos]; + const int id = ids_.front(); + ids_.pop_front(); DCHECK(in_ids_[id]); int degree = 0; - for (const IntegerVariable var : id_to_vars_func_(id)) { + absl::Span vars = id_to_vars_func_(id); + for (const IntegerVariable var : vars) { if (var_has_entry[var]) ++degree; } - if (degree < best_degree) { + // We select the min-degree and prefer lower constraint size. + // We however stop at the first degree zero. + if (degree < best_degree || + (degree == best_degree && vars.size() < best_num_vars)) { best_degree = degree; - best_pos = pos; - if (best_degree == 0) break; + best_num_vars = vars.size(); + best_id = id; + if (best_degree == 0) { + in_ids_.Clear(best_id); + return best_id; + } } - } - std::swap(ids_[best_pos], ids_.back()); - start_ = best_pos; - const int result = ids_.back(); - ids_.pop_back(); - in_ids_.Clear(result); - return result; + ids_.push_back(id); + } + + // We didn't find any degree zero, we scanned the whole queue. + // Extract best_id while keeping the order stable. + // + // We tried to randomize the order, it does add more variance but also seem + // worse overall. + int new_size = 0; + for (const int id : ids_) { + if (id == best_id) continue; + ids_[new_size++] = id; + } + ids_.resize(new_size); + in_ids_.Clear(best_id); + return best_id; } void UpdateBound(IntegerVariable var, IntegerValue lb) { @@ -267,10 +283,10 @@ class ConstraintPropagationOrder { absl::StrongVector var_to_pos_; std::vector to_clear_; - // Set/vector of constraint to be propagated. + // Set/queue of constraints to be propagated. int start_ = 0; Bitset64 in_ids_; - std::vector ids_; + std::deque ids_; }; // This is meant to supersede both IntegerSumLE and the PrecedencePropagator. @@ -323,8 +339,9 @@ class LinearPropagator : public PropagatorInterface, ReversibleInterface { absl::Span GetCoeffs(const ConstraintInfo& info); absl::Span GetVariables(const ConstraintInfo& info); - // Called when the lower bound of a variable changed. - void OnVariableChange(IntegerVariable var, IntegerValue lb); + // Called when the lower bound of a variable changed. The id is the constraint + // id that caused this change or -1 if it comes from an external source. + void OnVariableChange(IntegerVariable var, IntegerValue lb, int id); // Returns false on conflict. ABSL_MUST_USE_RESULT bool PropagateOneConstraint(int id);