Fix PriorityReplace

CHANGELOG.md

@@ -39,6 +39,8 @@
 #### Core solving
 
 - Solution quality regression when using lots of skills (#1193)
+- Invalid route reached by `PriorityReplace` (#1162)
+- Account for gain and assigned tasks to pick priority-improving moves (#1212)
 - Crash due to wrong delivery values in some validity checks (#1164)
 - Crash when input is valid JSON but not an object (#1172)
 - Capacity array check consistency (#1086)

src/algorithms/local_search/local_search.cpp

@@ -405,14 +405,17 @@ void LocalSearch<Route,
                                             std::vector<Eval>(_nb_vehicles,
                                                               Eval()));
 
-  // Store best priority increase for matching move. Only operators
-  // involving a single route and unassigned jobs can change overall
-  // priority (currently only UnassignedExchange).
+  // Store best priority increase and number of assigned tasks for use
+  // with operators involving a single route and unassigned jobs
+  // (UnassignedExchange and PriorityReplace).
   std::vector<Priority> best_priorities(_nb_vehicles, 0);
+  std::vector<unsigned> best_removals(_nb_vehicles,
+                                      std::numeric_limits<unsigned>::max());
 
   // Dummy init to enter first loop.
   Eval best_gain(static_cast<Cost>(1));
   Priority best_priority = 0;
+  auto best_removal = std::numeric_limits<unsigned>::max();
 
   while (best_gain.cost > 0 || best_priority > 0) {
     if (_deadline.has_value() && _deadline.value() < utils::now()) {
@@ -422,83 +425,6 @@ void LocalSearch<Route,
     if (_input.has_jobs()) {
       // Move(s) that don't make sense for shipment-only instances.
 
-      // PriorityReplace stuff
-      for (const Index u : _sol_state.unassigned) {
-        if (_input.jobs[u].type != JOB_TYPE::SINGLE) {
-          continue;
-        }
-
-        Priority u_priority = _input.jobs[u].priority;
-
-        for (const auto& [source, target] : s_t_pairs) {
-          if (source != target || !_input.vehicle_ok_with_job(source, u) ||
-              _sol[source].empty() ||
-              // We only search for net priority gains here.
-              (u_priority <= _sol_state.fwd_priority[source].front() &&
-               u_priority <= _sol_state.bwd_priority[source].back())) {
-            continue;
-          }
-
-          // Find where to stop when replacing beginning of route.
-          const auto fwd_over =
-            std::ranges::find_if(_sol_state.fwd_priority[source],
-                                 [u_priority](const auto p) {
-                                   return u_priority < p;
-                                 });
-          const Index fwd_over_rank =
-            std::distance(_sol_state.fwd_priority[source].begin(), fwd_over);
-          // A fwd_last_rank of zero will discard replacing the start
-          // of the route.
-          const Index fwd_last_rank =
-            (fwd_over_rank > 0) ? fwd_over_rank - 1 : 0;
-          const Priority begin_priority_gain =
-            u_priority - _sol_state.fwd_priority[source][fwd_last_rank];
-
-          // Find where to stop when replacing end of route.
-          const auto bwd_over =
-            std::find_if(_sol_state.bwd_priority[source].crbegin(),
-                         _sol_state.bwd_priority[source].crend(),
-                         [u_priority](const auto p) { return u_priority < p; });
-          const Index bwd_over_rank =
-            std::distance(_sol_state.bwd_priority[source].crbegin(), bwd_over);
-          const Index bwd_first_rank = _sol[source].size() - bwd_over_rank;
-          const Priority end_priority_gain =
-            u_priority - _sol_state.bwd_priority[source][bwd_first_rank];
-
-          assert(fwd_over_rank > 0 || bwd_over_rank > 0);
-
-          const auto best_current_priority =
-            std::max(begin_priority_gain, end_priority_gain);
-
-          if (best_current_priority > 0 &&
-              best_priorities[source] <= best_current_priority) {
-#ifdef LOG_LS_OPERATORS
-            ++tried_moves[OperatorName::PriorityReplace];
-#endif
-            PriorityReplace r(_input,
-                              _sol_state,
-                              _sol_state.unassigned,
-                              _sol[source],
-                              source,
-                              fwd_last_rank,
-                              bwd_first_rank,
-                              u,
-                              best_priorities[source]);
-
-            if (r.is_valid() &&
-                (best_priorities[source] < r.priority_gain() ||
-                 (best_priorities[source] == r.priority_gain() &&
-                  best_gains[source][source] < r.gain()))) {
-              best_priorities[source] = r.priority_gain();
-              // This may potentially define a negative value as best
-              // gain.
-              best_gains[source][source] = r.gain();
-              best_ops[source][source] = std::make_unique<PriorityReplace>(r);
-            }
-          }
-        }
-      }
-
       // UnassignedExchange stuff
       for (const Index u : _sol_state.unassigned) {
         if (_input.jobs[u].type != JOB_TYPE::SINGLE) {
@@ -593,6 +519,7 @@ void LocalSearch<Route,
 
                 if (better_if_valid && r.is_valid()) {
                   best_priorities[source] = priority_gain;
+                  best_removals[source] = 0;
                   // This may potentially define a negative value as
                   // best gain in case priority_gain is non-zero.
                   best_gains[source][source] = r.gain();
@@ -604,6 +531,110 @@ void LocalSearch<Route,
           }
         }
       }
+
+      // PriorityReplace stuff
+      for (const Index u : _sol_state.unassigned) {
+        if (_input.jobs[u].type != JOB_TYPE::SINGLE) {
+          continue;
+        }
+
+        Priority u_priority = _input.jobs[u].priority;
+
+        for (const auto& [source, target] : s_t_pairs) {
+          if (source != target || !_input.vehicle_ok_with_job(source, u) ||
+              _sol[source].empty() ||
+              // We only search for net priority gains here.
+              (u_priority <= _sol_state.fwd_priority[source].front() &&
+               u_priority <= _sol_state.bwd_priority[source].back())) {
+            continue;
+          }
+
+          // Find where to stop when replacing beginning of route in
+          // order to generate a net priority gain.
+          const auto fwd_over =
+            std::ranges::find_if(_sol_state.fwd_priority[source],
+                                 [u_priority](const auto p) {
+                                   return u_priority <= p;
+                                 });
+          const Index fwd_over_rank =
+            std::distance(_sol_state.fwd_priority[source].begin(), fwd_over);
+          // A fwd_last_rank of zero will discard replacing the start
+          // of the route.
+          Index fwd_last_rank = (fwd_over_rank > 0) ? fwd_over_rank - 1 : 0;
+          // Go back to find the biggest route beginning portion where
+          // splitting is possible.
+          while (fwd_last_rank > 0 &&
+                 _sol[source].has_pending_delivery_after_rank(fwd_last_rank)) {
+            --fwd_last_rank;
+          }
+          const Priority begin_priority_gain =
+            u_priority - _sol_state.fwd_priority[source][fwd_last_rank];
+
+          // Find where to stop when replacing end of route in order
+          // to generate a net priority gain.
+          const auto bwd_over =
+            std::find_if(_sol_state.bwd_priority[source].crbegin(),
+                         _sol_state.bwd_priority[source].crend(),
+                         [u_priority](const auto p) {
+                           return u_priority <= p;
+                         });
+          const Index bwd_over_rank =
+            std::distance(_sol_state.bwd_priority[source].crbegin(), bwd_over);
+          Index bwd_first_rank = _sol[source].size() - bwd_over_rank;
+          if (bwd_first_rank == 0) {
+            // Sum of priorities for whole route is lower than job
+            // priority. We elude this case as it is covered by start
+            // replacing (also whole route).
+            assert(fwd_last_rank == _sol[source].size() - 1);
+            ++bwd_first_rank;
+          }
+          while (
+            bwd_first_rank < _sol[source].size() - 1 &&
+            _sol[source].has_pending_delivery_after_rank(bwd_first_rank - 1)) {
+            ++bwd_first_rank;
+          }
+          const Priority end_priority_gain =
+            u_priority - _sol_state.bwd_priority[source][bwd_first_rank];
+
+          assert(fwd_over_rank > 0 || bwd_over_rank > 0);
+
+          const auto best_current_priority =
+            std::max(begin_priority_gain, end_priority_gain);
+
+          if (best_current_priority > 0 &&
+              best_priorities[source] <= best_current_priority) {
+#ifdef LOG_LS_OPERATORS
+            ++tried_moves[OperatorName::PriorityReplace];
+#endif
+            PriorityReplace r(_input,
+                              _sol_state,
+                              _sol_state.unassigned,
+                              _sol[source],
+                              source,
+                              fwd_last_rank,
+                              bwd_first_rank,
+                              u,
+                              best_priorities[source]);
+
+            if (r.is_valid()) {
+              const auto priority_gain = r.priority_gain();
+              const unsigned removal = _sol[source].size() - r.assigned();
+              const auto gain = r.gain();
+              if (std::tie(best_priorities[source],
+                           removal,
+                           best_gains[source][source]) <
+                  std::tie(priority_gain, best_removals[source], gain)) {
+                best_priorities[source] = priority_gain;
+                best_removals[source] = removal;
+                // This may potentially define a negative value as best
+                // gain.
+                best_gains[source][source] = r.gain();
+                best_ops[source][source] = std::make_unique<PriorityReplace>(r);
+              }
+            }
+          }
+        }
+      }
     }
 
     // CrossExchange stuff
@@ -1808,13 +1839,16 @@ void LocalSearch<Route,
     // Find best overall move, first checking priority increase then
     // best gain if no priority increase is available.
     best_priority = 0;
+    best_removal = std::numeric_limits<unsigned>::max();
     best_gain = Eval();
     Index best_source = 0;
     Index best_target = 0;
 
     for (unsigned s_v = 0; s_v < _nb_vehicles; ++s_v) {
-      if (best_priorities[s_v] > best_priority) {
+      if (std::tie(best_priority, best_removals[s_v], best_gain) <
+          std::tie(best_priorities[s_v], best_removal, best_gains[s_v][s_v])) {
         best_priority = best_priorities[s_v];
+        best_removal = best_removals[s_v];
         best_gain = best_gains[s_v][s_v];
         best_source = s_v;
         best_target = s_v;
@@ -1916,6 +1950,7 @@ void LocalSearch<Route,
       for (auto v_rank : update_candidates) {
         best_gains[v_rank].assign(_nb_vehicles, Eval());
         best_priorities[v_rank] = 0;
+        best_removals[v_rank] = std::numeric_limits<unsigned>::max();
         best_ops[v_rank] = std::vector<std::unique_ptr<Operator>>(_nb_vehicles);
       }
 
@@ -1958,6 +1993,7 @@ void LocalSearch<Route,
         if (invalidate_move) {
           best_gains[v][v] = Eval();
           best_priorities[v] = 0;
+          best_removals[v] = std::numeric_limits<unsigned>::max();
           best_ops[v][v] = std::unique_ptr<Operator>();
           s_t_pairs.emplace_back(v, v);
         }

src/problems/cvrp/operators/priority_replace.cpp

@@ -35,10 +35,13 @@ PriorityReplace::PriorityReplace(const Input& input,
                          _sol_state.fwd_priority[s_vehicle][s_rank]),
     _end_priority_gain(_input.jobs[u].priority -
                        _sol_state.bwd_priority[s_vehicle][t_rank]),
+    _start_assigned_number(s_route.size() - s_rank),
+    _end_assigned_number(t_rank + 1),
     _u(u),
     _best_known_priority_gain(best_known_priority_gain),
     _unassigned(unassigned) {
   assert(!s_route.empty());
+  assert(t_rank != 0);
   assert(_start_priority_gain > 0 or _end_priority_gain > 0);
 }
 
@@ -111,8 +114,8 @@ void PriorityReplace::compute_gain() {
 
   if (replace_start_valid && replace_end_valid) {
     // Decide based on priority and cost.
-    if (std::tie(_end_priority_gain, t_gain) <
-        std::tie(_start_priority_gain, s_gain)) {
+    if (std::tie(_end_priority_gain, _end_assigned_number, t_gain) <
+        std::tie(_start_priority_gain, _start_assigned_number, s_gain)) {
       replace_end_valid = false;
     } else {
       replace_start_valid = false;
@@ -144,6 +147,8 @@ bool PriorityReplace::is_valid() {
                                                   j.delivery,
                                                   0,
                                                   s_rank + 1);
+  assert(!replace_start_valid ||
+         !source.has_pending_delivery_after_rank(s_rank));
 
   // Don't bother if the candidate end portion is empty or with a
   // single job (that would be an UnassignedExchange move).
@@ -156,6 +161,8 @@ bool PriorityReplace::is_valid() {
                                                   j.delivery,
                                                   t_rank,
                                                   s_route.size());
+  assert(!replace_end_valid ||
+         !source.has_pending_delivery_after_rank(t_rank - 1));
 
   // Check validity with regard to vehicle range bounds, requires
   // valid gain values for both options.
@@ -216,6 +223,13 @@ Priority PriorityReplace::priority_gain() {
   return replace_start_valid ? _start_priority_gain : _end_priority_gain;
 }
 
+unsigned PriorityReplace::assigned() const {
+  assert(gain_computed);
+  assert(replace_start_valid xor replace_end_valid);
+
+  return replace_start_valid ? _start_assigned_number : _end_assigned_number;
+}
+
 std::vector<Index> PriorityReplace::addition_candidates() const {
   return {s_vehicle};
 }

src/problems/cvrp/operators/priority_replace.h

@@ -20,6 +20,8 @@ class PriorityReplace : public ls::Operator {
   bool _end_gain_computed{false};
   const Priority _start_priority_gain;
   const Priority _end_priority_gain;
+  const unsigned _start_assigned_number;
+  const unsigned _end_assigned_number;
 
 protected:
   const Index _u; // Unassigned job to insert.
@@ -55,6 +57,8 @@ class PriorityReplace : public ls::Operator {
 
   Priority priority_gain();
 
+  unsigned assigned() const;
+
   std::vector<Index> addition_candidates() const override;
 
   std::vector<Index> update_candidates() const override;