From 6543b5041241ea4194c51e2176aaf698e4570c73 Mon Sep 17 00:00:00 2001 From: Laurent Perron Date: Tue, 9 May 2023 17:50:26 +0200 Subject: [PATCH] [CP-SAT] fix bug with evaluations and violation_ls --- ortools/sat/constraint_violation.cc | 17 +++++--- ortools/sat/constraint_violation.h | 17 ++++++-- ortools/sat/feasibility_jump.cc | 67 +++++++++++++++++------------ ortools/sat/feasibility_jump.h | 2 +- 4 files changed, 65 insertions(+), 38 deletions(-) diff --git a/ortools/sat/constraint_violation.cc b/ortools/sat/constraint_violation.cc index 0609f2bc92..927978987e 100644 --- a/ortools/sat/constraint_violation.cc +++ b/ortools/sat/constraint_violation.cc @@ -562,12 +562,15 @@ int64_t LinearIncrementalEvaluator::Violation(int c) const { } bool LinearIncrementalEvaluator::IsViolated(int c) const { - return Violation(c) > 0; + DCHECK_EQ(is_violated_[c], Violation(c) > 0); + return is_violated_[c]; } -void LinearIncrementalEvaluator::ReduceBounds(int c, int64_t lb, int64_t ub) { +bool LinearIncrementalEvaluator::ReduceBounds(int c, int64_t lb, int64_t ub) { + if (domains_[c].Min() >= lb && domains_[c].Max() <= ub) return false; domains_[c] = domains_[c].IntersectionWith(Domain(lb, ub)); distances_[c] = domains_[c].Distance(activities_[c]); + return true; } double LinearIncrementalEvaluator::WeightedViolation( @@ -1398,14 +1401,16 @@ void LsEvaluator::CompileConstraintsAndObjective() { linear_evaluator_.PrecomputeCompactView(); } -void LsEvaluator::ReduceObjectiveBounds(int64_t lb, int64_t ub) { - if (!model_.has_objective()) return; - linear_evaluator_.ReduceBounds(/*c=*/0, lb, ub); +bool LsEvaluator::ReduceObjectiveBounds(int64_t lb, int64_t ub) { + if (!model_.has_objective()) return false; + return linear_evaluator_.ReduceBounds(/*c=*/0, lb, ub); } -void LsEvaluator::ComputeInitialViolations(absl::Span solution) { +void LsEvaluator::OverwriteCurrentSolution(absl::Span solution) { current_solution_.assign(solution.begin(), solution.end()); +} +void LsEvaluator::ComputeAllViolations() { // Linear constraints. linear_evaluator_.ComputeInitialActivities(current_solution_); diff --git a/ortools/sat/constraint_violation.h b/ortools/sat/constraint_violation.h index 7c7e36f1dc..d7490eef90 100644 --- a/ortools/sat/constraint_violation.h +++ b/ortools/sat/constraint_violation.h @@ -86,7 +86,8 @@ class LinearIncrementalEvaluator { bool VarIsConsistent(int var) const; // Intersect constraint bounds with [lb..ub]. - void ReduceBounds(int c, int64_t lb, int64_t ub); + // It returns true if a reduction of the domain took place. + bool ReduceBounds(int c, int64_t lb, int64_t ub); // Model getters. int num_constraints() const { return num_constraints_; } @@ -277,10 +278,14 @@ class LsEvaluator { bool ModelIsSupported() const { return model_is_supported_; } // Intersects the domain of the objective with [lb..ub]. - void ReduceObjectiveBounds(int64_t lb, int64_t ub); + // It returns true if a reduction of the domain took place. + bool ReduceObjectiveBounds(int64_t lb, int64_t ub); - // Sets the current solution, and computes violations for each constraint. - void ComputeInitialViolations(absl::Span solution); + // Overwrites the current solution. + void OverwriteCurrentSolution(absl::Span solution); + + // Computes the violations of all constraints. + void ComputeAllViolations(); // Recompute the violations of non linear constraints. void UpdateAllNonLinearViolations(); @@ -337,6 +342,10 @@ class LsEvaluator { return current_solution_; } + std::vector* mutable_current_solution() { + return ¤t_solution_; + } + private: void CompileConstraintsAndObjective(); void CompileOneConstraint(const ConstraintProto& ct_proto); diff --git a/ortools/sat/feasibility_jump.cc b/ortools/sat/feasibility_jump.cc index b7bdf167b2..53c0ff0120 100644 --- a/ortools/sat/feasibility_jump.cc +++ b/ortools/sat/feasibility_jump.cc @@ -144,15 +144,18 @@ int64_t RandomValueNearValue(const Domain& domain, int64_t value, } // namespace -void FeasibilityJumpSolver::RestartFromDefaultSolution() { +void FeasibilityJumpSolver::ResetCurrentSolution() { const int num_variables = linear_model_->model_proto().variables().size(); const double default_value_probability = 1.0 - params_.feasibility_jump_var_randomization_probability(); const double range_ratio = params_.feasibility_jump_var_perburbation_range_ratio(); + std::vector& solution = *evaluator_->mutable_current_solution(); + + // Resize the solution if needed. + solution.resize(num_variables); // Starts with values closest to zero. - std::vector solution(num_variables); for (int var = 0; var < num_variables; ++var) { if (var_domains_[var].IsFixed()) { solution[var] = var_domains_[var].FixedValue(); @@ -196,7 +199,6 @@ void FeasibilityJumpSolver::RestartFromDefaultSolution() { } } } - evaluator_->ComputeInitialViolations(solution); // Starts with weights at one. weights_.assign(evaluator_->NumEvaluatorConstraints(), 1.0); @@ -208,7 +210,7 @@ void FeasibilityJumpSolver::PerturbateCurrentSolution() { params_.feasibility_jump_var_randomization_probability(); const double perturbation_ratio = params_.feasibility_jump_var_perburbation_range_ratio(); - std::vector solution = evaluator_->current_solution(); + std::vector& solution = *evaluator_->mutable_current_solution(); for (int var = 0; var < num_variables; ++var) { if (var_domains_[var].IsFixed()) continue; if (absl::Bernoulli(random_, pertubation_probability)) { @@ -216,10 +218,6 @@ void FeasibilityJumpSolver::PerturbateCurrentSolution() { perturbation_ratio, random_); } } - evaluator_->ComputeInitialViolations(solution); - - // Starts with weights at one. - weights_.assign(evaluator_->NumEvaluatorConstraints(), 1.0); } std::string FeasibilityJumpSolver::OneLineStats() const { @@ -246,7 +244,7 @@ std::string FeasibilityJumpSolver::OneLineStats() const { // Improving jumps and infeasible constraints. const int num_infeasible_cts = evaluator_->NumInfeasibleConstraints(); const std::string non_solution_str = - good_jumps_.empty() && num_infeasible_cts == 0 + num_infeasible_cts == 0 ? "" : absl::StrCat(" #good_lin_moves:", FormatCounter(good_jumps_.size()), " #inf_cts:", @@ -267,6 +265,9 @@ std::function FeasibilityJumpSolver::GenerateTask(int64_t /*task_id*/) { // to scan the whole model, so we want to do this part in parallel. if (!is_initialized_) Initialize(); + bool should_recompute_violations = false; + bool reset_weights = false; + // In incomplete mode, query the starting solution for the shared response // manager. if (type() == SubSolver::INCOMPLETE) { @@ -277,10 +278,9 @@ std::function FeasibilityJumpSolver::GenerateTask(int64_t /*task_id*/) { const SharedSolutionRepository::Solution solution = repo.GetRandomBiasedSolution(random_); if (solution.rank < last_solution_rank_) { - evaluator_->ComputeInitialViolations(solution.variable_values); - - // Reset weights for each new solution. - weights_.assign(evaluator_->NumEvaluatorConstraints(), 1.0); + evaluator_->OverwriteCurrentSolution(solution.variable_values); + should_recompute_violations = true; + reset_weights = true; // Update last solution rank. last_solution_rank_ = solution.rank; @@ -294,6 +294,8 @@ std::function FeasibilityJumpSolver::GenerateTask(int64_t /*task_id*/) { params_.violation_ls_perturbation_frequency(); ++num_perturbations_; PerturbateCurrentSolution(); + should_recompute_violations = true; + reset_weights = true; } } else { // Restart? Note that we always "restart" the first time. @@ -303,11 +305,15 @@ std::function FeasibilityJumpSolver::GenerateTask(int64_t /*task_id*/) { num_weight_updates_ >= update_restart_threshold_) { if (num_restarts_ == 0 || params_.feasibility_jump_enable_restarts()) { ++num_restarts_; - RestartFromDefaultSolution(); + ResetCurrentSolution(); + should_recompute_violations = true; + reset_weights = true; } else if (params_.feasibility_jump_var_randomization_probability() > 0.0) { - ++num_perturbations_; PerturbateCurrentSolution(); + ++num_perturbations_; + should_recompute_violations = true; + reset_weights = true; } // We use luby restart with a base of 1 deterministic unit. @@ -329,7 +335,9 @@ std::function FeasibilityJumpSolver::GenerateTask(int64_t /*task_id*/) { const IntegerValue lb = shared_response_->GetInnerObjectiveLowerBound(); const IntegerValue ub = shared_response_->GetInnerObjectiveUpperBound(); if (ub < lb) return; // Search is finished. - evaluator_->ReduceObjectiveBounds(lb.value(), ub.value()); + if (evaluator_->ReduceObjectiveBounds(lb.value(), ub.value())) { + should_recompute_violations = true; + } } // Update the variable domains with the last information. @@ -344,21 +352,26 @@ std::function FeasibilityJumpSolver::GenerateTask(int64_t /*task_id*/) { // Checks the current solution is compatible with updated domains. { - std::vector fixed_solution = evaluator_->current_solution(); - // Make sure the solution is within the potentially updated domain. - bool some_values_have_changed = false; - for (int var = 0; var < fixed_solution.size(); ++var) { - const int64_t old_value = evaluator_->current_solution()[var]; - fixed_solution[var] = - var_domains_[var].ClosestValue(fixed_solution[var]); - some_values_have_changed |= old_value != fixed_solution[var]; - } - if (some_values_have_changed) { - evaluator_->ComputeInitialViolations(fixed_solution); + std::vector& current_solution = + *evaluator_->mutable_current_solution(); + for (int var = 0; var < current_solution.size(); ++var) { + const int64_t old_value = current_solution[var]; + const int64_t new_value = var_domains_[var].ClosestValue(old_value); + if (new_value != old_value) { + current_solution[var] = new_value; + should_recompute_violations = true; + } } } + if (should_recompute_violations) { + evaluator_->ComputeAllViolations(); + } + if (reset_weights) { + weights_.assign(evaluator_->NumEvaluatorConstraints(), 1.0); + } + // Search for feasible solution. if (DoSomeLinearIterations() && DoSomeGeneralIterations()) { // Checks for infeasibility induced by the non supported constraints. diff --git a/ortools/sat/feasibility_jump.h b/ortools/sat/feasibility_jump.h index f199bc1b20..0d8e9f42ee 100644 --- a/ortools/sat/feasibility_jump.h +++ b/ortools/sat/feasibility_jump.h @@ -93,7 +93,7 @@ class FeasibilityJumpSolver : public SubSolver { private: void Initialize(); - void RestartFromDefaultSolution(); + void ResetCurrentSolution(); void PerturbateCurrentSolution(); std::string OneLineStats() const;