From ef298bfe70d43673e6fd8cb1582fe1b53d638632 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 15 Jan 2025 18:10:26 +0100 Subject: [PATCH 01/13] Filter out invalid moves in PriorityReplace::is_valid. --- src/problems/cvrp/operators/priority_replace.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/problems/cvrp/operators/priority_replace.cpp b/src/problems/cvrp/operators/priority_replace.cpp index 1bcf797e..c9f3ff2f 100644 --- a/src/problems/cvrp/operators/priority_replace.cpp +++ b/src/problems/cvrp/operators/priority_replace.cpp @@ -139,6 +139,7 @@ bool PriorityReplace::is_valid() { replace_start_valid = (0 < _start_priority_gain) && (_best_known_priority_gain <= _start_priority_gain) && (s_rank > 0) && + (!source.has_pending_delivery_after_rank(s_rank)) && source.is_valid_addition_for_capacity_margins(_input, j.pickup, j.delivery, @@ -151,6 +152,7 @@ bool PriorityReplace::is_valid() { (0 < _end_priority_gain) && (_best_known_priority_gain <= _end_priority_gain) && (t_rank < s_route.size() - 1) && + (t_rank == 0 || !source.has_pending_delivery_after_rank(t_rank - 1)) && source.is_valid_addition_for_capacity_margins(_input, j.pickup, j.delivery, From 45313c832dfdf6416e0ce0c514189db135d30418 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 15 Jan 2025 18:43:06 +0100 Subject: [PATCH 02/13] Find valid smaller route start replacement if possible. --- src/algorithms/local_search/local_search.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index 457ddd52..1c981484 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -439,7 +439,8 @@ void LocalSearch 0) ? fwd_over_rank - 1 : 0; + 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]; From 1afbe669e393171ad61a2bad17e0005b319510e9 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 15 Jan 2025 18:44:10 +0100 Subject: [PATCH 03/13] Turn void start validity check into assertion. --- src/problems/cvrp/operators/priority_replace.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/problems/cvrp/operators/priority_replace.cpp b/src/problems/cvrp/operators/priority_replace.cpp index c9f3ff2f..1801998d 100644 --- a/src/problems/cvrp/operators/priority_replace.cpp +++ b/src/problems/cvrp/operators/priority_replace.cpp @@ -139,12 +139,13 @@ bool PriorityReplace::is_valid() { replace_start_valid = (0 < _start_priority_gain) && (_best_known_priority_gain <= _start_priority_gain) && (s_rank > 0) && - (!source.has_pending_delivery_after_rank(s_rank)) && source.is_valid_addition_for_capacity_margins(_input, j.pickup, 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). From 230c182665fca3169a397b8bcbe6d13212c5334f Mon Sep 17 00:00:00 2001 From: jcoupey Date: Thu, 16 Jan 2025 11:19:00 +0100 Subject: [PATCH 04/13] Account for assigned tasks when deciding about start or end replace. --- src/problems/cvrp/operators/priority_replace.cpp | 6 ++++-- src/problems/cvrp/operators/priority_replace.h | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/problems/cvrp/operators/priority_replace.cpp b/src/problems/cvrp/operators/priority_replace.cpp index 1801998d..cff29fd0 100644 --- a/src/problems/cvrp/operators/priority_replace.cpp +++ b/src/problems/cvrp/operators/priority_replace.cpp @@ -35,6 +35,8 @@ 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) { @@ -111,8 +113,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; diff --git a/src/problems/cvrp/operators/priority_replace.h b/src/problems/cvrp/operators/priority_replace.h index 4aac4644..c48a5669 100644 --- a/src/problems/cvrp/operators/priority_replace.h +++ b/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. From b3379caf34f1513aba1140612bcc484738145bca Mon Sep 17 00:00:00 2001 From: jcoupey Date: Thu, 16 Jan 2025 11:20:08 +0100 Subject: [PATCH 05/13] Find valid smaller route end replacement if possible. --- src/algorithms/local_search/local_search.cpp | 17 +++++++++++++++-- .../cvrp/operators/priority_replace.cpp | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index 1c981484..69277254 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -460,14 +460,27 @@ void LocalSearch 0 or _end_priority_gain > 0); } From b999d2796ff2d782134ebc13b092f259b0c9956e Mon Sep 17 00:00:00 2001 From: jcoupey Date: Thu, 16 Jan 2025 11:20:32 +0100 Subject: [PATCH 06/13] Turn void end validity check into assertion. --- src/problems/cvrp/operators/priority_replace.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/problems/cvrp/operators/priority_replace.cpp b/src/problems/cvrp/operators/priority_replace.cpp index b683d532..38a48263 100644 --- a/src/problems/cvrp/operators/priority_replace.cpp +++ b/src/problems/cvrp/operators/priority_replace.cpp @@ -156,12 +156,13 @@ bool PriorityReplace::is_valid() { (0 < _end_priority_gain) && (_best_known_priority_gain <= _end_priority_gain) && (t_rank < s_route.size() - 1) && - (t_rank == 0 || !source.has_pending_delivery_after_rank(t_rank - 1)) && source.is_valid_addition_for_capacity_margins(_input, j.pickup, 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. From d8c3c8a38713e0ac74658c5f7af51e51465be119 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Thu, 16 Jan 2025 11:49:35 +0100 Subject: [PATCH 07/13] Mention PriorityReplace fix. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9394c639..58b7675d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ #### Core solving - Solution quality regression when using lots of skills (#1193) +- Invalid route reached by `PriorityReplace` (#1162) - 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) From d61176340d147254d94c8e58275d60d166a11318 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 21 Jan 2025 15:09:23 +0100 Subject: [PATCH 08/13] Aim for net priority gains only in PriorityReplace. --- src/algorithms/local_search/local_search.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index 69277254..84207d07 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -444,7 +444,7 @@ void LocalSearch Date: Tue, 21 Jan 2025 15:25:06 +0100 Subject: [PATCH 09/13] Apply UnassignedExchange checks before PriorityReplace. --- src/algorithms/local_search/local_search.cpp | 196 ++++++++++--------- 1 file changed, 100 insertions(+), 96 deletions(-) diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index 84207d07..ba9eb4f7 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -422,102 +422,6 @@ void LocalSearch 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() && - (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(r); - } - } - } - } - // UnassignedExchange stuff for (const Index u : _sol_state.unassigned) { if (_input.jobs[u].type != JOB_TYPE::SINGLE) { @@ -623,6 +527,106 @@ void LocalSearch 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() && + (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(r); + } + } + } + } } // CrossExchange stuff From fad9d8a158eca8d6a94021a426188d8358568382 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 21 Jan 2025 16:08:38 +0100 Subject: [PATCH 10/13] Expose resulting assigned number from PriorityReplace move. --- src/problems/cvrp/operators/priority_replace.cpp | 7 +++++++ src/problems/cvrp/operators/priority_replace.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/src/problems/cvrp/operators/priority_replace.cpp b/src/problems/cvrp/operators/priority_replace.cpp index 38a48263..17871b4e 100644 --- a/src/problems/cvrp/operators/priority_replace.cpp +++ b/src/problems/cvrp/operators/priority_replace.cpp @@ -223,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 PriorityReplace::addition_candidates() const { return {s_vehicle}; } diff --git a/src/problems/cvrp/operators/priority_replace.h b/src/problems/cvrp/operators/priority_replace.h index c48a5669..8dd53195 100644 --- a/src/problems/cvrp/operators/priority_replace.h +++ b/src/problems/cvrp/operators/priority_replace.h @@ -57,6 +57,8 @@ class PriorityReplace : public ls::Operator { Priority priority_gain(); + unsigned assigned() const; + std::vector addition_candidates() const override; std::vector update_candidates() const override; From bdd3c20d2f5586267772087a2240880318b04d42 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 21 Jan 2025 16:13:01 +0100 Subject: [PATCH 11/13] Account for number of assigned tasks to pick best PriorityReplace move. --- src/algorithms/local_search/local_search.cpp | 36 ++++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index ba9eb4f7..2e8875a0 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -405,10 +405,11 @@ void LocalSearch(_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 best_priorities(_nb_vehicles, 0); + std::vector best_assigned(_nb_vehicles, 0); // Dummy init to enter first loop. Eval best_gain(static_cast(1)); @@ -516,6 +517,7 @@ void LocalSearch(r); + if (r.is_valid()) { + const auto priority_gain = r.priority_gain(); + const auto assigned = r.assigned(); + const auto gain = r.gain(); + if (std::tie(best_priorities[source], + best_assigned[source], + best_gains[source][source]) < + std::tie(priority_gain, assigned, gain)) { + best_priorities[source] = priority_gain; + best_assigned[source] = r.assigned(); + // This may potentially define a negative value as best + // gain. + best_gains[source][source] = r.gain(); + best_ops[source][source] = std::make_unique(r); + } } } } From 669989b01e707d55793c7b173f7f9647017ef6b8 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 21 Jan 2025 16:50:21 +0100 Subject: [PATCH 12/13] Account for number of removals to pick best priority-modifying move. --- src/algorithms/local_search/local_search.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index 2e8875a0..9f683ccb 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -409,7 +409,8 @@ void LocalSearch best_priorities(_nb_vehicles, 0); - std::vector best_assigned(_nb_vehicles, 0); + std::vector best_removals(_nb_vehicles, + std::numeric_limits::max()); // Dummy init to enter first loop. Eval best_gain(static_cast(1)); @@ -517,7 +518,7 @@ void LocalSearch::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; From 1564f9828ccc35341a465cec046a516692cf7e1e Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 29 Jan 2025 10:19:07 +0100 Subject: [PATCH 13/13] Properly update best_removals. --- CHANGELOG.md | 1 + src/algorithms/local_search/local_search.cpp | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58b7675d..414258d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ - 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) diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index 9f683ccb..f19034bd 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -415,6 +415,7 @@ void LocalSearch(1)); Priority best_priority = 0; + auto best_removal = std::numeric_limits::max(); while (best_gain.cost > 0 || best_priority > 0) { if (_deadline.has_value() && _deadline.value() < utils::now()) { @@ -1838,7 +1839,7 @@ void LocalSearch::max(); + best_removal = std::numeric_limits::max(); best_gain = Eval(); Index best_source = 0; Index best_target = 0; @@ -1949,6 +1950,7 @@ void LocalSearch::max(); best_ops[v_rank] = std::vector>(_nb_vehicles); } @@ -1991,6 +1993,7 @@ void LocalSearch::max(); best_ops[v][v] = std::unique_ptr(); s_t_pairs.emplace_back(v, v); }