From 136d180aba09ef61ee65bd07b81048a875f23e16 Mon Sep 17 00:00:00 2001 From: Laurent Perron Date: Fri, 26 Jan 2024 21:12:26 +0100 Subject: [PATCH] [CP-SAT] disable problematic presolve on no_overlap_2d with constant dimension; fix bug in diffn propagation, fix #4068 --- ortools/sat/cp_model_presolve.cc | 11 +++++-- ortools/sat/diffn.cc | 54 +++++++++++++++++--------------- ortools/sat/diffn.h | 4 +-- 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/ortools/sat/cp_model_presolve.cc b/ortools/sat/cp_model_presolve.cc index 69f98e7496..35abe4782c 100644 --- a/ortools/sat/cp_model_presolve.cc +++ b/ortools/sat/cp_model_presolve.cc @@ -5383,6 +5383,7 @@ bool CpModelPresolver::PresolveNoOverlap2D(int /*c*/, ConstraintProto* ct) { bool x_constant = true; bool y_constant = true; + bool has_zero_sized_interval = false; // Filter absent boxes. int new_size = 0; @@ -5413,6 +5414,10 @@ bool CpModelPresolver::PresolveNoOverlap2D(int /*c*/, ConstraintProto* ct) { if (y_constant && !context_->IntervalIsConstant(y_interval_index)) { y_constant = false; } + if (context_->SizeMax(x_interval_index) == 0 || + context_->SizeMax(y_interval_index) == 0) { + has_zero_sized_interval = true; + } } std::vector> components = GetOverlappingRectangleComponents( @@ -5433,10 +5438,10 @@ bool CpModelPresolver::PresolveNoOverlap2D(int /*c*/, ConstraintProto* ct) { return RemoveConstraint(ct); } - if (x_constant || y_constant) { + if (!has_zero_sized_interval && (x_constant || y_constant)) { context_->UpdateRuleStats( - "no_overlap_2d: a dimension is constant, splitting into many no " - "overlaps"); + "no_overlap_2d: a dimension is constant, splitting into many " + "no_overlaps"); std::vector indexed_intervals; for (int i = 0; i < new_size; ++i) { int x = proto.x_intervals(i); diff --git a/ortools/sat/diffn.cc b/ortools/sat/diffn.cc index eca299ab9e..a1c3a6ae7c 100644 --- a/ortools/sat/diffn.cc +++ b/ortools/sat/diffn.cc @@ -272,8 +272,6 @@ bool NonOverlappingRectanglesEnergyPropagator::Propagate() { .x_max = std::numeric_limits::min(), .y_min = std::numeric_limits::max(), .y_max = std::numeric_limits::min()}; - IntegerValue max_x_item_size = IntegerValue(0); - IntegerValue max_y_item_size = IntegerValue(0); std::vector active_box_ranges; active_box_ranges.reserve(num_boxes); for (int box = 0; box < num_boxes; ++box) { @@ -293,8 +291,6 @@ bool NonOverlappingRectanglesEnergyPropagator::Propagate() { .y_max = y_.StartMax(box) + y_.SizeMin(box)}, .x_size = x_.SizeMin(box), .y_size = y_.SizeMin(box)}); - max_x_item_size = std::max(max_x_item_size, x_.SizeMin(box)); - max_y_item_size = std::max(max_y_item_size, y_.SizeMin(box)); } if (active_box_ranges.size() < 2) { @@ -314,8 +310,8 @@ bool NonOverlappingRectanglesEnergyPropagator::Propagate() { return true; } - std::optional best_conflict = FindConflict( - std::move(active_box_ranges), max_x_item_size, max_y_item_size); + std::optional best_conflict = + FindConflict(std::move(active_box_ranges)); if (!best_conflict.has_value()) { return true; } @@ -328,8 +324,7 @@ bool NonOverlappingRectanglesEnergyPropagator::Propagate() { IntegerValue best_explanation_size = best_conflict->items.size(); bool refined = false; while (true) { - const std::optional conflict = - FindConflict(best_conflict->items, max_x_item_size, max_y_item_size); + const std::optional conflict = FindConflict(best_conflict->items); if (!conflict.has_value()) break; // We prefer an explanation with the least number of boxes. if (conflict->items.size() >= best_explanation_size) { @@ -356,8 +351,7 @@ bool NonOverlappingRectanglesEnergyPropagator::Propagate() { std::optional NonOverlappingRectanglesEnergyPropagator::FindConflict( - std::vector active_box_ranges, - const IntegerValue& max_x_item_size, const IntegerValue& max_y_item_size) { + std::vector active_box_ranges) { const auto rectangles_with_too_much_energy = FindRectanglesWithEnergyConflictMC(active_box_ranges, *random_, 1.0, 0.8); @@ -385,25 +379,35 @@ NonOverlappingRectanglesEnergyPropagator::FindConflict( int opp_problem_size = 0; // Also try the DFF on known conflicts, hopefully we will get a smaller // explanation. - std::vector filtered_rectangles; - filtered_rectangles.reserve( - rectangles_with_too_much_energy.conflicts.size() + - rectangles_with_too_much_energy.candidates.size()); - filtered_rectangles = rectangles_with_too_much_energy.conflicts; - // Our current DFF does nothing if all elements are smaller than half of the - // size of the rectangle. So we filter all rectangles that are twice the - // dimensions of the largest item of our problem. - for (const Rectangle& r : rectangles_with_too_much_energy.candidates) { - if (r.SizeX() <= 2 * max_x_item_size || r.SizeY() <= 2 * max_y_item_size) { - filtered_rectangles.push_back(r); - } - } // Sample 10 rectangles for looking for a conflict with DFF. This avoids // making this heuristic too costly. constexpr int kSampleSize = 10; absl::InlinedVector sampled_rectangles; - std::sample(filtered_rectangles.begin(), filtered_rectangles.end(), - std::back_inserter(sampled_rectangles), kSampleSize, *random_); + std::sample(rectangles_with_too_much_energy.conflicts.begin(), + rectangles_with_too_much_energy.conflicts.end(), + std::back_inserter(sampled_rectangles), 5, *random_); + std::sample(rectangles_with_too_much_energy.candidates.begin(), + rectangles_with_too_much_energy.candidates.end(), + std::back_inserter(sampled_rectangles), + kSampleSize - sampled_rectangles.size(), *random_); + std::sort(sampled_rectangles.begin(), sampled_rectangles.end(), + [](const Rectangle& a, const Rectangle& b) { + const bool larger = std::make_pair(a.SizeX(), a.SizeY()) > + std::make_pair(b.SizeX(), b.SizeY()); + // Double-check the invariant from + // FindRectanglesWithEnergyConflictMC() that given two returned + // rectangles there is one that is fully inside the other. + if (larger) { + // Rectangle b is fully contained inside a + DCHECK(a.x_min <= b.x_min && a.x_max >= b.x_max && + a.y_min <= b.y_min && a.y_max >= b.y_max); + } else { + // Rectangle a is fully contained inside b + DCHECK(a.x_min >= b.x_min && a.x_max <= b.x_max && + a.y_min >= b.y_min && a.y_max <= b.y_max); + } + return larger; + }); std::vector sizes_x, sizes_y; sizes_x.reserve(active_box_ranges.size()); sizes_y.reserve(active_box_ranges.size()); diff --git a/ortools/sat/diffn.h b/ortools/sat/diffn.h index 23bfc35738..3792d405b7 100644 --- a/ortools/sat/diffn.h +++ b/ortools/sat/diffn.h @@ -16,6 +16,7 @@ #include #include +#include #include #include "absl/container/flat_hash_set.h" @@ -60,8 +61,7 @@ class NonOverlappingRectanglesEnergyPropagator : public PropagatorInterface { int opp_problem_size; }; std::optional FindConflict( - std::vector active_box_ranges, - const IntegerValue& max_x_item_size, const IntegerValue& max_y_item_size); + std::vector active_box_ranges); std::vector GeneralizeExplanation(const Conflict& conflict);