diff --git a/ortools/sat/cp_model_lns.h b/ortools/sat/cp_model_lns.h index dc36e83415..fbae076883 100644 --- a/ortools/sat/cp_model_lns.h +++ b/ortools/sat/cp_model_lns.h @@ -47,9 +47,10 @@ struct Neighborhood { static constexpr int kDefaultArenaSizePerVariable = 128; explicit Neighborhood(int num_variables_hint = 10) - : arena_buffer(kDefaultArenaSizePerVariable * num_variables_hint), - arena(std::make_unique(arena_buffer.data(), - arena_buffer.size())), + : arena(std::make_unique( + google::protobuf::ArenaOptions( + {.start_block_size = static_cast( + kDefaultArenaSizePerVariable * num_variables_hint)}))), delta(*google::protobuf::Arena::Create(arena.get())) {} // True if neighborhood generator was able to generate a neighborhood. @@ -66,7 +67,6 @@ struct Neighborhood { // The delta will contains all variables from the initial model, potentially // with updated domains. // It can contains new variables and new constraints, and solution hinting. - std::vector arena_buffer; std::unique_ptr arena; CpModelProto& delta; diff --git a/ortools/sat/cp_model_presolve.cc b/ortools/sat/cp_model_presolve.cc index 7f0f37a505..548ea23b1e 100644 --- a/ortools/sat/cp_model_presolve.cc +++ b/ortools/sat/cp_model_presolve.cc @@ -9609,16 +9609,17 @@ void CpModelPresolver::DetectDuplicateConstraintsWithDifferentEnforcements( if (context_->VariableWithCostIsUniqueAndRemovable(a) && context_->VariableWithCostIsUniqueAndRemovable(b)) { // Both these case should be presolved before, but it is easy to deal with - // if we encounter them here in some corner cases. + // if we encounter them here in some corner cases. And the code after + // 'continue' uses this, in particular to update the hint. bool skip = false; if (RefIsPositive(a) == context_->ObjectiveCoeff(PositiveRef(a)) > 0) { context_->UpdateRuleStats("duplicate: dual fixing enforcement."); - if (!context_->SetLiteralToFalse(a)) return; + if (!context_->SetLiteralAndHintToFalse(a)) return; skip = true; } if (RefIsPositive(b) == context_->ObjectiveCoeff(PositiveRef(b)) > 0) { context_->UpdateRuleStats("duplicate: dual fixing enforcement."); - if (!context_->SetLiteralToFalse(b)) return; + if (!context_->SetLiteralAndHintToFalse(b)) return; skip = true; } if (skip) continue; @@ -9643,6 +9644,16 @@ void CpModelPresolver::DetectDuplicateConstraintsWithDifferentEnforcements( // Sign is correct, i.e. ignoring the constraint is expensive. // The two enforcement can be made equivalent. context_->UpdateRuleStats("duplicate: dual equivalence of enforcement"); + // If `a` and `b` hints are different then the whole hint satisfies + // the enforced constraint. We can thus change them to true (this cannot + // increase the objective value thanks to the `skip` test above -- the + // objective domain is non-constraining, but this only guarantees that + // singleton variables can freely *decrease* the objective). + if (context_->LiteralSolutionHint(a) != + context_->LiteralSolutionHint(b)) { + context_->UpdateLiteralSolutionHint(a, true); + context_->UpdateLiteralSolutionHint(b, true); + } context_->StoreBooleanEqualityRelation(a, b); // We can also remove duplicate constraint now. It will be done later but @@ -14364,38 +14375,9 @@ bool CpModelPresolver::MaybeRemoveFixedVariables( } // TODO(user): Right now the copy do not remove fixed variable from the - // objective, so we do that here so that these variable should not appear - // anymore. Fix that. + // objective, but ReadObjectiveFromProto() does it. Maybe we should just not + // copy them in the first place. if (context_->working_model->has_objective()) { - auto* objective = context_->working_model->mutable_objective(); - auto* mutable_vars = objective->mutable_vars(); - auto* mutable_coeffs = objective->mutable_coeffs(); - const int old_size = objective->vars().size(); - int64_t offset = 0; - int new_size = 0; - for (int i = 0; i < old_size; ++i) { - const int var = objective->vars(i); - const int64_t coeff = objective->coeffs(i); - if (context_->IsFixed(var)) { - offset += context_->FixedValue(var) * coeff; - continue; - } - mutable_vars->Set(new_size, var); - mutable_coeffs->Set(new_size, coeff); - ++new_size; - } - mutable_vars->Truncate(new_size); - mutable_coeffs->Truncate(new_size); - - // Tricky: The objective domain is on the linear objective expression - // without the offset, so we need to shift it if there is one. - objective->set_offset(objective->offset() + offset); - if (!objective->domain().empty()) { - Domain obj_domain = - ReadDomainFromProto(*objective).AdditionWith(Domain(-offset)); - FillDomainInProto(obj_domain, objective); - } - context_->ReadObjectiveFromProto(); if (!context_->CanonicalizeObjective()) return false; if (!PropagateObjective()) return false; diff --git a/ortools/sat/cp_model_solver.cc b/ortools/sat/cp_model_solver.cc index 60242a452b..160aaf0ef3 100644 --- a/ortools/sat/cp_model_solver.cc +++ b/ortools/sat/cp_model_solver.cc @@ -464,7 +464,7 @@ std::string CpModelStats(const CpModelProto& model_proto) { if (num_constants > 0) { const std::string temp = - absl::StrCat(" - ", num_constants, " constants in {", + absl::StrCat(" - ", FormatCounter(num_constants), " constants in {", absl::StrJoin(constant_values, ","), "} \n"); absl::StrAppend(&result, Summarize(temp)); } @@ -503,62 +503,62 @@ std::string CpModelStats(const CpModelProto& model_proto) { FormatCounter(name_to_num_complex_domain[c_name]), ")"); } if (name == "kInterval" && num_fixed_intervals > 0) { - absl::StrAppend(&constraints.back(), " (#fixed: ", num_fixed_intervals, - ")"); - } else if (name == "kNoOverlap2D") { absl::StrAppend(&constraints.back(), - " (#rectangles: ", no_overlap_2d_num_rectangles); + " (#fixed: ", FormatCounter(num_fixed_intervals), ")"); + } else if (name == "kNoOverlap2D") { + absl::StrAppend(&constraints.back(), " (#rectangles: ", + FormatCounter(no_overlap_2d_num_rectangles)); if (no_overlap_2d_num_optional_rectangles > 0) { - absl::StrAppend(&constraints.back(), - ", #optional: ", no_overlap_2d_num_optional_rectangles); + absl::StrAppend(&constraints.back(), ", #optional: ", + FormatCounter(no_overlap_2d_num_optional_rectangles)); } if (no_overlap_2d_num_linear_areas > 0) { - absl::StrAppend(&constraints.back(), - ", #linear_areas: ", no_overlap_2d_num_linear_areas); + absl::StrAppend(&constraints.back(), ", #linear_areas: ", + FormatCounter(no_overlap_2d_num_linear_areas)); } if (no_overlap_2d_num_quadratic_areas > 0) { absl::StrAppend(&constraints.back(), ", #quadratic_areas: ", - no_overlap_2d_num_quadratic_areas); + FormatCounter(no_overlap_2d_num_quadratic_areas)); } if (no_overlap_2d_num_fixed_rectangles > 0) { absl::StrAppend(&constraints.back(), ", #fixed_rectangles: ", - no_overlap_2d_num_fixed_rectangles); + FormatCounter(no_overlap_2d_num_fixed_rectangles)); } absl::StrAppend(&constraints.back(), ")"); } else if (name == "kCumulative") { - absl::StrAppend(&constraints.back(), - " (#intervals: ", cumulative_num_intervals); + absl::StrAppend(&constraints.back(), " (#intervals: ", + FormatCounter(cumulative_num_intervals)); if (cumulative_num_optional_intervals > 0) { - absl::StrAppend(&constraints.back(), - ", #optional: ", cumulative_num_optional_intervals); + absl::StrAppend(&constraints.back(), ", #optional: ", + FormatCounter(cumulative_num_optional_intervals)); } if (cumulative_num_variable_sizes > 0) { - absl::StrAppend(&constraints.back(), - ", #variable_sizes: ", cumulative_num_variable_sizes); + absl::StrAppend(&constraints.back(), ", #variable_sizes: ", + FormatCounter(cumulative_num_variable_sizes)); } if (cumulative_num_variable_demands > 0) { absl::StrAppend(&constraints.back(), ", #variable_demands: ", cumulative_num_variable_demands); } if (cumulative_num_fixed_intervals > 0) { - absl::StrAppend(&constraints.back(), - ", #fixed_intervals: ", cumulative_num_fixed_intervals); + absl::StrAppend(&constraints.back(), ", #fixed_intervals: ", + FormatCounter(cumulative_num_fixed_intervals)); } absl::StrAppend(&constraints.back(), ")"); } else if (name == "kNoOverlap") { - absl::StrAppend(&constraints.back(), - " (#intervals: ", no_overlap_num_intervals); + absl::StrAppend(&constraints.back(), " (#intervals: ", + FormatCounter(no_overlap_num_intervals)); if (no_overlap_num_optional_intervals > 0) { - absl::StrAppend(&constraints.back(), - ", #optional: ", no_overlap_num_optional_intervals); + absl::StrAppend(&constraints.back(), ", #optional: ", + FormatCounter(no_overlap_num_optional_intervals)); } if (no_overlap_num_variable_sizes > 0) { - absl::StrAppend(&constraints.back(), - ", #variable_sizes: ", no_overlap_num_variable_sizes); + absl::StrAppend(&constraints.back(), ", #variable_sizes: ", + FormatCounter(no_overlap_num_variable_sizes)); } if (no_overlap_num_fixed_intervals > 0) { - absl::StrAppend(&constraints.back(), - ", #fixed_intervals: ", no_overlap_num_fixed_intervals); + absl::StrAppend(&constraints.back(), ", #fixed_intervals: ", + FormatCounter(no_overlap_num_fixed_intervals)); } absl::StrAppend(&constraints.back(), ")"); } @@ -1282,13 +1282,13 @@ class LnsSolver : public SubSolver { shared_->time_limit->UpdateLocalLimit(local_time_limit); // Presolve and solve the LNS fragment. - int64_t buffer_size; + size_t buffer_size; { absl::MutexLock l(&next_arena_size_mutex_); buffer_size = next_arena_size_; } - auto arena_buffer = std::make_unique(buffer_size); - google::protobuf::Arena arena(arena_buffer.get(), buffer_size); + google::protobuf::Arena arena( + google::protobuf::ArenaOptions({.start_block_size = buffer_size})); CpModelProto& lns_fragment = *google::protobuf::Arena::Create(&arena); CpModelProto& mapping_proto = diff --git a/ortools/sat/presolve_context.cc b/ortools/sat/presolve_context.cc index 2aede237b7..de69add9b2 100644 --- a/ortools/sat/presolve_context.cc +++ b/ortools/sat/presolve_context.cc @@ -1951,29 +1951,41 @@ void PresolveContext::ReadObjectiveFromProto() { // summing an objective partial sum. Because of the model validation, this // shouldn't overflow, and we make sure it stays this way. objective_overflow_detection_ = std::abs(objective_integer_before_offset_); + int64_t fixed_terms = 0; objective_map_.clear(); for (int i = 0; i < obj.vars_size(); ++i) { - const int ref = obj.vars(i); - const int64_t var_max_magnitude = - std::max(std::abs(MinOf(ref)), std::abs(MaxOf(ref))); + int var = obj.vars(i); + int64_t coeff = obj.coeffs(i); - // Skipping var fixed to zero allow to avoid some overflow in situation - // were we can deal with it. - if (var_max_magnitude == 0) continue; + // TODO(user): There should be no negative reference here ! + if (!RefIsPositive(var)) { + var = NegatedRef(var); + coeff = -coeff; + } - const int64_t coeff = obj.coeffs(i); - objective_overflow_detection_ += var_max_magnitude * std::abs(coeff); + // We remove fixed terms as we read the objective. This can help a lot on + // LNS problems with a large proportions of fixed terms. + if (IsFixed(var)) { + fixed_terms += FixedValue(var) * coeff; + continue; + } - const int var = PositiveRef(ref); - objective_map_[var] += RefIsPositive(ref) ? coeff : -coeff; + const int64_t var_max_magnitude = + std::max(std::abs(MinOf(var)), std::abs(MaxOf(var))); + objective_overflow_detection_ += var_max_magnitude * std::abs(coeff); + objective_map_[var] += RefIsPositive(var) ? coeff : -coeff; if (objective_map_[var] == 0) { RemoveVariableFromObjective(var); } else { var_to_constraints_[var].insert(kObjectiveConstraint); } } + + if (fixed_terms != 0) { + AddToObjectiveOffset(fixed_terms); + } } bool PresolveContext::CanonicalizeOneObjectiveVariable(int var) { diff --git a/ortools/sat/presolve_context.h b/ortools/sat/presolve_context.h index 1929e71128..e5e95ce239 100644 --- a/ortools/sat/presolve_context.h +++ b/ortools/sat/presolve_context.h @@ -543,9 +543,9 @@ class PresolveContext { return it == objective_map_.end() ? 0 : it->second; } - // Returns true if the variables in the objective with a positive (resp. + // Returns false if the variables in the objective with a positive (resp. // negative) coefficient can freely decrease (resp. increase) within their - // domain (if we ignore the other constraints). + // domain (if we ignore the other constraints). Otherwise, returns true. bool ObjectiveDomainIsConstraining() const { return objective_domain_is_constraining_; }