[CP-SAT] experimental integration of variable shaving; tentative fix for #4615; more bugfixes

Author: lperron

ortools/sat/BUILD.bazel

@@ -41,6 +41,26 @@ cc_library(
     ],
 )
 
+cc_test(
+    name = "cp_model_test",
+    size = "small",
+    srcs = ["cp_model_test.cc"],
+    deps = [
+        ":cp_model",
+        ":cp_model_cc_proto",
+        ":cp_model_checker",
+        ":cp_model_solver",
+        ":model",
+        ":sat_parameters_cc_proto",
+        "//ortools/base",
+        "//ortools/base:container_logging",
+        "//ortools/base:gmock_main",
+        "//ortools/base:parse_test_proto",
+        "//ortools/util:sorted_interval_list",
+        "@abseil-cpp//absl/types:span",
+    ],
+)
+
 cc_library(
     name = "model",
     hdrs = ["model.h"],
@@ -2838,7 +2858,6 @@ cc_library(
         ":cuts",
         ":integer",
         ":integer_base",
-        ":intervals",
         ":linear_constraint",
         ":linear_constraint_manager",
         ":model",
@@ -2854,6 +2873,7 @@ cc_library(
         "@abseil-cpp//absl/base:core_headers",
         "@abseil-cpp//absl/container:btree",
         "@abseil-cpp//absl/container:flat_hash_map",
+        "@abseil-cpp//absl/log",
         "@abseil-cpp//absl/log:check",
         "@abseil-cpp//absl/strings",
         "@abseil-cpp//absl/types:span",

ortools/sat/constraint_violation.cc

@@ -59,23 +59,6 @@ int64_t AffineValue(const ViewOfAffineLinearExpressionProto& affine,
   return affine.coeff * solution[affine.var] + affine.offset;
 }
 
-LinearExpressionProto ExprDiff(const LinearExpressionProto& a,
-                               const LinearExpressionProto& b) {
-  LinearExpressionProto result;
-  result.set_offset(a.offset() - b.offset());
-  result.mutable_vars()->Reserve(a.vars().size() + b.vars().size());
-  result.mutable_coeffs()->Reserve(a.vars().size() + b.vars().size());
-  for (int i = 0; i < a.vars().size(); ++i) {
-    result.add_vars(a.vars(i));
-    result.add_coeffs(a.coeffs(i));
-  }
-  for (int i = 0; i < b.vars().size(); ++i) {
-    result.add_vars(b.vars(i));
-    result.add_coeffs(-b.coeffs(i));
-  }
-  return result;
-}
-
 LinearExpressionProto LinearExprSum(LinearExpressionProto a,
                                     LinearExpressionProto b) {
   LinearExpressionProto result;
@@ -1105,56 +1088,38 @@ int64_t CompiledAllDiffConstraint::ComputeViolation(
   return violation;
 }
 
-// ----- NoOverlapBetweenTwoIntervals -----
+// ----- CompiledNoOverlapWithTwoIntervals -----
 
-NoOverlapBetweenTwoIntervals::NoOverlapBetweenTwoIntervals(
-    int interval_0, int interval_1, const CpModelProto& cp_model) {
-  const ConstraintProto& ct0 = cp_model.constraints(interval_0);
-  const ConstraintProto& ct1 = cp_model.constraints(interval_1);
-
-  // The more compact the better, hence the size + int[].
-  num_enforcements_ =
-      ct0.enforcement_literal().size() + ct1.enforcement_literal().size();
-  if (num_enforcements_ > 0) {
-    enforcements_.reset(new int[num_enforcements_]);
-    int i = 0;
-    for (const int lit : ct0.enforcement_literal()) enforcements_[i++] = lit;
-    for (const int lit : ct1.enforcement_literal()) enforcements_[i++] = lit;
+template <bool has_enforcement>
+int64_t CompiledNoOverlapWithTwoIntervals<has_enforcement>::ViolationDelta(
+    int /*var*/, int64_t /*old_value*/, absl::Span<const int64_t> solution) {
+  if (has_enforcement) {
+    for (const int lit : enforcements_) {
+      if (!LiteralValue(lit, solution)) return -violation_;
+    }
   }
 
-  // We prefer to use start + size instead of end so that moving "start" moves
-  // the whole interval around (for the non-fixed duration case).
-  end_minus_start_1_ =
-      ExprDiff(LinearExprSum(ct0.interval().start(), ct0.interval().size()),
-               ct1.interval().start());
-  end_minus_start_2_ =
-      ExprDiff(LinearExprSum(ct1.interval().start(), ct1.interval().size()),
-               ct0.interval().start());
+  const int64_t s1 = AffineValue(interval1_.start, solution);
+  const int64_t e1 = AffineValue(interval1_.end, solution);
+  const int64_t s2 = AffineValue(interval2_.start, solution);
+  const int64_t e2 = AffineValue(interval2_.end, solution);
+  const int64_t repair = std::min(e2 - s1, e1 - s2);
+  if (repair <= 0) return -violation_;  // disjoint
+  return repair - violation_;
 }
 
-// Same as NoOverlapMinRepairDistance().
-int64_t NoOverlapBetweenTwoIntervals::ComputeViolationInternal(
-    absl::Span<const int64_t> solution) {
-  for (int i = 0; i < num_enforcements_; ++i) {
-    if (!LiteralValue(enforcements_[i], solution)) return 0;
-  }
-  const int64_t diff1 = ExprValue(end_minus_start_1_, solution);
-  const int64_t diff2 = ExprValue(end_minus_start_2_, solution);
-  return std::max(std::min(diff1, diff2), int64_t{0});
-}
-
-std::vector<int> NoOverlapBetweenTwoIntervals::UsedVariables(
+template <bool has_enforcement>
+std::vector<int>
+CompiledNoOverlapWithTwoIntervals<has_enforcement>::UsedVariables(
     const CpModelProto& /*model_proto*/) const {
   std::vector<int> result;
-  for (int i = 0; i < num_enforcements_; ++i) {
-    result.push_back(PositiveRef(enforcements_[i]));
-  }
-  for (const int var : end_minus_start_1_.vars()) {
-    result.push_back(PositiveRef(var));
-  }
-  for (const int var : end_minus_start_2_.vars()) {
-    result.push_back(PositiveRef(var));
+  if (has_enforcement) {
+    for (const int ref : enforcements_) result.push_back(PositiveRef(ref));
   }
+  interval1_.start.AppendVarTo(result);
+  interval1_.end.AppendVarTo(result);
+  interval2_.start.AppendVarTo(result);
+  interval2_.end.AppendVarTo(result);
   gtl::STLSortAndRemoveDuplicates(&result);
   result.shrink_to_fit();
   return result;
@@ -1291,6 +1256,7 @@ CompiledNoOverlap2dWithTwoBoxes<has_enforcement>::UsedVariables(
   box2_.y_min.AppendVarTo(result);
   box2_.y_max.AppendVarTo(result);
   gtl::STLSortAndRemoveDuplicates(&result);
+  result.shrink_to_fit();
   return result;
 }
 
@@ -1722,20 +1688,31 @@ void LsEvaluator::CompileOneConstraint(const ConstraintProto& ct) {
         // We expand the no_overlap constraints into a quadratic number of
         // disjunctions.
         for (int i = 0; i + 1 < size; ++i) {
-          const IntervalConstraintProto& interval_i =
-              cp_model_.constraints(ct.no_overlap().intervals(i)).interval();
+          const ConstraintProto& proto_i =
+              cp_model_.constraints(ct.no_overlap().intervals(i));
+          const IntervalConstraintProto& interval_i = proto_i.interval();
           const int64_t min_start_i = ExprMin(interval_i.start(), cp_model_);
           const int64_t max_end_i = ExprMax(interval_i.end(), cp_model_);
           for (int j = i + 1; j < size; ++j) {
-            const IntervalConstraintProto& interval_j =
-                cp_model_.constraints(ct.no_overlap().intervals(j)).interval();
+            const ConstraintProto& proto_j =
+                cp_model_.constraints(ct.no_overlap().intervals(j));
+            const IntervalConstraintProto& interval_j = proto_j.interval();
             const int64_t min_start_j = ExprMin(interval_j.start(), cp_model_);
             const int64_t max_end_j = ExprMax(interval_j.end(), cp_model_);
             if (min_start_i >= max_end_j || min_start_j >= max_end_i) continue;
 
-            constraints_.emplace_back(new NoOverlapBetweenTwoIntervals(
-                ct.no_overlap().intervals(i), ct.no_overlap().intervals(j),
-                cp_model_));
+            const bool has_enforcement =
+                !proto_i.enforcement_literal().empty() ||
+                !proto_j.enforcement_literal().empty();
+            if (has_enforcement) {
+              constraints_.emplace_back(
+                  new CompiledNoOverlapWithTwoIntervals<true>(proto_i,
+                                                              proto_j));
+            } else {
+              constraints_.emplace_back(
+                  new CompiledNoOverlapWithTwoIntervals<false>(proto_i,
+                                                               proto_j));
+            }
           }
         }
       }
@@ -2202,7 +2179,7 @@ int64_t CompiledReservoirConstraint::IncrementalViolation(
   int64_t previous_time = std::numeric_limits<int64_t>::min();
 
   // TODO(user): This code is the hotspot for our local search on cumulative.
-  // It can probably be slighlty improved. We might also be able to abort early
+  // It can probably be slightly improved. We might also be able to abort early
   // if we know that capacity is high enough compared to the highest point of
   // the profile.
   int i = 0;

ortools/sat/constraint_violation.h

@@ -22,6 +22,7 @@
 #include <vector>
 
 #include "absl/container/flat_hash_map.h"
+#include "absl/log/check.h"
 #include "absl/types/span.h"
 #include "ortools/base/stl_util.h"
 #include "ortools/sat/cp_model.pb.h"
@@ -540,36 +541,75 @@ class CompiledAllDiffConstraint : public CompiledConstraintWithProto {
   std::vector<int64_t> values_;
 };
 
+// This is more compact and faster to destroy than storing a
+// LinearExpressionProto.
+struct ViewOfAffineLinearExpressionProto {
+  explicit ViewOfAffineLinearExpressionProto(
+      const LinearExpressionProto& proto) {
+    if (!proto.vars().empty()) {
+      DCHECK_EQ(proto.vars().size(), 1);
+      var = proto.vars(0);
+      coeff = proto.coeffs(0);
+    }
+    offset = proto.offset();
+  }
+
+  void AppendVarTo(std::vector<int>& result) const {
+    if (coeff != 0) result.push_back(var);
+  }
+
+  int var = 0;
+  int64_t coeff = 0;
+  int64_t offset = 0;
+};
+
 // Special constraint for no overlap between two intervals.
 // We usually expand small no-overlap in n^2 such constraint, so we want to
 // be compact and efficient here.
-class NoOverlapBetweenTwoIntervals : public CompiledConstraint {
+template <bool has_enforcement = true>
+class CompiledNoOverlapWithTwoIntervals : public CompiledConstraint {
  public:
-  NoOverlapBetweenTwoIntervals(int interval_0, int interval_1,
-                               const CpModelProto& cp_model);
-  ~NoOverlapBetweenTwoIntervals() override = default;
+  struct Interval {
+    explicit Interval(const ConstraintProto& x)
+        : start(x.interval().start()), end(x.interval().end()) {}
+    ViewOfAffineLinearExpressionProto start;
+    ViewOfAffineLinearExpressionProto end;
+  };
+
+  CompiledNoOverlapWithTwoIntervals(const ConstraintProto& x1,
+                                    const ConstraintProto& x2)
+      : interval1_(x1), interval2_(x2) {
+    if (has_enforcement) {
+      enforcements_.assign(x1.enforcement_literal().begin(),
+                           x1.enforcement_literal().end());
+      enforcements_.insert(enforcements_.end(),
+                           x2.enforcement_literal().begin(),
+                           x2.enforcement_literal().end());
+      gtl::STLSortAndRemoveDuplicates(&enforcements_);
+    }
+  }
+
+  ~CompiledNoOverlapWithTwoIntervals() final = default;
 
   int64_t ComputeViolation(absl::Span<const int64_t> solution) final {
-    return ComputeViolationInternal(solution);
+    // Optimization hack: If we create a ComputeViolationInternal() that we call
+    // from here and in ViolationDelta(), then the later is not inlined below in
+    // ViolationDelta() where it matter a lot for performance.
+    violation_ = 0;
+    violation_ = ViolationDelta(0, 0, solution);
+    return violation_;
   }
 
-  // Note(user): this is the same implementation as the base one, but it
-  // avoid one virtual call !
   int64_t ViolationDelta(
       int /*var*/, int64_t /*old_value*/,
-      absl::Span<const int64_t> solution_with_new_value) final {
-    return ComputeViolationInternal(solution_with_new_value) - violation();
-  }
+      absl::Span<const int64_t> solution_with_new_value) final;
 
   std::vector<int> UsedVariables(const CpModelProto& model_proto) const final;
 
  private:
-  int64_t ComputeViolationInternal(absl::Span<const int64_t> solution);
-
-  int num_enforcements_;
-  std::unique_ptr<int[]> enforcements_;
-  LinearExpressionProto end_minus_start_1_;
-  LinearExpressionProto end_minus_start_2_;
+  std::vector<int> enforcements_;
+  const Interval interval1_;
+  const Interval interval2_;
 };
 
 class CompiledNoOverlap2dConstraint : public CompiledConstraintWithProto {
@@ -584,28 +624,6 @@ class CompiledNoOverlap2dConstraint : public CompiledConstraintWithProto {
   const CpModelProto& cp_model_;
 };
 
-// This is more compact and faster to destroy than storing a
-// LinearExpressionProto.
-struct ViewOfAffineLinearExpressionProto {
-  explicit ViewOfAffineLinearExpressionProto(
-      const LinearExpressionProto& proto) {
-    if (!proto.vars().empty()) {
-      DCHECK_EQ(proto.vars().size(), 1);
-      var = proto.vars(0);
-      coeff = proto.coeffs(0);
-    }
-    offset = proto.offset();
-  }
-
-  void AppendVarTo(std::vector<int>& result) const {
-    if (coeff != 0) result.push_back(var);
-  }
-
-  int var = 0;
-  int64_t coeff = 0;
-  int64_t offset = 0;
-};
-
 template <bool has_enforcement = true>
 class CompiledNoOverlap2dWithTwoBoxes : public CompiledConstraint {
  public:
@@ -668,8 +686,8 @@ class CompiledNoOverlap2dWithTwoBoxes : public CompiledConstraint {
 };
 
 // This can be used to encode reservoir or a cumulative constraints for LS. We
-// have a set of event time, and we use for overal violation the sum of overload
-// over time.
+// have a set of event time, and we use for overall violation the sum of
+// overload over time.
 //
 // This version support an incremental computation when just a few events
 // changes, which is roughly O(n) instead of O(n log n) which makes it

ortools/sat/cp_model_copy.cc

@@ -84,7 +84,9 @@ bool ModelCopy::ImportAndSimplifyConstraints(
 
   starting_constraint_index_ = context_->working_model->constraints_size();
   for (int c = 0; c < in_model.constraints_size(); ++c) {
-    if (active_constraints != nullptr && !active_constraints(c)) continue;
+    if (active_constraints != nullptr && !active_constraints(c)) {
+      continue;
+    }
     const ConstraintProto& ct = in_model.constraints(c);
     if (first_copy) {
       if (!PrepareEnforcementCopyWithDup(ct)) continue;
@@ -915,14 +917,17 @@ bool ImportModelWithBasicPresolveIntoContext(const CpModelProto& in_model,
 
 bool ImportModelAndDomainsWithBasicPresolveIntoContext(
     const CpModelProto& in_model, absl::Span<const Domain> domains,
-    std::function<bool(int)> active_constraints, PresolveContext* context) {
+    std::function<bool(int)> active_constraints, PresolveContext* context,
+    std::vector<int>* interval_mapping) {
   CHECK_EQ(domains.size(), in_model.variables_size());
   ModelCopy copier(context);
   copier.CreateVariablesFromDomains(domains);
   if (copier.ImportAndSimplifyConstraints(in_model, /*first_copy=*/false,
                                           active_constraints)) {
     CopyEverythingExceptVariablesAndConstraintsFieldsIntoContext(in_model,
                                                                  context);
+    interval_mapping->assign(copier.InternalIntervalMapping().begin(),
+                             copier.InternalIntervalMapping().end());
     return true;
   }
   return !context->ModelIsUnsat();