From a7ec8b315a2ab4caab9991c924fd54fdff73380d Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 12:22:42 +0100 Subject: [PATCH 01/27] Constexpr inline is redundant. --- src/structures/typedefs.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/structures/typedefs.h b/src/structures/typedefs.h index d4549577..f57d120a 100644 --- a/src/structures/typedefs.h +++ b/src/structures/typedefs.h @@ -208,21 +208,21 @@ struct StringHash { }; namespace utils { -constexpr inline Duration scale_from_user_duration(UserDuration d) { +constexpr Duration scale_from_user_duration(UserDuration d) { return DURATION_FACTOR * static_cast(d); } -constexpr inline UserDuration scale_to_user_duration(Duration d) { +constexpr UserDuration scale_to_user_duration(Duration d) { assert(d <= scale_from_user_duration(std::numeric_limits::max())); return static_cast(d / DURATION_FACTOR); } -constexpr inline Cost scale_from_user_cost(UserCost c) { +constexpr Cost scale_from_user_cost(UserCost c) { return DURATION_FACTOR * COST_FACTOR * static_cast(c); } -constexpr inline UserCost scale_to_user_cost(Cost c) { +constexpr UserCost scale_to_user_cost(Cost c) { assert(c <= scale_from_user_cost(std::numeric_limits::max())); return static_cast(c / (DURATION_FACTOR * COST_FACTOR)); } From 35338e6e1c83615cbc87438a9274715280c9bf2e Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 12:25:12 +0100 Subject: [PATCH 02/27] Do not use alternative operators. --- libvroom_examples/libvroom.cpp | 2 +- src/algorithms/validation/choose_ETA.cpp | 2 +- src/structures/typedefs.h | 5 ----- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/libvroom_examples/libvroom.cpp b/libvroom_examples/libvroom.cpp index 027c0d12..92c6b3cf 100644 --- a/libvroom_examples/libvroom.cpp +++ b/libvroom_examples/libvroom.cpp @@ -64,7 +64,7 @@ void log_solution(const vroom::Solution& sol, bool geometry) { std::cout << type; // Add job/pickup/delivery/break ids. - if (step.step_type != vroom::STEP_TYPE::START and + if (step.step_type != vroom::STEP_TYPE::START && step.step_type != vroom::STEP_TYPE::END) { std::cout << " " << step.id; } diff --git a/src/algorithms/validation/choose_ETA.cpp b/src/algorithms/validation/choose_ETA.cpp index c15d5a12..3de7cef8 100644 --- a/src/algorithms/validation/choose_ETA.cpp +++ b/src/algorithms/validation/choose_ETA.cpp @@ -298,7 +298,7 @@ Route choose_ETA(const Input& input, get_violation(v.breaks[step.rank].tws, earliest_date); const auto& tws = v.breaks[step.rank].tws; - if ((tws.size() != 1) or !tws.front().is_default()) { + if ((tws.size() != 1) || !tws.front().is_default()) { step_has_TW[s] = true; horizon_start_lead_times[s] = tws.front().start - horizon_start; diff --git a/src/structures/typedefs.h b/src/structures/typedefs.h index f57d120a..90a66c45 100644 --- a/src/structures/typedefs.h +++ b/src/structures/typedefs.h @@ -20,11 +20,6 @@ All rights reserved (see LICENSE). #include #include -#ifdef _MSC_VER -// include support for "and"/"or" -#include -#endif - namespace vroom { // To easily differentiate variable types. From 8b460d232945d9023f0a1f3c56f9c3a73ca629cd Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 12:32:24 +0100 Subject: [PATCH 03/27] Do not use size to check for emptyness. --- src/problems/cvrp/operators/mixed_exchange.cpp | 2 +- src/problems/cvrp/operators/relocate.cpp | 2 +- src/problems/cvrp/operators/reverse_two_opt.cpp | 4 ++-- src/problems/cvrp/operators/route_exchange.cpp | 2 +- src/problems/cvrp/operators/swap_star.cpp | 4 ++-- src/problems/cvrp/operators/two_opt.cpp | 4 ++-- src/utils/input_parser.cpp | 6 +++--- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/problems/cvrp/operators/mixed_exchange.cpp b/src/problems/cvrp/operators/mixed_exchange.cpp index eb791b44..7b9de232 100644 --- a/src/problems/cvrp/operators/mixed_exchange.cpp +++ b/src/problems/cvrp/operators/mixed_exchange.cpp @@ -36,7 +36,7 @@ MixedExchange::MixedExchange(const Input& input, target_delivery(_input.jobs[this->t_route[t_rank]].delivery + _input.jobs[this->t_route[t_rank + 1]].delivery) { assert(s_vehicle != t_vehicle); - assert(s_route.size() >= 1); + assert(!s_route.empty()); assert(t_route.size() >= 2); assert(s_rank < s_route.size()); assert(t_rank < t_route.size() - 1); diff --git a/src/problems/cvrp/operators/relocate.cpp b/src/problems/cvrp/operators/relocate.cpp index c1c44c6c..a179f8cc 100644 --- a/src/problems/cvrp/operators/relocate.cpp +++ b/src/problems/cvrp/operators/relocate.cpp @@ -30,7 +30,7 @@ Relocate::Relocate(const Input& input, t_vehicle, t_rank) { assert(s_vehicle != t_vehicle); - assert(s_route.size() >= 1); + assert(!s_route.empty()); assert(s_rank < s_route.size()); assert(t_rank <= t_route.size()); diff --git a/src/problems/cvrp/operators/reverse_two_opt.cpp b/src/problems/cvrp/operators/reverse_two_opt.cpp index 8d1824fc..03028719 100644 --- a/src/problems/cvrp/operators/reverse_two_opt.cpp +++ b/src/problems/cvrp/operators/reverse_two_opt.cpp @@ -31,8 +31,8 @@ ReverseTwoOpt::ReverseTwoOpt(const Input& input, _s_delivery(source.bwd_deliveries(s_rank)), _t_delivery(target.fwd_deliveries(t_rank)) { assert(s_vehicle != t_vehicle); - assert(s_route.size() >= 1); - assert(t_route.size() >= 1); + assert(!s_route.empty()); + assert(!t_route.empty()); assert(s_rank < s_route.size()); assert(t_rank < t_route.size()); diff --git a/src/problems/cvrp/operators/route_exchange.cpp b/src/problems/cvrp/operators/route_exchange.cpp index 6b516f41..f2a9f728 100644 --- a/src/problems/cvrp/operators/route_exchange.cpp +++ b/src/problems/cvrp/operators/route_exchange.cpp @@ -27,7 +27,7 @@ RouteExchange::RouteExchange(const Input& input, t_vehicle, 0) { // Dummy value assert(s_vehicle != t_vehicle); - assert(s_route.size() >= 1 || t_route.size() >= 1); + assert(!s_route.empty() || !t_route.empty()); // Whole routes should be transferable. assert(_sol_state.bwd_skill_rank[s_vehicle][t_vehicle] == 0); diff --git a/src/problems/cvrp/operators/swap_star.cpp b/src/problems/cvrp/operators/swap_star.cpp index 6da5d60c..afbc6f58 100644 --- a/src/problems/cvrp/operators/swap_star.cpp +++ b/src/problems/cvrp/operators/swap_star.cpp @@ -30,8 +30,8 @@ SwapStar::SwapStar(const Input& input, 0), _best_known_gain(best_known_gain) { assert(s_vehicle != t_vehicle); - assert(s_route.size() >= 1); - assert(t_route.size() >= 1); + assert(!s_route.empty()); + assert(!t_route.empty()); assert(_input.vehicle_ok_with_vehicle(s_vehicle, t_vehicle)); } diff --git a/src/problems/cvrp/operators/two_opt.cpp b/src/problems/cvrp/operators/two_opt.cpp index 78adc6c9..570a05be 100644 --- a/src/problems/cvrp/operators/two_opt.cpp +++ b/src/problems/cvrp/operators/two_opt.cpp @@ -31,8 +31,8 @@ TwoOpt::TwoOpt(const Input& input, _s_delivery(source.bwd_deliveries(s_rank)), _t_delivery(target.bwd_deliveries(t_rank)) { assert(s_vehicle != t_vehicle); - assert(s_route.size() >= 1); - assert(t_route.size() >= 1); + assert(!s_route.empty()); + assert(!t_route.empty()); assert(s_rank < s_route.size()); assert(t_rank < t_route.size()); diff --git a/src/utils/input_parser.cpp b/src/utils/input_parser.cpp index bdcad24d..0b01e678 100644 --- a/src/utils/input_parser.cpp +++ b/src/utils/input_parser.cpp @@ -532,9 +532,9 @@ void parse(Input& input, const std::string& input_str, bool geometry) { const auto& first_vehicle = json_input["vehicles"][0]; check_id(first_vehicle, "vehicle"); - bool first_vehicle_has_capacity = (first_vehicle.HasMember("capacity") && - first_vehicle["capacity"].IsArray() && - first_vehicle["capacity"].Size() > 0); + bool first_vehicle_has_capacity = + (first_vehicle.HasMember("capacity") && + first_vehicle["capacity"].IsArray() && !first_vehicle["capacity"].Empty()); const unsigned amount_size = first_vehicle_has_capacity ? first_vehicle["capacity"].Size() : 0; From 925c3eef5e0a5e542efea980be0a095b8ab65411 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 12:57:32 +0100 Subject: [PATCH 04/27] Use std::format in a couple places. --- src/algorithms/validation/choose_ETA.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/algorithms/validation/choose_ETA.cpp b/src/algorithms/validation/choose_ETA.cpp index 3de7cef8..bf10157d 100644 --- a/src/algorithms/validation/choose_ETA.cpp +++ b/src/algorithms/validation/choose_ETA.cpp @@ -992,8 +992,7 @@ Route choose_ETA(const Input& input, auto status = glp_mip_status(lp); if (status == GLP_UNDEF || status == GLP_NOFEAS) { - throw InputException("Infeasible route for vehicle " + - std::to_string(v.id) + "."); + throw InputException(std::format("Infeasible route for vehicle {}.", v.id)); } // We should not get GLP_FEAS. assert(status == GLP_OPT); @@ -1042,8 +1041,7 @@ Route choose_ETA(const Input& input, status = glp_mip_status(lp); if (status == GLP_UNDEF || status == GLP_NOFEAS) { - throw InputException("Infeasible route for vehicle " + - std::to_string(v.id) + "."); + throw InputException(std::format("Infeasible route for vehicle {}.", v.id)); } // We should not get GLP_FEAS. assert(status == GLP_OPT); From b8325fd41b1235e5d8653789b7e324e0388bc8b7 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 13:03:56 +0100 Subject: [PATCH 05/27] Use default operator== whenever possible. --- src/structures/vroom/eval.h | 10 +--------- src/structures/vroom/vehicle.h | 6 ++---- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/structures/vroom/eval.h b/src/structures/vroom/eval.h index c11bc795..6fb15b0e 100644 --- a/src/structures/vroom/eval.h +++ b/src/structures/vroom/eval.h @@ -65,15 +65,7 @@ struct Eval { return lhs.cost <= rhs.cost; } - friend bool operator==(const Eval& lhs, const Eval& rhs) { - return lhs.cost == rhs.cost && lhs.duration == rhs.duration && - lhs.distance == rhs.distance; - } - - friend bool operator!=(const Eval& lhs, const Eval& rhs) { - return lhs.cost != rhs.cost || lhs.duration != rhs.duration || - lhs.distance != rhs.distance; - } + friend bool operator==(const Eval& lhs, const Eval& rhs) = default; }; constexpr Eval NO_EVAL = {std::numeric_limits::max(), 0, 0}; diff --git a/src/structures/vroom/vehicle.h b/src/structures/vroom/vehicle.h index b4408dff..48b82337 100644 --- a/src/structures/vroom/vehicle.h +++ b/src/structures/vroom/vehicle.h @@ -37,10 +37,8 @@ struct VehicleCosts { per_hour(static_cast(per_hour)), per_km(static_cast(per_km)){}; - friend bool operator==(const VehicleCosts& lhs, const VehicleCosts& rhs) { - return lhs.fixed == rhs.fixed && lhs.per_hour == rhs.per_hour && - lhs.per_km == rhs.per_km; - } + friend bool operator==(const VehicleCosts& lhs, + const VehicleCosts& rhs) = default; friend bool operator<(const VehicleCosts& lhs, const VehicleCosts& rhs) { return std::tie(lhs.fixed, lhs.per_hour, lhs.per_km) < From 1c9c105d28997f6a74a30fd5c3895e04ee5cddb8 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 13:45:12 +0100 Subject: [PATCH 06/27] Remove nested if statements. --- .../local_search/route_split_utils.h | 84 +++++++++---------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/src/algorithms/local_search/route_split_utils.h b/src/algorithms/local_search/route_split_utils.h index f8d9caa0..9ba933ec 100644 --- a/src/algorithms/local_search/route_split_utils.h +++ b/src/algorithms/local_search/route_split_utils.h @@ -88,28 +88,26 @@ compute_best_route_split_choice(const Input& input, continue; } - if (current_end_eval < second_best_end_eval) { - // Worth checking end route full validity. - - if (empty_routes[v_rank].is_valid_addition_for_tw(input, - end_delivery, - source.route.begin() + - r, - source.route.end(), - 0, - 0)) { - if (current_end_eval < first_best_end_eval) { - // We have a new first choice. - second_v_end = first_v_end; - second_best_end_eval = first_best_end_eval; - - first_v_end = v_rank; - first_best_end_eval = current_end_eval; - } else { - // We have a new second choice. - second_v_end = v_rank; - second_best_end_eval = current_end_eval; - } + if (current_end_eval < second_best_end_eval && + // Worth checking end route full validity. + empty_routes[v_rank].is_valid_addition_for_tw(input, + end_delivery, + source.route.begin() + + r, + source.route.end(), + 0, + 0)) { + if (current_end_eval < first_best_end_eval) { + // We have a new first choice. + second_v_end = first_v_end; + second_best_end_eval = first_best_end_eval; + + first_v_end = v_rank; + first_best_end_eval = current_end_eval; + } else { + // We have a new second choice. + second_v_end = v_rank; + second_best_end_eval = current_end_eval; } } } @@ -160,28 +158,26 @@ compute_best_route_split_choice(const Input& input, continue; } - if (current_begin_eval < second_best_begin_eval) { - // Worth checking begin route full validity. - - if (empty_routes[v_rank].is_valid_addition_for_tw(input, - begin_delivery, - source.route.begin(), - source.route.begin() + - r, - 0, - 0)) { - if (current_begin_eval < first_best_begin_eval) { - // We have a new first choice. - second_v_begin = first_v_begin; - second_best_begin_eval = first_best_begin_eval; - - first_v_begin = v_rank; - first_best_begin_eval = current_begin_eval; - } else { - // We have a new second choice. - second_v_begin = v_rank; - second_best_begin_eval = current_begin_eval; - } + if (current_begin_eval < second_best_begin_eval && + // Worth checking begin route full validity. + empty_routes[v_rank].is_valid_addition_for_tw(input, + begin_delivery, + source.route.begin(), + source.route.begin() + + r, + 0, + 0)) { + if (current_begin_eval < first_best_begin_eval) { + // We have a new first choice. + second_v_begin = first_v_begin; + second_best_begin_eval = first_best_begin_eval; + + first_v_begin = v_rank; + first_best_begin_eval = current_begin_eval; + } else { + // We have a new second choice. + second_v_begin = v_rank; + second_best_begin_eval = current_begin_eval; } } } From 88711dc4fa20c18647da55d295f6f2c99b91f5f3 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 13:57:31 +0100 Subject: [PATCH 07/27] Use raw string literal. --- src/routing/ors_wrapper.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/routing/ors_wrapper.cpp b/src/routing/ors_wrapper.cpp index 9e5bcb94..e2b676b4 100644 --- a/src/routing/ors_wrapper.cpp +++ b/src/routing/ors_wrapper.cpp @@ -18,8 +18,7 @@ OrsWrapper::OrsWrapper(const std::string& profile, const Server& server) "durations", "distances", "directions", - "\"geometry_simplify\":\"false\",\"continue_straight\":" - "\"false\"") { + R"("geometry_simplify":"false","continue_straight":"false")") { } std::string OrsWrapper::build_query(const std::vector& locations, @@ -41,7 +40,7 @@ std::string OrsWrapper::build_query(const std::vector& locations, body += "," + _routing_args; } else { assert(service == _matrix_service); - body += ",\"metrics\":[\"duration\",\"distance\"]"; + body += R"(,"metrics":["duration","distance"])"; } body += "}"; From 7ffe35a796089f675e77c57f226a80d28afaf936 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 14:33:46 +0100 Subject: [PATCH 08/27] Use a lambda instead of taking the address of library functions. --- src/problems/tsp/tsp.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/problems/tsp/tsp.cpp b/src/problems/tsp/tsp.cpp index a6021948..52272270 100644 --- a/src/problems/tsp/tsp.cpp +++ b/src/problems/tsp/tsp.cpp @@ -134,13 +134,15 @@ TSP::TSP(const Input& input, std::vector&& job_ranks, Index vehicle_rank) // Compute symmetrized matrix and update _is_symmetric flag. _symmetrized_matrix = Matrix(_matrix.size()); - const UserCost& (*sym_f)(const UserCost&, const UserCost&) = - std::min; - if ((_has_start && !_has_end) || (!_has_start && _has_end)) { - // Using symmetrization with max as when only start or only end is - // forced, the matrix has a line or a column filled with zeros. - sym_f = std::max; - } + // Using symmetrization with max when only start or only end is + // forced, as the matrix has a line or a column filled with zeros. + const bool sym_with_max = + ((_has_start && !_has_end) || (!_has_start && _has_end)); + + const auto sym_f = + sym_with_max + ? [](const UserCost& c1, const UserCost& c2) { return std::max(c1, c2); } + : [](const UserCost& c1, const UserCost& c2) { return std::min(c1, c2); }; for (Index i = 0; i < _matrix.size(); ++i) { _symmetrized_matrix[i][i] = _matrix[i][i]; for (Index j = i + 1; j < _matrix.size(); ++j) { From 42b59cdd764d3cd98b77703ac805129b165de97c Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 14:44:00 +0100 Subject: [PATCH 09/27] Make sure to use operator |= with bool variables. --- src/algorithms/local_search/insertion_search.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/algorithms/local_search/insertion_search.h b/src/algorithms/local_search/insertion_search.h index 8e8538d8..128477b8 100644 --- a/src/algorithms/local_search/insertion_search.h +++ b/src/algorithms/local_search/insertion_search.h @@ -120,7 +120,7 @@ RouteInsertion compute_best_insertion_pd(const Input& input, valid_delivery_insertions[d_rank] = route.is_valid_addition_for_tw_without_max_load(input, j + 1, d_rank); } - found_valid |= valid_delivery_insertions[d_rank]; + found_valid |= static_cast(valid_delivery_insertions[d_rank]); } if (!found_valid) { From 1f9cf284bd569e69a97172843f4cf911c97758be Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 15:34:47 +0100 Subject: [PATCH 10/27] Use transparent equality and hasher with std::unordered_set. --- src/structures/vroom/input/input.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/structures/vroom/input/input.h b/src/structures/vroom/input/input.h index 93ee31a6..be9479dd 100644 --- a/src/structures/vroom/input/input.h +++ b/src/structures/vroom/input/input.h @@ -38,7 +38,7 @@ class Input { TimePoint _end_loading; TimePoint _end_solving; TimePoint _end_routing; - std::unordered_set _profiles; + std::unordered_set> _profiles; std::vector> _routing_wrappers; bool _apply_TSPFix; bool _no_addition_yet{true}; From 78698193ee30c83dfba75d1bb81991e5e4709c4c Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 17:27:08 +0100 Subject: [PATCH 11/27] Handle part of the send_then_receive work in functions. --- src/routing/http_wrapper.cpp | 58 ++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/src/routing/http_wrapper.cpp b/src/routing/http_wrapper.cpp index e0bccb2c..1224548c 100644 --- a/src/routing/http_wrapper.cpp +++ b/src/routing/http_wrapper.cpp @@ -36,6 +36,36 @@ HttpWrapper::HttpWrapper(const std::string& profile, _routing_args(std::move(routing_args)) { } +void set_response(auto& s, std::string& response) { + char buf[512]; // NOLINT + std::error_code error; + for (;;) { + std::size_t len = s.read_some(asio::buffer(buf), error); + response.append(buf, len); // NOLINT + if (error == asio::error::eof) { + // Connection closed cleanly. + break; + } + if (error) { + throw std::system_error(error); + } + } +} + +std::string get_json(const std::string& response) { + // Removing headers. + auto start = response.find('{'); + if (start == std::string::npos) { + throw RoutingException("Invalid routing response: " + response); + } + auto end = response.rfind('}'); + if (end == std::string::npos) { + throw RoutingException("Invalid routing response: " + response); + } + + return response.substr(start, end - start + 1); +} + std::string HttpWrapper::send_then_receive(const std::string& query) const { std::string response; @@ -51,37 +81,13 @@ std::string HttpWrapper::send_then_receive(const std::string& query) const { asio::write(s, asio::buffer(query)); - char buf[512]; // NOLINT - std::error_code error; - for (;;) { - std::size_t len = s.read_some(asio::buffer(buf), error); - response.append(buf, len); // NOLINT - if (error == asio::error::eof) { - // Connection closed cleanly. - break; - } - if (error) { - throw std::system_error(error); - } - } + set_response(s, response); } catch (std::system_error&) { throw RoutingException("Failed to connect to " + _server.host + ":" + _server.port); } - // Removing headers. - auto start = response.find('{'); - if (start == std::string::npos) { - throw RoutingException("Invalid routing response: " + response); - } - auto end = response.rfind('}'); - if (end == std::string::npos) { - throw RoutingException("Invalid routing response: " + response); - } - - std::string json_string = response.substr(start, end - start + 1); - - return json_string; + return get_json(response); } std::string HttpWrapper::ssl_send_then_receive(const std::string& query) const { From b7574a44090104e10e9ad9d0895142dddbb38132 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 17:27:38 +0100 Subject: [PATCH 12/27] Reduce duplication in ssl_send_then_receive. --- src/routing/http_wrapper.cpp | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/src/routing/http_wrapper.cpp b/src/routing/http_wrapper.cpp index 1224548c..2dc7a158 100644 --- a/src/routing/http_wrapper.cpp +++ b/src/routing/http_wrapper.cpp @@ -108,36 +108,13 @@ std::string HttpWrapper::ssl_send_then_receive(const std::string& query) const { asio::write(ssock, asio::buffer(query)); - char buf[512]; // NOLINT - std::error_code error; - for (;;) { - std::size_t len = ssock.read_some(asio::buffer(buf), error); - response.append(buf, len); // NOLINT - if (error == asio::error::eof) { - // Connection closed cleanly. - break; - } - if (error) { - throw std::system_error(error); - } - } + set_response(ssock, response); } catch (std::system_error&) { throw RoutingException("Failed to connect to " + _server.host + ":" + _server.port); } - // Removing headers. - auto start = response.find('{'); - if (start == std::string::npos) { - throw RoutingException("Invalid routing response: " + response); - } - auto end = response.rfind('}'); - if (end == std::string::npos) { - throw RoutingException("Invalid routing response: " + response); - } - std::string json_string = response.substr(start, end - start + 1); - - return json_string; + return get_json(response); } std::string HttpWrapper::run_query(const std::string& query) const { From 499ba86f30851a3f1276a980c4b458881fa3a68c Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 20:39:23 +0100 Subject: [PATCH 13/27] Apply some readability fixes. --- .clang-tidy | 1 + src/routing/http_wrapper.cpp | 2 +- src/routing/wrapper.h | 7 +++---- src/structures/cl_args.cpp | 2 +- src/structures/typedefs.h | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 09a8be00..8f7285d2 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -38,6 +38,7 @@ Checks: > -readability-convert-member-functions-to-static, -readability-implicit-bool-conversion, -readability-avoid-const-params-in-decls + -readability-uppercase-literal-suffix WarningsAsErrors: '*' HeaderFilterRegex: '.*' diff --git a/src/routing/http_wrapper.cpp b/src/routing/http_wrapper.cpp index 2dc7a158..37044a98 100644 --- a/src/routing/http_wrapper.cpp +++ b/src/routing/http_wrapper.cpp @@ -138,7 +138,7 @@ Matrices HttpWrapper::get_matrices(const std::vector& locs) const { const std::size_t m_size = locs.size(); rapidjson::Document json_result; - this->parse_response(json_result, json_string); + HttpWrapper::parse_response(json_result, json_string); this->check_response(json_result, locs, _matrix_service); if (!json_result.HasMember(_matrix_durations_key.c_str())) { diff --git a/src/routing/wrapper.h b/src/routing/wrapper.h index fbe2db61..5523e29f 100644 --- a/src/routing/wrapper.h +++ b/src/routing/wrapper.h @@ -123,10 +123,9 @@ class Wrapper { explicit Wrapper(std::string profile) : profile(std::move(profile)) { } - static inline void - check_unfound(const std::vector& locs, - const std::vector& nb_unfound_from_loc, - const std::vector& nb_unfound_to_loc) { + static void check_unfound(const std::vector& locs, + const std::vector& nb_unfound_from_loc, + const std::vector& nb_unfound_to_loc) { assert(nb_unfound_from_loc.size() == nb_unfound_to_loc.size()); unsigned max_unfound_routes_for_a_loc = 0; unsigned error_loc = 0; // Initial value never actually used. diff --git a/src/structures/cl_args.cpp b/src/structures/cl_args.cpp index 5dfeac31..c3a828b5 100644 --- a/src/structures/cl_args.cpp +++ b/src/structures/cl_args.cpp @@ -18,7 +18,7 @@ void update_host(Servers& servers, std::string_view value) { // Determine profile and host from a "car:0.0.0.0"-like value. std::string profile = DEFAULT_PROFILE; std::string host; - std::string path = ""; + std::string path; auto index = value.find(':'); if (index == std::string::npos) { diff --git a/src/structures/typedefs.h b/src/structures/typedefs.h index 90a66c45..0a0180fb 100644 --- a/src/structures/typedefs.h +++ b/src/structures/typedefs.h @@ -91,7 +91,7 @@ enum class ROUTER { OSRM, LIBOSRM, ORS, VALHALLA }; struct Server { std::string host; std::string port; - std::string path{""}; + std::string path; Server() : host("0.0.0.0"), port("5000") { } From de07144e3a15c83b2618e4ae501e4d721b4aebb7 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 21:40:45 +0100 Subject: [PATCH 14/27] Add pointer qualification to auto-typed variable deduced to pointer. --- src/algorithms/validation/choose_ETA.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/algorithms/validation/choose_ETA.cpp b/src/algorithms/validation/choose_ETA.cpp index bf10157d..5e9663e0 100644 --- a/src/algorithms/validation/choose_ETA.cpp +++ b/src/algorithms/validation/choose_ETA.cpp @@ -682,7 +682,7 @@ Route choose_ETA(const Input& input, // Makespan and \sum Y_i dummy constraints (used for second solving // phase). - auto name = "Makespan"; + const auto* name = "Makespan"; glp_set_row_name(lp, current_row, name); glp_set_row_bnds(lp, current_row, GLP_LO, 0, 0); From 91f5e38382edb6b714a3febada15b938bff5dcab Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 21:53:45 +0100 Subject: [PATCH 15/27] Use small underlying type for enums. --- src/structures/typedefs.h | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/structures/typedefs.h b/src/structures/typedefs.h index 0a0180fb..9ab22f9f 100644 --- a/src/structures/typedefs.h +++ b/src/structures/typedefs.h @@ -85,7 +85,7 @@ constexpr auto DEFAULT_MAX_TRAVEL_TIME = std::numeric_limits::max(); constexpr auto DEFAULT_MAX_DISTANCE = std::numeric_limits::max(); // Available routing engines. -enum class ROUTER { OSRM, LIBOSRM, ORS, VALHALLA }; +enum class ROUTER : std::uint8_t { OSRM, LIBOSRM, ORS, VALHALLA }; // Used to describe a routing server. struct Server { @@ -107,15 +107,21 @@ struct Server { // 'Single' job is a regular one-stop job without precedence // constraints. -enum class JOB_TYPE { SINGLE, PICKUP, DELIVERY }; +enum class JOB_TYPE : std::uint8_t { SINGLE, PICKUP, DELIVERY }; // Available location status. -enum class STEP_TYPE { START, JOB, BREAK, END }; +enum class STEP_TYPE : std::uint8_t { START, JOB, BREAK, END }; // Heuristic options. -enum class HEURISTIC { BASIC, DYNAMIC }; -enum class INIT { NONE, HIGHER_AMOUNT, NEAREST, FURTHEST, EARLIEST_DEADLINE }; -enum class SORT { AVAILABILITY, COST }; +enum class HEURISTIC : std::uint8_t { BASIC, DYNAMIC }; +enum class INIT : std::uint8_t { + NONE, + HIGHER_AMOUNT, + NEAREST, + FURTHEST, + EARLIEST_DEADLINE +}; +enum class SORT : std::uint8_t { AVAILABILITY, COST }; struct HeuristicParameters { HEURISTIC heuristic; @@ -132,7 +138,7 @@ struct HeuristicParameters { }; // Possible violations. -enum class VIOLATION { +enum class VIOLATION : std::uint8_t { LEAD_TIME, DELAY, LOAD, @@ -145,7 +151,7 @@ enum class VIOLATION { MAX_DISTANCE }; -enum OperatorName { +enum OperatorName : std::uint8_t { UnassignedExchange, CrossExchange, MixedExchange, From 6a30fb4acb289f5140978c6073605cca2f2fd149 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Tue, 10 Dec 2024 22:35:45 +0100 Subject: [PATCH 16/27] Remove some headers. --- src/algorithms/local_search/top_insertions.cpp | 1 - src/algorithms/validation/check.cpp | 2 +- src/algorithms/validation/choose_ETA.cpp | 1 - src/main.cpp | 1 - src/problems/cvrp/cvrp.cpp | 1 - src/problems/cvrp/operators/pd_shift.cpp | 1 - src/problems/cvrp/operators/priority_replace.cpp | 1 - src/problems/vrptw/operators/pd_shift.cpp | 1 - src/problems/vrptw/vrptw.cpp | 2 -- src/structures/vroom/solution_state.cpp | 1 - 10 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/algorithms/local_search/top_insertions.cpp b/src/algorithms/local_search/top_insertions.cpp index ebcc7fc1..a7db8ae3 100644 --- a/src/algorithms/local_search/top_insertions.cpp +++ b/src/algorithms/local_search/top_insertions.cpp @@ -8,7 +8,6 @@ All rights reserved (see LICENSE). */ #include "algorithms/local_search/top_insertions.h" -#include "structures/vroom/tw_route.h" #include "utils/helpers.h" namespace vroom::ls { diff --git a/src/algorithms/validation/check.cpp b/src/algorithms/validation/check.cpp index 1e086e57..77a3458f 100644 --- a/src/algorithms/validation/check.cpp +++ b/src/algorithms/validation/check.cpp @@ -8,6 +8,7 @@ All rights reserved (see LICENSE). */ #include +#include #include #include #include @@ -16,7 +17,6 @@ All rights reserved (see LICENSE). #include "algorithms/validation/check.h" #include "algorithms/validation/choose_ETA.h" #include "structures/vroom/tw_route.h" -#include "utils/exception.h" #include "utils/helpers.h" namespace vroom::validation { diff --git a/src/algorithms/validation/choose_ETA.cpp b/src/algorithms/validation/choose_ETA.cpp index 5e9663e0..6bb84d0b 100644 --- a/src/algorithms/validation/choose_ETA.cpp +++ b/src/algorithms/validation/choose_ETA.cpp @@ -9,7 +9,6 @@ All rights reserved (see LICENSE). #include #include -#include #include diff --git a/src/main.cpp b/src/main.cpp index f3076c0d..ed50033b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -17,7 +17,6 @@ All rights reserved (see LICENSE). #include "../include/cxxopts/include/cxxopts.hpp" -#include "problems/vrp.h" #include "structures/cl_args.h" #include "utils/exception.h" #include "utils/helpers.h" diff --git a/src/problems/cvrp/cvrp.cpp b/src/problems/cvrp/cvrp.cpp index ef877ea5..840f416f 100644 --- a/src/problems/cvrp/cvrp.cpp +++ b/src/problems/cvrp/cvrp.cpp @@ -8,7 +8,6 @@ All rights reserved (see LICENSE). */ #include "problems/cvrp/cvrp.h" -#include "algorithms/heuristics/heuristics.h" #include "algorithms/local_search/local_search.h" #include "problems/cvrp/operators/cross_exchange.h" #include "problems/cvrp/operators/intra_cross_exchange.h" diff --git a/src/problems/cvrp/operators/pd_shift.cpp b/src/problems/cvrp/operators/pd_shift.cpp index 4376abe4..ac07fde1 100644 --- a/src/problems/cvrp/operators/pd_shift.cpp +++ b/src/problems/cvrp/operators/pd_shift.cpp @@ -8,7 +8,6 @@ All rights reserved (see LICENSE). */ #include "problems/cvrp/operators/pd_shift.h" -#include "utils/helpers.h" #include "algorithms/local_search/insertion_search.h" diff --git a/src/problems/cvrp/operators/priority_replace.cpp b/src/problems/cvrp/operators/priority_replace.cpp index fa1ecc0d..1bcf797e 100644 --- a/src/problems/cvrp/operators/priority_replace.cpp +++ b/src/problems/cvrp/operators/priority_replace.cpp @@ -10,7 +10,6 @@ All rights reserved (see LICENSE). #include #include "problems/cvrp/operators/priority_replace.h" -#include "utils/helpers.h" namespace vroom::cvrp { diff --git a/src/problems/vrptw/operators/pd_shift.cpp b/src/problems/vrptw/operators/pd_shift.cpp index ee03b9e7..de5d1714 100644 --- a/src/problems/vrptw/operators/pd_shift.cpp +++ b/src/problems/vrptw/operators/pd_shift.cpp @@ -9,7 +9,6 @@ All rights reserved (see LICENSE). #include "problems/vrptw/operators/pd_shift.h" #include "algorithms/local_search/insertion_search.h" -#include "utils/helpers.h" namespace vroom::vrptw { diff --git a/src/problems/vrptw/vrptw.cpp b/src/problems/vrptw/vrptw.cpp index f051bb65..95d9c762 100644 --- a/src/problems/vrptw/vrptw.cpp +++ b/src/problems/vrptw/vrptw.cpp @@ -8,7 +8,6 @@ All rights reserved (see LICENSE). */ #include "problems/vrptw/vrptw.h" -#include "algorithms/heuristics/heuristics.h" #include "algorithms/local_search/local_search.h" #include "problems/vrptw/operators/cross_exchange.h" #include "problems/vrptw/operators/intra_cross_exchange.h" @@ -29,7 +28,6 @@ All rights reserved (see LICENSE). #include "problems/vrptw/operators/tsp_fix.h" #include "problems/vrptw/operators/two_opt.h" #include "problems/vrptw/operators/unassigned_exchange.h" -#include "utils/helpers.h" namespace vroom { diff --git a/src/structures/vroom/solution_state.cpp b/src/structures/vroom/solution_state.cpp index dbccba60..7edcb1bd 100644 --- a/src/structures/vroom/solution_state.cpp +++ b/src/structures/vroom/solution_state.cpp @@ -9,7 +9,6 @@ All rights reserved (see LICENSE). #include #include -#include #include "structures/vroom/solution_state.h" #include "utils/helpers.h" From d1700d38d836d67a05cfb1896ee8bfc8ba599530 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 09:52:00 +0100 Subject: [PATCH 17/27] Replace iterator-based loop with some range stuff. --- src/problems/tsp/heuristics/local_search.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/problems/tsp/heuristics/local_search.cpp b/src/problems/tsp/heuristics/local_search.cpp index e638da10..15eb7fc2 100644 --- a/src/problems/tsp/heuristics/local_search.cpp +++ b/src/problems/tsp/heuristics/local_search.cpp @@ -10,6 +10,7 @@ All rights reserved (see LICENSE). #include #include #include +#include #include #include @@ -467,9 +468,9 @@ UserCost LocalSearch::two_opt_step() { // Performing exchange. Index current = best_edge_2_start; _edges[best_edge_1_start] = current; - for (auto next = to_reverse.rbegin(); next != to_reverse.rend(); ++next) { - _edges[current] = *next; - current = *next; + for (const auto& next : std::ranges::reverse_view(to_reverse)) { + _edges[current] = next; + current = next; } _edges[current] = best_edge_2_end; } @@ -603,9 +604,9 @@ UserCost LocalSearch::asym_two_opt_step() { // Performing exchange. Index current = best_edge_2_start; _edges[best_edge_1_start] = current; - for (auto next = to_reverse.rbegin(); next != to_reverse.rend(); ++next) { - _edges[current] = *next; - current = *next; + for (const auto& next : std::ranges::reverse_view(to_reverse)) { + _edges[current] = next; + current = next; } _edges[current] = best_edge_2_end; } From 5c43145ba116427bb0b9df0df71f0c65b2cc748b Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 09:58:29 +0100 Subject: [PATCH 18/27] Update tidy rules. --- .clang-tidy | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 8f7285d2..b25e2ece 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -7,13 +7,15 @@ Checks: > -bugprone-too-small-loop-variable, -clang-analyzer-*, cppcoreguidelines-*, - - cppcoreguidelines-init-variables, - - cppcoreguidelines-narrowing-conversions, - - cppcoreguidelines-non-private-member-variables-in-classes, - - cppcoreguidelines-owning-memory, - - cppcoreguidelines-pro-bounds-pointer-arithmetic, - - cppcoreguidelines-pro-type-member-init, - - cppcoreguidelines-special-member-functions, + -cppcoreguidelines-avoid-const-or-ref-data-members, + -cppcoreguidelines-avoid-do-while, + -cppcoreguidelines-init-variables, + -cppcoreguidelines-narrowing-conversions, + -cppcoreguidelines-non-private-member-variables-in-classes, + -cppcoreguidelines-owning-memory, + -cppcoreguidelines-pro-bounds-pointer-arithmetic, + -cppcoreguidelines-pro-type-member-init, + -cppcoreguidelines-special-member-functions, google-*, -google-default-arguments, -google-explicit-constructor, @@ -23,13 +25,15 @@ Checks: > llvm-*, -llvm-header-guard, misc-*, + -misc-include-cleaner, -misc-non-private-member-variables-in-classes, modernize-*, -modernize-return-braced-init-list, -modernize-use-trailing-return-type, -modernize-use-nodiscard, performance-*, - - performance-move-const-arg, + -performance-avoid-endl, + -performance-move-const-arg, readability-*, -readability-function-size, -readability-identifier-length, From 0ca4451b4573ab2d8cff1a7215c2f180a17fe10b Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 10:13:10 +0100 Subject: [PATCH 19/27] Const all the things! --- src/algorithms/heuristics/heuristics.cpp | 17 ++--- src/algorithms/kruskal.cpp | 8 +-- .../local_search/insertion_search.h | 4 +- src/algorithms/local_search/local_search.cpp | 41 +++++------ src/algorithms/munkres.cpp | 4 +- src/algorithms/validation/check.cpp | 2 +- src/main.cpp | 16 ++--- src/problems/cvrp/cvrp.cpp | 2 +- .../cvrp/operators/cross_exchange.cpp | 8 +-- .../cvrp/operators/intra_cross_exchange.cpp | 8 +-- .../cvrp/operators/intra_exchange.cpp | 12 ++-- .../cvrp/operators/intra_mixed_exchange.cpp | 6 +- src/problems/cvrp/operators/intra_or_opt.cpp | 4 +- src/problems/cvrp/operators/intra_two_opt.cpp | 12 ++-- .../cvrp/operators/mixed_exchange.cpp | 6 +- src/problems/cvrp/operators/or_opt.cpp | 4 +- .../cvrp/operators/reverse_two_opt.cpp | 22 +++--- src/problems/cvrp/operators/tsp_fix.cpp | 4 +- src/problems/cvrp/operators/two_opt.cpp | 12 ++-- src/problems/tsp/heuristics/christofides.cpp | 15 ++-- src/problems/tsp/heuristics/local_search.cpp | 70 +++++++++--------- src/problems/tsp/tsp.cpp | 8 +-- src/problems/vrp.h | 4 +- src/routing/http_wrapper.cpp | 8 +-- src/routing/valhalla_wrapper.cpp | 3 +- src/routing/wrapper.h | 4 +- src/structures/generic/undirected_graph.cpp | 4 +- src/structures/typedefs.h | 2 +- src/structures/vroom/input/input.cpp | 21 +++--- src/structures/vroom/solution_state.cpp | 10 +-- src/structures/vroom/tw_route.cpp | 7 +- src/structures/vroom/vehicle.cpp | 4 +- src/utils/helpers.cpp | 17 ++--- src/utils/helpers.h | 10 +-- src/utils/input_parser.cpp | 72 +++++++++---------- 35 files changed, 229 insertions(+), 222 deletions(-) diff --git a/src/algorithms/heuristics/heuristics.cpp b/src/algorithms/heuristics/heuristics.cpp index 91b3572a..0e601f7c 100644 --- a/src/algorithms/heuristics/heuristics.cpp +++ b/src/algorithms/heuristics/heuristics.cpp @@ -44,7 +44,7 @@ inline void seed_route(const Input& input, continue; } - bool is_pickup = (current_job.type == JOB_TYPE::PICKUP); + const bool is_pickup = (current_job.type == JOB_TYPE::PICKUP); if (route.size() + (is_pickup ? 2 : 1) > vehicle.max_tasks) { continue; @@ -57,9 +57,9 @@ inline void seed_route(const Input& input, higher_amount < current_job.delivery); } if (init == INIT::EARLIEST_DEADLINE) { - Duration current_deadline = is_pickup - ? input.jobs[job_rank + 1].tws.back().end - : current_job.tws.back().end; + const Duration current_deadline = + is_pickup ? input.jobs[job_rank + 1].tws.back().end + : current_job.tws.back().end; try_validity = (current_deadline < earliest_deadline); } if (init == INIT::FURTHEST) { @@ -177,8 +177,9 @@ inline Eval fill_route(const Input& input, const auto current_eval = utils::addition_cost(input, job_rank, vehicle, route.route, r); - double current_cost = static_cast(current_eval.cost) - - lambda * static_cast(regrets[job_rank]); + const double current_cost = + static_cast(current_eval.cost) - + lambda * static_cast(regrets[job_rank]); if (current_cost < best_cost && (vehicle.ok_for_range_bounds(route_eval + current_eval)) && @@ -266,7 +267,7 @@ inline Eval fill_route(const Input& input, current_eval = p_add + d_adds[delivery_r]; } - double current_cost = + const double current_cost = current_eval.cost - lambda * static_cast(regrets[job_rank]); @@ -274,7 +275,7 @@ inline Eval fill_route(const Input& input, modified_with_pd.push_back(job_rank + 1); // Update best cost depending on validity. - bool valid = + const bool valid = (vehicle.ok_for_range_bounds(route_eval + current_eval)) && route .is_valid_addition_for_capacity_inclusion(input, diff --git a/src/algorithms/kruskal.cpp b/src/algorithms/kruskal.cpp index 604814e4..6a4103c8 100644 --- a/src/algorithms/kruskal.cpp +++ b/src/algorithms/kruskal.cpp @@ -36,11 +36,11 @@ UndirectedGraph minimum_spanning_tree(const UndirectedGraph& graph) { std::iota(representative.begin(), representative.end(), 0); for (const auto& edge : edges) { - Index first_vertex = edge.get_first_vertex(); - Index second_vertex = edge.get_second_vertex(); + const Index first_vertex = edge.get_first_vertex(); + const Index second_vertex = edge.get_second_vertex(); - Index first_rep = representative[first_vertex]; - Index second_rep = representative[second_vertex]; + const Index first_rep = representative[first_vertex]; + const Index second_rep = representative[second_vertex]; if (first_rep != second_rep) { // Adding current edge won't create a cycle as vertices are in // separate connected components. diff --git a/src/algorithms/local_search/insertion_search.h b/src/algorithms/local_search/insertion_search.h index 128477b8..9dba5541 100644 --- a/src/algorithms/local_search/insertion_search.h +++ b/src/algorithms/local_search/insertion_search.h @@ -43,7 +43,7 @@ compute_best_insertion_single(const Input& input, for (Index rank = sol_state.insertion_ranks_begin[v][j]; rank < sol_state.insertion_ranks_end[v][j]; ++rank) { - Eval current_eval = + const Eval current_eval = utils::addition_cost(input, j, v_target, route.route, rank); if (current_eval.cost < result.eval.cost && v_target.ok_for_range_bounds(sol_state.route_evals[v] + @@ -131,7 +131,7 @@ RouteInsertion compute_best_insertion_pd(const Input& input, for (Index pickup_r = sol_state.insertion_ranks_begin[v][j]; pickup_r < sol_state.insertion_ranks_end[v][j]; ++pickup_r) { - Eval p_add = + const Eval p_add = utils::addition_cost(input, j, v_target, route.route, pickup_r); if (result.eval < p_add) { // Even without delivery insertion more expensive than current best. diff --git a/src/algorithms/local_search/local_search.cpp b/src/algorithms/local_search/local_search.cpp index ae2fb1d8..561ece74 100644 --- a/src/algorithms/local_search/local_search.cpp +++ b/src/algorithms/local_search/local_search.cpp @@ -423,7 +423,7 @@ void LocalSearch& m) { if (matching_y != matching_yx.end()) { // Chosen y is actually matched in M, update S and T_set and // proceed to step 2. - Index matched_x = matching_y->second; + const Index matched_x = matching_y->second; if (const auto [iter, insert_ok] = S.insert(matched_x); insert_ok) { S_list.push_back(matched_x); @@ -155,7 +155,7 @@ minimum_weight_perfect_matching(const Matrix& m) { Index current_x = alternating_tree.at(current_y); while (current_x != unmatched_x) { - Index next_y = matching_xy.at(current_x); + const Index next_y = matching_xy.at(current_x); // Remove alternating edge from current matching. matching_xy.erase(matching_xy.find(current_x)); diff --git a/src/algorithms/validation/check.cpp b/src/algorithms/validation/check.cpp index 77a3458f..451a0649 100644 --- a/src/algorithms/validation/check.cpp +++ b/src/algorithms/validation/check.cpp @@ -72,7 +72,7 @@ check_and_set_ETA(const Input& input, routes[route_rank] = choose_ETA(input, v, input.vehicles[v].steps); } } catch (...) { - std::scoped_lock lock(ep_m); + const std::scoped_lock lock(ep_m); ep = std::current_exception(); } }; diff --git a/src/main.cpp b/src/main.cpp index ed50033b..194dcff2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -206,7 +206,7 @@ int main(int argc, char** argv) { // Get input problem from first input file, then positional arg, // then stdin. if (!cl_args.input_file.empty()) { - std::ifstream ifs(cl_args.input_file); + const std::ifstream ifs(cl_args.input_file); if (!ifs) { const auto exc = vroom::InputException("Can't read file: " + cl_args.input_file); @@ -233,13 +233,13 @@ int main(int argc, char** argv) { cl_args.apply_TSPFix); vroom::io::parse(problem_instance, cl_args.input, cl_args.geometry); - vroom::Solution sol = (cl_args.check) - ? problem_instance.check(cl_args.nb_threads) - : problem_instance.solve(cl_args.nb_searches, - cl_args.depth, - cl_args.nb_threads, - cl_args.timeout, - cl_args.h_params); + const vroom::Solution sol = (cl_args.check) + ? problem_instance.check(cl_args.nb_threads) + : problem_instance.solve(cl_args.nb_searches, + cl_args.depth, + cl_args.nb_threads, + cl_args.timeout, + cl_args.h_params); // Write solution. vroom::io::write_to_json(sol, diff --git a/src/problems/cvrp/cvrp.cpp b/src/problems/cvrp/cvrp.cpp index 840f416f..f4f30ea1 100644 --- a/src/problems/cvrp/cvrp.cpp +++ b/src/problems/cvrp/cvrp.cpp @@ -156,7 +156,7 @@ Solution CVRP::solve(const unsigned nb_searches, std::vector job_ranks(_input.jobs.size()); std::iota(job_ranks.begin(), job_ranks.end(), 0); - TSP p(_input, std::move(job_ranks), 0); + const TSP p(_input, std::move(job_ranks), 0); RawRoute r(_input, 0, 0); r.set_route(_input, p.raw_solve(nb_threads, timeout)); diff --git a/src/problems/cvrp/operators/cross_exchange.cpp b/src/problems/cvrp/operators/cross_exchange.cpp index 37db05cf..48588d91 100644 --- a/src/problems/cvrp/operators/cross_exchange.cpp +++ b/src/problems/cvrp/operators/cross_exchange.cpp @@ -75,10 +75,10 @@ Eval CrossExchange::gain_upper_bound() { // (for adjacent edges) is stored in // _sol_state.edge_evals_around_edge. reverse_* checks whether we // should change the target edge order. - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index s_after_index = _input.jobs[s_route[s_rank + 1]].index(); - Index t_index = _input.jobs[t_route[t_rank]].index(); - Index t_after_index = _input.jobs[t_route[t_rank + 1]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index s_after_index = _input.jobs[s_route[s_rank + 1]].index(); + const Index t_index = _input.jobs[t_route[t_rank]].index(); + const Index t_after_index = _input.jobs[t_route[t_rank + 1]].index(); // Determine costs added with target edge. Eval previous_cost; diff --git a/src/problems/cvrp/operators/intra_cross_exchange.cpp b/src/problems/cvrp/operators/intra_cross_exchange.cpp index 9c14d1ad..1128d6c4 100644 --- a/src/problems/cvrp/operators/intra_cross_exchange.cpp +++ b/src/problems/cvrp/operators/intra_cross_exchange.cpp @@ -73,10 +73,10 @@ Eval IntraCrossExchange::gain_upper_bound() { // target edge. Part of that cost (for adjacent edges) is stored in // _sol_state.edge_evals_around_edge. reverse_* checks whether we // should change the target edge order. - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index s_after_index = _input.jobs[s_route[s_rank + 1]].index(); - Index t_index = _input.jobs[s_route[t_rank]].index(); - Index t_after_index = _input.jobs[s_route[t_rank + 1]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index s_after_index = _input.jobs[s_route[s_rank + 1]].index(); + const Index t_index = _input.jobs[s_route[t_rank]].index(); + const Index t_after_index = _input.jobs[s_route[t_rank + 1]].index(); // Determine costs added with target edge. Eval previous_cost; diff --git a/src/problems/cvrp/operators/intra_exchange.cpp b/src/problems/cvrp/operators/intra_exchange.cpp index c2734428..58570820 100644 --- a/src/problems/cvrp/operators/intra_exchange.cpp +++ b/src/problems/cvrp/operators/intra_exchange.cpp @@ -49,8 +49,8 @@ void IntraExchange::compute_gain() { // Consider the cost of replacing job at rank s_rank with target // job. Part of that cost (for adjacent edges) is stored in // _sol_state.edge_evals_around_node. - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index t_index = _input.jobs[t_route[t_rank]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index t_index = _input.jobs[t_route[t_rank]].index(); // Determine costs added with target job. Eval new_previous_cost; @@ -68,8 +68,8 @@ void IntraExchange::compute_gain() { auto n_index = _input.jobs[s_route[s_rank + 1]].index(); Eval new_next_cost = v.eval(t_index, n_index); - Eval s_gain = _sol_state.edge_evals_around_node[s_vehicle][s_rank] - - new_previous_cost - new_next_cost; + const Eval s_gain = _sol_state.edge_evals_around_node[s_vehicle][s_rank] - + new_previous_cost - new_next_cost; // Consider the cost of replacing job at rank t_rank with source // job. Part of that cost (for adjacent edges) is stored in @@ -91,8 +91,8 @@ void IntraExchange::compute_gain() { new_next_cost = v.eval(s_index, n_index); } - Eval t_gain = _sol_state.edge_evals_around_node[s_vehicle][t_rank] - - new_previous_cost - new_next_cost; + const Eval t_gain = _sol_state.edge_evals_around_node[s_vehicle][t_rank] - + new_previous_cost - new_next_cost; stored_gain = s_gain + t_gain; gain_computed = true; diff --git a/src/problems/cvrp/operators/intra_mixed_exchange.cpp b/src/problems/cvrp/operators/intra_mixed_exchange.cpp index ce417a37..4ed25251 100644 --- a/src/problems/cvrp/operators/intra_mixed_exchange.cpp +++ b/src/problems/cvrp/operators/intra_mixed_exchange.cpp @@ -81,9 +81,9 @@ Eval IntraMixedExchange::gain_upper_bound() { // edge. Part of that cost (for adjacent edges) is stored in // _sol_state.edge_evals_around_node. reverse_* checks whether we // should change the target edge order. - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index t_index = _input.jobs[s_route[t_rank]].index(); - Index t_after_index = _input.jobs[s_route[t_rank + 1]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index t_index = _input.jobs[s_route[t_rank]].index(); + const Index t_after_index = _input.jobs[s_route[t_rank + 1]].index(); // Determine costs added with target edge. Eval previous_cost; diff --git a/src/problems/cvrp/operators/intra_or_opt.cpp b/src/problems/cvrp/operators/intra_or_opt.cpp index 8b7a6bd4..6ce1ff47 100644 --- a/src/problems/cvrp/operators/intra_or_opt.cpp +++ b/src/problems/cvrp/operators/intra_or_opt.cpp @@ -80,8 +80,8 @@ Eval IntraOrOpt::gain_upper_bound() { new_rank += 2; } - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index after_s_index = _input.jobs[s_route[s_rank + 1]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index after_s_index = _input.jobs[s_route[s_rank + 1]].index(); Eval previous_cost; Eval next_cost; diff --git a/src/problems/cvrp/operators/intra_two_opt.cpp b/src/problems/cvrp/operators/intra_two_opt.cpp index 5bda441c..95a2e312 100644 --- a/src/problems/cvrp/operators/intra_two_opt.cpp +++ b/src/problems/cvrp/operators/intra_two_opt.cpp @@ -39,8 +39,8 @@ IntraTwoOpt::IntraTwoOpt(const Input& input, void IntraTwoOpt::compute_gain() { const auto& s_v = _input.vehicles[s_vehicle]; - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index t_index = _input.jobs[t_route[t_rank]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index t_index = _input.jobs[t_route[t_rank]].index(); // Cost of reversing vehicle route between s_rank and t_rank // included. @@ -51,12 +51,12 @@ void IntraTwoOpt::compute_gain() { // Cost of going to t_rank first instead of s_rank. if (s_rank > 0) { - Index previous_index = _input.jobs[s_route[s_rank - 1]].index(); + const Index previous_index = _input.jobs[s_route[s_rank - 1]].index(); stored_gain += s_v.eval(previous_index, s_index); stored_gain -= s_v.eval(previous_index, t_index); } else { if (s_v.has_start()) { - Index start_index = s_v.start.value().index(); + const Index start_index = s_v.start.value().index(); stored_gain += s_v.eval(start_index, s_index); stored_gain -= s_v.eval(start_index, t_index); } @@ -64,12 +64,12 @@ void IntraTwoOpt::compute_gain() { // Cost of going from s_rank after instead of t_rank. if (t_rank < s_route.size() - 1) { - Index next_index = _input.jobs[s_route[t_rank + 1]].index(); + const Index next_index = _input.jobs[s_route[t_rank + 1]].index(); stored_gain += s_v.eval(t_index, next_index); stored_gain -= s_v.eval(s_index, next_index); } else { if (s_v.has_end()) { - Index end_index = s_v.end.value().index(); + const Index end_index = s_v.end.value().index(); stored_gain += s_v.eval(t_index, end_index); stored_gain -= s_v.eval(s_index, end_index); } diff --git a/src/problems/cvrp/operators/mixed_exchange.cpp b/src/problems/cvrp/operators/mixed_exchange.cpp index 7b9de232..98b7d438 100644 --- a/src/problems/cvrp/operators/mixed_exchange.cpp +++ b/src/problems/cvrp/operators/mixed_exchange.cpp @@ -63,9 +63,9 @@ Eval MixedExchange::gain_upper_bound() { // s_rank with target edge. Part of that cost (for adjacent edges) // is stored in _sol_state.edge_evals_around_node. reverse_t_edge // checks whether we should change the target edge order. - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index t_index = _input.jobs[t_route[t_rank]].index(); - Index t_after_index = _input.jobs[t_route[t_rank + 1]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index t_index = _input.jobs[t_route[t_rank]].index(); + const Index t_after_index = _input.jobs[t_route[t_rank + 1]].index(); // Determine costs added with target edge. Eval previous_cost; diff --git a/src/problems/cvrp/operators/or_opt.cpp b/src/problems/cvrp/operators/or_opt.cpp index 2e3be761..91f349ff 100644 --- a/src/problems/cvrp/operators/or_opt.cpp +++ b/src/problems/cvrp/operators/or_opt.cpp @@ -50,8 +50,8 @@ Eval OrOpt::gain_upper_bound() { // For target vehicle, we consider the cost of adding source edge at // rank t_rank. reverse_* checks whether we should change the // source edge order. - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index after_s_index = _input.jobs[s_route[s_rank + 1]].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index after_s_index = _input.jobs[s_route[s_rank + 1]].index(); Eval previous_cost; Eval next_cost; diff --git a/src/problems/cvrp/operators/reverse_two_opt.cpp b/src/problems/cvrp/operators/reverse_two_opt.cpp index 03028719..7d17faed 100644 --- a/src/problems/cvrp/operators/reverse_two_opt.cpp +++ b/src/problems/cvrp/operators/reverse_two_opt.cpp @@ -44,13 +44,13 @@ void ReverseTwoOpt::compute_gain() { const auto& s_v = _input.vehicles[s_vehicle]; const auto& t_v = _input.vehicles[t_vehicle]; - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index t_index = _input.jobs[t_route[t_rank]].index(); - Index last_s = _input.jobs[s_route.back()].index(); - Index first_t = _input.jobs[t_route.front()].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index t_index = _input.jobs[t_route[t_rank]].index(); + const Index last_s = _input.jobs[s_route.back()].index(); + const Index first_t = _input.jobs[t_route.front()].index(); - bool last_in_source = (s_rank == s_route.size() - 1); - bool last_in_target = (t_rank == t_route.size() - 1); + const bool last_in_source = (s_rank == s_route.size() - 1); + const bool last_in_target = (t_rank == t_route.size() - 1); // Cost of swapping route for vehicle s_vehicle after step // s_rank with route for vehicle t_vehicle up to step @@ -68,17 +68,17 @@ void ReverseTwoOpt::compute_gain() { if (!last_in_target) { // Spare next edge in target route. - Index next_index = _input.jobs[t_route[t_rank + 1]].index(); + const Index next_index = _input.jobs[t_route[t_rank + 1]].index(); t_gain += t_v.eval(t_index, next_index); } if (!last_in_source) { // Spare next edge in source route. - Index next_index = _input.jobs[s_route[s_rank + 1]].index(); + const Index next_index = _input.jobs[s_route[s_rank + 1]].index(); s_gain += s_v.eval(s_index, next_index); // Part of source route is moved to target route. - Index next_s_index = _input.jobs[s_route[s_rank + 1]].index(); + const Index next_s_index = _input.jobs[s_route[s_rank + 1]].index(); // Cost or reverting source route portion. First remove forward // cost for end of source route as seen from source vehicle @@ -99,7 +99,7 @@ void ReverseTwoOpt::compute_gain() { } } else { // Add new target -> source edge. - Index next_t_index = _input.jobs[t_route[t_rank + 1]].index(); + const Index next_t_index = _input.jobs[t_route[t_rank + 1]].index(); t_gain -= t_v.eval(next_s_index, next_t_index); } } @@ -121,7 +121,7 @@ void ReverseTwoOpt::compute_gain() { // No job from source route actually swapped to target route. if (!last_in_target) { // Going straight from start to next job in target route. - Index next_index = _input.jobs[t_route[t_rank + 1]].index(); + const Index next_index = _input.jobs[t_route[t_rank + 1]].index(); t_gain -= t_v.eval(start_t, next_index); } else { // Emptying the whole target route here, so also gaining cost diff --git a/src/problems/cvrp/operators/tsp_fix.cpp b/src/problems/cvrp/operators/tsp_fix.cpp index ca3495e0..2c7c0e33 100644 --- a/src/problems/cvrp/operators/tsp_fix.cpp +++ b/src/problems/cvrp/operators/tsp_fix.cpp @@ -32,7 +32,7 @@ TSPFix::TSPFix(const Input& input, void TSPFix::compute_gain() { std::vector jobs = s_route; - TSP tsp(_input, std::move(jobs), s_vehicle); + const TSP tsp(_input, std::move(jobs), s_vehicle); tsp_route = tsp.raw_solve(1, Timeout()); s_gain = _sol_state.route_evals[s_vehicle] - @@ -45,7 +45,7 @@ bool TSPFix::is_valid() { bool valid = is_valid_for_source_range_bounds(); if (valid) { - RawRoute route(_input, s_vehicle, _input.zero_amount().size()); + const RawRoute route(_input, s_vehicle, _input.zero_amount().size()); valid = route.is_valid_addition_for_capacity_inclusion(_input, _s_delivery, diff --git a/src/problems/cvrp/operators/two_opt.cpp b/src/problems/cvrp/operators/two_opt.cpp index 570a05be..5135aba6 100644 --- a/src/problems/cvrp/operators/two_opt.cpp +++ b/src/problems/cvrp/operators/two_opt.cpp @@ -44,10 +44,10 @@ void TwoOpt::compute_gain() { const auto& s_v = _input.vehicles[s_vehicle]; const auto& t_v = _input.vehicles[t_vehicle]; - Index s_index = _input.jobs[s_route[s_rank]].index(); - Index t_index = _input.jobs[t_route[t_rank]].index(); - Index last_s = _input.jobs[s_route.back()].index(); - Index last_t = _input.jobs[t_route.back()].index(); + const Index s_index = _input.jobs[s_route[s_rank]].index(); + const Index t_index = _input.jobs[t_route[t_rank]].index(); + const Index last_s = _input.jobs[s_route.back()].index(); + const Index last_t = _input.jobs[t_route.back()].index(); Index new_last_s = last_t; Index new_last_t = last_s; @@ -59,7 +59,7 @@ void TwoOpt::compute_gain() { // Basic costs in case we really swap jobs and not only the end of // the route. Otherwise remember that last job does not change. if (s_rank < s_route.size() - 1) { - Index next_index = _input.jobs[s_route[s_rank + 1]].index(); + const Index next_index = _input.jobs[s_route[s_rank + 1]].index(); s_gain += s_v.eval(s_index, next_index); t_gain -= t_v.eval(t_index, next_index); @@ -74,7 +74,7 @@ void TwoOpt::compute_gain() { new_last_t = t_index; } if (t_rank < t_route.size() - 1) { - Index next_index = _input.jobs[t_route[t_rank + 1]].index(); + const Index next_index = _input.jobs[t_route[t_rank + 1]].index(); t_gain += t_v.eval(t_index, next_index); s_gain -= s_v.eval(s_index, next_index); diff --git a/src/problems/tsp/heuristics/christofides.cpp b/src/problems/tsp/heuristics/christofides.cpp index 9e6639e8..ef5ff683 100644 --- a/src/problems/tsp/heuristics/christofides.cpp +++ b/src/problems/tsp/heuristics/christofides.cpp @@ -29,7 +29,7 @@ std::list christofides(const Matrix& sym_matrix) { // Getting minimum spanning tree of associated graph under the form // of an adjacency list. - std::unordered_map> adjacency_list = + const std::unordered_map> adjacency_list = mst_graph.get_adjacency_list(); // Getting odd degree vertices from the minimum spanning tree. @@ -41,7 +41,8 @@ std::list christofides(const Matrix& sym_matrix) { } // Getting corresponding matrix for the generated sub-graph. - Matrix sub_matrix = sym_matrix.get_sub_matrix(mst_odd_vertices); + const Matrix sub_matrix = + sym_matrix.get_sub_matrix(mst_odd_vertices); // Computing minimum weight perfect matching. std::unordered_map mwpm = @@ -62,7 +63,7 @@ std::list christofides(const Matrix& sym_matrix) { } if (!wrong_vertices.empty()) { - std::unordered_map remaining_greedy_mwpm = + const std::unordered_map remaining_greedy_mwpm = utils::greedy_symmetric_approx_mwpm( sub_matrix.get_sub_matrix(wrong_vertices)); @@ -85,8 +86,8 @@ std::list christofides(const Matrix& sym_matrix) { // need to remember the one already added. std::unordered_set already_added; for (const auto& [source, target] : mwpm_final) { - Index first_index = mst_odd_vertices[source]; - Index second_index = mst_odd_vertices[target]; + const Index first_index = mst_odd_vertices[source]; + const Index second_index = mst_odd_vertices[target]; if (!already_added.contains(first_index)) { eulerian_graph_edges.emplace_back(first_index, second_index, @@ -96,7 +97,7 @@ std::list christofides(const Matrix& sym_matrix) { } // Building Eulerian graph from the edges. - utils::UndirectedGraph eulerian_graph( + const utils::UndirectedGraph eulerian_graph( std::move(eulerian_graph_edges)); assert(eulerian_graph.size() >= 2); @@ -128,7 +129,7 @@ std::list christofides(const Matrix& sym_matrix) { if (!complete_tour) { // Add new tour to initial eulerian path and check again. std::list new_tour; - Index initial_vertex = *new_tour_start; + const Index initial_vertex = *new_tour_start; Index current_vertex = initial_vertex; Index next_vertex; // Start building new tour. diff --git a/src/problems/tsp/heuristics/local_search.cpp b/src/problems/tsp/heuristics/local_search.cpp index 15eb7fc2..546ecdd0 100644 --- a/src/problems/tsp/heuristics/local_search.cpp +++ b/src/problems/tsp/heuristics/local_search.cpp @@ -30,11 +30,11 @@ LocalSearch::LocalSearch(const Matrix& matrix, _rank_limits(_nb_threads) { // Build _edges vector representation. auto location = tour.cbegin(); - Index first_index = *location; + const Index first_index = *location; Index last_index = first_index; ++location; while (location != tour.cend()) { - Index current_index = *location; + const Index current_index = *location; _edges[last_index] = current_index; last_index = current_index; ++location; @@ -44,7 +44,7 @@ LocalSearch::LocalSearch(const Matrix& matrix, // Build a vector of bounds that easily split the [0, _edges.size()] // look-up range 'evenly' between threads for relocate and or-opt // operator. - std::size_t range_width = _edges.size() / _nb_threads; + const std::size_t range_width = _edges.size() / _nb_threads; std::iota(_rank_limits.begin(), _rank_limits.end(), 0); std::transform(_rank_limits.begin(), _rank_limits.end(), @@ -53,7 +53,7 @@ LocalSearch::LocalSearch(const Matrix& matrix, // Shifting the limits to dispatch remaining ranks among more // threads for a more even load balance. This way the load // difference between ranges should be at most 1. - std::size_t remainder = _edges.size() % _nb_threads; + const std::size_t remainder = _edges.size() % _nb_threads; std::size_t shift = 0; for (std::size_t i = 1; i < _rank_limits.size(); ++i) { if (shift < remainder) { @@ -85,8 +85,8 @@ LocalSearch::LocalSearch(const Matrix& matrix, number_of_lookups.end(), std::back_inserter(cumulated_lookups)); - unsigned total_lookups = _edges.size() * (_edges.size() - 3) / 2; - unsigned thread_lookup_share = total_lookups / _nb_threads; + const unsigned total_lookups = _edges.size() * (_edges.size() - 3) / 2; + const unsigned thread_lookup_share = total_lookups / _nb_threads; Index rank = 0; for (std::size_t i = 1; i < _nb_threads; ++i) { @@ -115,14 +115,14 @@ UserCost LocalSearch::relocate_step() { Index& best_edge_1_start, Index& best_edge_2_start) { for (Index edge_1_start = start; edge_1_start < end; ++edge_1_start) { - Index edge_1_end = _edges[edge_1_start]; + const Index edge_1_end = _edges[edge_1_start]; // Going through the tour while checking for insertion of // edge_1_end between two other nodes (edge_2_*). // // Namely edge_1_start --> edge_1_end --> next is replaced by // edge_1_start --> next while edge_2_start --> edge_2_end is // replaced by edge_2_start --> edge_1_end --> edge_2_end. - Index next = _edges[edge_1_end]; + const Index next = _edges[edge_1_end]; // Precomputing weights not depending on edge_2_*. auto first_potential_add = _matrix[edge_1_start][next]; @@ -143,7 +143,7 @@ UserCost LocalSearch::relocate_step() { Index edge_2_start = next; while (edge_2_start != edge_1_start) { - Index edge_2_end = _edges[edge_2_start]; + const Index edge_2_end = _edges[edge_2_start]; const auto before_cost = edge_1_weight + edge_1_end_next_weight + _matrix[edge_2_start][edge_2_end]; @@ -189,13 +189,13 @@ UserCost LocalSearch::relocate_step() { auto best_rank = std::distance(best_gains.begin(), std::ranges::max_element(best_gains)); auto best_gain = best_gains[best_rank]; - Index best_edge_1_start = best_edge_1_starts[best_rank]; - Index best_edge_2_start = best_edge_2_starts[best_rank]; + const Index best_edge_1_start = best_edge_1_starts[best_rank]; + const Index best_edge_2_start = best_edge_2_starts[best_rank]; if (best_gain > 0) { // Performing best possible exchange. - Index best_edge_1_end = _edges[best_edge_1_start]; - Index best_edge_2_end = _edges[best_edge_2_start]; + const Index best_edge_1_end = _edges[best_edge_1_start]; + const Index best_edge_2_end = _edges[best_edge_2_start]; _edges[best_edge_1_start] = _edges[best_edge_1_end]; _edges[best_edge_1_end] = best_edge_2_end; @@ -262,7 +262,7 @@ UserCost LocalSearch::avoid_loop_step() { if (!_avoid_start_relocate.first || candidate != _avoid_start_relocate.second) { while ((current != previous_candidate) && !candidate_relocatable) { - Index next = _edges[current]; + const Index next = _edges[current]; if ((_matrix[current][candidate] + _matrix[candidate][next] <= _matrix[current][next]) && (_matrix[current][candidate] > 0) && @@ -389,7 +389,7 @@ UserCost LocalSearch::two_opt_step() { Index& best_edge_1_start, Index& best_edge_2_start) { for (Index edge_1_start = start; edge_1_start < end; ++edge_1_start) { - Index edge_1_end = _edges[edge_1_start]; + const Index edge_1_end = _edges[edge_1_start]; for (Index edge_2_start = edge_1_start + 1; edge_2_start < _edges.size(); ++edge_2_start) { // Trying to improve two "crossing edges". @@ -403,7 +403,7 @@ UserCost LocalSearch::two_opt_step() { // is the same as with (e_1, e_2), so assuming edge_1_start < // edge_2_start avoids testing pairs in both orders. - Index edge_2_end = _edges[edge_2_start]; + const Index edge_2_end = _edges[edge_2_start]; if ((edge_2_start == edge_1_end) || (edge_2_end == edge_1_start)) { // Operator doesn't make sense. continue; @@ -453,12 +453,12 @@ UserCost LocalSearch::two_opt_step() { auto best_rank = std::distance(best_gains.begin(), std::ranges::max_element(best_gains)); auto best_gain = best_gains[best_rank]; - Index best_edge_1_start = best_edge_1_starts[best_rank]; - Index best_edge_2_start = best_edge_2_starts[best_rank]; + const Index best_edge_1_start = best_edge_1_starts[best_rank]; + const Index best_edge_2_start = best_edge_2_starts[best_rank]; if (best_gain > 0) { - Index best_edge_1_end = _edges[best_edge_1_start]; - Index best_edge_2_end = _edges[best_edge_2_start]; + const Index best_edge_1_end = _edges[best_edge_1_start]; + const Index best_edge_2_end = _edges[best_edge_2_start]; // Storing part of the tour that needs to be reversed. std::vector to_reverse; for (Index current = best_edge_1_end; current != best_edge_2_start; @@ -486,7 +486,7 @@ UserCost LocalSearch::asym_two_opt_step() { // The initial node for the first edge is arbitrary but it is handy // to keep in mind the previous one for stopping conditions. - Index previous_init = _edges.front(); + const Index previous_init = _edges.front(); Index init = _edges[previous_init]; // Lambda function to search for the best move in a range of @@ -500,7 +500,7 @@ UserCost LocalSearch::asym_two_opt_step() { do { // Going through the edges in the order of the current tour. - Index edge_1_end = _edges[edge_1_start]; + const Index edge_1_end = _edges[edge_1_start]; Index edge_2_start = _edges[edge_1_end]; Index edge_2_end = _edges[edge_2_start]; // Trying to improve two "crossing edges". @@ -552,7 +552,7 @@ UserCost LocalSearch::asym_two_opt_step() { std::vector best_gains(_nb_threads, 0); std::vector best_edge_1_starts(_nb_threads); std::vector best_edge_2_starts(_nb_threads); - std::size_t thread_range = _edges.size() / _nb_threads; + const std::size_t thread_range = _edges.size() / _nb_threads; // The limits in the range given to each thread are not ranks but // actual nodes used to browse a piece of the current tour. @@ -589,12 +589,12 @@ UserCost LocalSearch::asym_two_opt_step() { auto best_rank = std::distance(best_gains.begin(), std::ranges::max_element(best_gains)); auto best_gain = best_gains[best_rank]; - Index best_edge_1_start = best_edge_1_starts[best_rank]; - Index best_edge_2_start = best_edge_2_starts[best_rank]; + const Index best_edge_1_start = best_edge_1_starts[best_rank]; + const Index best_edge_2_start = best_edge_2_starts[best_rank]; if (best_gain > 0) { - Index best_edge_1_end = _edges[best_edge_1_start]; - Index best_edge_2_end = _edges[best_edge_2_start]; + const Index best_edge_1_end = _edges[best_edge_1_start]; + const Index best_edge_2_end = _edges[best_edge_2_start]; // Storing part of the tour that needs to be reversed. std::vector to_reverse; for (Index current = best_edge_1_end; current != best_edge_2_start; @@ -664,9 +664,9 @@ UserCost LocalSearch::or_opt_step() { Index& best_edge_1_start, Index& best_edge_2_start) { for (Index edge_1_start = start; edge_1_start < end; ++edge_1_start) { - Index edge_1_end = _edges[edge_1_start]; - Index next = _edges[edge_1_end]; - Index next_2 = _edges[next]; + const Index edge_1_end = _edges[edge_1_start]; + const Index next = _edges[edge_1_end]; + const Index next_2 = _edges[next]; Index edge_2_start = next_2; // Going through the tour while checking the move of edge after // edge_1_end in place of another edge (edge_2_*). @@ -682,7 +682,7 @@ UserCost LocalSearch::or_opt_step() { auto next_next_2_weight = _matrix[next][next_2]; while (edge_2_start != edge_1_start) { - Index edge_2_end = _edges[edge_2_start]; + const Index edge_2_end = _edges[edge_2_start]; const auto before_cost = edge_1_weight + next_next_2_weight + _matrix[edge_2_start][edge_2_end]; if (const auto after_cost = first_potential_add + @@ -727,12 +727,12 @@ UserCost LocalSearch::or_opt_step() { auto best_rank = std::distance(best_gains.begin(), std::ranges::max_element(best_gains)); auto best_gain = best_gains[best_rank]; - Index best_edge_1_start = best_edge_1_starts[best_rank]; - Index best_edge_2_start = best_edge_2_starts[best_rank]; + const Index best_edge_1_start = best_edge_1_starts[best_rank]; + const Index best_edge_2_start = best_edge_2_starts[best_rank]; if (best_gain > 0) { - Index best_edge_1_end = _edges[best_edge_1_start]; - Index next = _edges[best_edge_1_end]; + const Index best_edge_1_end = _edges[best_edge_1_start]; + const Index next = _edges[best_edge_1_end]; // Performing exchange. _edges[best_edge_1_start] = _edges[next]; diff --git a/src/problems/tsp/tsp.cpp b/src/problems/tsp/tsp.cpp index 52272270..5ace2166 100644 --- a/src/problems/tsp/tsp.cpp +++ b/src/problems/tsp/tsp.cpp @@ -147,7 +147,7 @@ TSP::TSP(const Input& input, std::vector&& job_ranks, Index vehicle_rank) _symmetrized_matrix[i][i] = _matrix[i][i]; for (Index j = i + 1; j < _matrix.size(); ++j) { _is_symmetric = _is_symmetric && (_matrix[i][j] == _matrix[j][i]); - UserCost val = sym_f(_matrix[i][j], _matrix[j][i]); + const UserCost val = sym_f(_matrix[i][j], _matrix[j][i]); _symmetrized_matrix[i][j] = val; _symmetrized_matrix[j][i] = val; } @@ -169,7 +169,7 @@ std::vector TSP::raw_solve(unsigned nb_threads, timeout.has_value() ? utils::now() + timeout.value() : Deadline(); // Applying heuristic. - std::list christo_sol = tsp::christofides(_symmetrized_matrix); + const std::list christo_sol = tsp::christofides(_symmetrized_matrix); Deadline sym_deadline = deadline; if (deadline.has_value() && !_is_symmetric) { @@ -233,8 +233,8 @@ std::vector TSP::raw_solve(unsigned nb_threads, // Back to the asymmetric problem, picking the best way. std::list reverse_current_sol(current_sol); reverse_current_sol.reverse(); - UserCost direct_cost = this->cost(current_sol); - UserCost reverse_cost = this->cost(reverse_current_sol); + const UserCost direct_cost = this->cost(current_sol); + const UserCost reverse_cost = this->cost(reverse_current_sol); // Local search on asymmetric problem. tsp::LocalSearch diff --git a/src/problems/vrp.h b/src/problems/vrp.h index 8f51d962..ede36b9e 100644 --- a/src/problems/vrp.h +++ b/src/problems/vrp.h @@ -90,7 +90,7 @@ template struct SolvingContext { bool heuristic_solution_already_found(unsigned rank) { assert(rank < sol_indicators.size()); - std::scoped_lock lock(heuristic_indicators_m); + const std::scoped_lock lock(heuristic_indicators_m); const auto [dummy, insertion_ok] = heuristic_indicators.insert(sol_indicators[rank]); @@ -283,7 +283,7 @@ class VRP { context); } } catch (...) { - std::scoped_lock lock(ep_m); + const std::scoped_lock lock(ep_m); ep = std::current_exception(); } }; diff --git a/src/routing/http_wrapper.cpp b/src/routing/http_wrapper.cpp index 37044a98..6feb2e59 100644 --- a/src/routing/http_wrapper.cpp +++ b/src/routing/http_wrapper.cpp @@ -40,7 +40,7 @@ void set_response(auto& s, std::string& response) { char buf[512]; // NOLINT std::error_code error; for (;;) { - std::size_t len = s.read_some(asio::buffer(buf), error); + const std::size_t len = s.read_some(asio::buffer(buf), error); response.append(buf, len); // NOLINT if (error == asio::error::eof) { // Connection closed cleanly. @@ -74,7 +74,7 @@ std::string HttpWrapper::send_then_receive(const std::string& query) const { tcp::resolver r(io_service); - tcp::resolver::query q(_server.host, _server.port); + const tcp::resolver::query q(_server.host, _server.port); tcp::socket s(io_service); asio::connect(s, r.resolve(q)); @@ -101,7 +101,7 @@ std::string HttpWrapper::ssl_send_then_receive(const std::string& query) const { tcp::resolver r(io_service); - tcp::resolver::query q(_server.host, _server.port); + const tcp::resolver::query q(_server.host, _server.port); asio::connect(ssock.lowest_layer(), r.resolve(q)); ssock.handshake(asio::ssl::stream_base::handshake_type::client); @@ -199,7 +199,7 @@ void HttpWrapper::update_sparse_matrix(const std::vector& route_locs, assert(legs.Size() == route_locs.size() - 1); for (rapidjson::SizeType i = 0; i < legs.Size(); ++i) { - std::scoped_lock lock(matrix_m); + const std::scoped_lock lock(matrix_m); m.durations[route_locs[i].index()][route_locs[i + 1].index()] = get_leg_duration(legs[i]); m.distances[route_locs[i].index()][route_locs[i + 1].index()] = diff --git a/src/routing/valhalla_wrapper.cpp b/src/routing/valhalla_wrapper.cpp index 08fc286e..544ed223 100644 --- a/src/routing/valhalla_wrapper.cpp +++ b/src/routing/valhalla_wrapper.cpp @@ -102,7 +102,8 @@ void ValhallaWrapper::check_response(const rapidjson::Document& json_result, // keys can be expected so we're playing guesses. This happens // e.g. when requested matrix/route size goes over the server // limit. - std::string service_str = (service == _route_service) ? "route" : "matrix"; + const std::string service_str = + (service == _route_service) ? "route" : "matrix"; std::string error = "Valhalla " + service_str + " error ("; if (json_result.HasMember("error") && json_result["error"].IsString()) { diff --git a/src/routing/wrapper.h b/src/routing/wrapper.h index 5523e29f..d47f9398 100644 --- a/src/routing/wrapper.h +++ b/src/routing/wrapper.h @@ -35,7 +35,7 @@ class Wrapper { const std::vector& vehicles, const std::vector& jobs, std::vector& vehicles_geometry) const { - std::size_t m_size = locs.size(); + const std::size_t m_size = locs.size(); Matrices m(m_size); std::exception_ptr ep = nullptr; @@ -83,7 +83,7 @@ class Wrapper { vehicles_geometry[v_rank]); } } catch (...) { - std::scoped_lock lock(ep_m); + const std::scoped_lock lock(ep_m); ep = std::current_exception(); } }; diff --git a/src/structures/generic/undirected_graph.cpp b/src/structures/generic/undirected_graph.cpp index 2ec76cee..dbcdb850 100644 --- a/src/structures/generic/undirected_graph.cpp +++ b/src/structures/generic/undirected_graph.cpp @@ -47,8 +47,8 @@ template UndirectedGraph::UndirectedGraph(std::vector>&& edges) : _edges(std::move(edges)) { for (auto const& edge : _edges) { - Index first = edge.get_first_vertex(); - Index second = edge.get_second_vertex(); + const Index first = edge.get_first_vertex(); + const Index second = edge.get_second_vertex(); _adjacency_list[first].push_back(second); _adjacency_list[second].push_back(first); diff --git a/src/structures/typedefs.h b/src/structures/typedefs.h index 9ab22f9f..95bb4fcd 100644 --- a/src/structures/typedefs.h +++ b/src/structures/typedefs.h @@ -203,7 +203,7 @@ struct StringHash { using is_transparent = void; // enables heterogenous lookup std::size_t operator()(std::string_view sv) const { - std::hash hasher; + const std::hash hasher; return hasher(sv); } }; diff --git a/src/structures/vroom/input/input.cpp b/src/structures/vroom/input/input.cpp index 1152580c..5b6273f2 100644 --- a/src/structures/vroom/input/input.cpp +++ b/src/structures/vroom/input/input.cpp @@ -121,7 +121,7 @@ void Input::check_job(Job& job) { check_amount_size(job.pickup); // Ensure that location index are either always or never provided. - bool has_location_index = job.location.user_index(); + const bool has_location_index = job.location.user_index(); if (_no_addition_yet) { _no_addition_yet = false; _has_custom_location_index = has_location_index; @@ -479,7 +479,8 @@ UserCost Input::check_cost_bound(const Matrix& matrix) const { max_cost_per_column[j.index()]); } - UserCost jobs_bound = std::max(jobs_departure_bound, jobs_arrival_bound); + const UserCost jobs_bound = + std::max(jobs_departure_bound, jobs_arrival_bound); UserCost start_bound = 0; UserCost end_bound = 0; @@ -496,7 +497,7 @@ UserCost Input::check_cost_bound(const Matrix& matrix) const { } } - UserCost bound = utils::add_without_overflow(start_bound, jobs_bound); + const UserCost bound = utils::add_without_overflow(start_bound, jobs_bound); return utils::add_without_overflow(bound, end_bound); } @@ -529,7 +530,7 @@ void Input::set_extra_compatibility() { // an empty route for vehicle based on the timing constraints (when // they apply). for (std::size_t v = 0; v < vehicles.size(); ++v) { - TWRoute empty_route(*this, v, _zero.size()); + const TWRoute empty_route(*this, v, _zero.size()); for (Index j = 0; j < jobs.size(); ++j) { if (_vehicle_to_job_compatibility[v][j]) { bool is_compatible = @@ -538,7 +539,7 @@ void Input::set_extra_compatibility() { jobs[j].delivery, 0); - bool is_shipment_pickup = (jobs[j].type == JOB_TYPE::PICKUP); + const bool is_shipment_pickup = (jobs[j].type == JOB_TYPE::PICKUP); if (is_compatible && _has_TW) { if (jobs[j].type == JOB_TYPE::SINGLE) { @@ -750,8 +751,8 @@ void Input::set_jobs_vehicles_evals() { Eval(_cost_upper_bound))); for (std::size_t j = 0; j < jobs.size(); ++j) { - Index j_index = jobs[j].index(); - bool is_pickup = (jobs[j].type == JOB_TYPE::PICKUP); + const Index j_index = jobs[j].index(); + const bool is_pickup = (jobs[j].type == JOB_TYPE::PICKUP); Index last_job_index = j_index; if (is_pickup) { @@ -1069,7 +1070,7 @@ void Input::set_matrices(unsigned nb_thread, bool sparse_filling) { // Check for potential overflow in solution cost. const UserCost current_bound = check_cost_bound(c_m->second); - std::scoped_lock lock(cost_bound_m); + const std::scoped_lock lock(cost_bound_m); _cost_upper_bound = std::max(_cost_upper_bound, utils::scale_from_user_cost(current_bound)); @@ -1081,7 +1082,7 @@ void Input::set_matrices(unsigned nb_thread, bool sparse_filling) { assert(search != _max_cost_per_hour.end()); const auto max_cost_per_hour_for_profile = search->second; - std::scoped_lock lock(cost_bound_m); + const std::scoped_lock lock(cost_bound_m); _cost_upper_bound = std::max(_cost_upper_bound, max_cost_per_hour_for_profile * @@ -1089,7 +1090,7 @@ void Input::set_matrices(unsigned nb_thread, bool sparse_filling) { } } } catch (...) { - std::scoped_lock lock(ep_m); + const std::scoped_lock lock(ep_m); ep = std::current_exception(); } }; diff --git a/src/structures/vroom/solution_state.cpp b/src/structures/vroom/solution_state.cpp index 7edcb1bd..c5a8dc7e 100644 --- a/src/structures/vroom/solution_state.cpp +++ b/src/structures/vroom/solution_state.cpp @@ -267,7 +267,7 @@ void SolutionState::set_node_gains(const std::vector& route, Index v) { } void SolutionState::set_edge_gains(const std::vector& route, Index v) { - std::size_t nb_edges = (route.size() < 2) ? 0 : route.size() - 1; + const std::size_t nb_edges = (route.size() < 2) ? 0 : route.size() - 1; edge_gains[v] = std::vector(nb_edges); edge_evals_around_edge[v] = std::vector(nb_edges); @@ -405,9 +405,9 @@ void SolutionState::set_pd_gains(const std::vector& route, Index v) { if (_input.jobs[route[pickup_rank]].type != JOB_TYPE::PICKUP) { continue; } - Index pickup_index = _input.jobs[route[pickup_rank]].index(); - unsigned delivery_rank = matching_delivery_rank[v][pickup_rank]; - Index delivery_index = _input.jobs[route[delivery_rank]].index(); + const Index pickup_index = _input.jobs[route[pickup_rank]].index(); + const Index delivery_rank = matching_delivery_rank[v][pickup_rank]; + const Index delivery_index = _input.jobs[route[delivery_rank]].index(); if (pickup_rank + 1 == delivery_rank) { // Pickup and delivery in a row. @@ -609,7 +609,7 @@ void SolutionState::update_cheapest_job_rank_in_routes( cheapest_job_rank_in_routes_to[v1][v2].assign(route_1.size(), 0); for (std::size_t r1 = 0; r1 < route_1.size(); ++r1) { - Index index_r1 = _input.jobs[route_1[r1]].index(); + const Index index_r1 = _input.jobs[route_1[r1]].index(); auto min_from = std::numeric_limits::max(); auto min_to = std::numeric_limits::max(); diff --git a/src/structures/vroom/tw_route.cpp b/src/structures/vroom/tw_route.cpp index 6cd15a27..e0a653ee 100644 --- a/src/structures/vroom/tw_route.cpp +++ b/src/structures/vroom/tw_route.cpp @@ -213,7 +213,7 @@ void TWRoute::fwd_update_earliest_from(const Input& input, Index rank) { if (handle_last_breaks) { // Update earliest dates and margins for potential breaks right // before route end. - Index i = route.size(); + const Index i = route.size(); Duration remaining_travel_time = (v.has_end()) ? v.duration(input.jobs[route[i - 1]].index(), v.end.value().index()) @@ -329,7 +329,7 @@ void TWRoute::bwd_update_latest_from(const Input& input, Index rank) { if (handle_first_breaks) { // Update latest dates and margins for breaks right before the // first job. - Index next_i = 0; + const Index next_i = 0; assert(breaks_at_rank[next_i] <= breaks_counts[next_i]); Index break_rank = breaks_counts[next_i]; @@ -781,7 +781,8 @@ bool TWRoute::is_valid_addition_for_tw(const Input& input, const auto previous_init_load = (route.empty()) ? input.zero_amount() : load_at_step(first_rank); assert(delivery_in_range(first_rank, last_rank) <= previous_init_load); - Amount delta_delivery = delivery - delivery_in_range(first_rank, last_rank); + const Amount delta_delivery = + delivery - delivery_in_range(first_rank, last_rank); if (current_break != 0 && !(delta_delivery <= diff --git a/src/structures/vroom/vehicle.cpp b/src/structures/vroom/vehicle.cpp index bc996993..6c475aba 100644 --- a/src/structures/vroom/vehicle.cpp +++ b/src/structures/vroom/vehicle.cpp @@ -137,9 +137,9 @@ bool Vehicle::cost_based_on_metrics() const { } Duration Vehicle::available_duration() const { - Duration available = tw.end - tw.start; + const Duration available = tw.end - tw.start; - Duration breaks_duration = + const Duration breaks_duration = std::accumulate(breaks.begin(), breaks.end(), 0, diff --git a/src/utils/helpers.cpp b/src/utils/helpers.cpp index 76c6efb5..81018bf8 100644 --- a/src/utils/helpers.cpp +++ b/src/utils/helpers.cpp @@ -469,13 +469,14 @@ Route format_route(const Input& input, } } - bool same_location = (r > 1 && input.jobs[tw_r.route[r - 2]].index() == - previous_job.index()) || - (r == 1 && v.has_start() && - v.start.value().index() == previous_job.index()); + const bool same_location = + (r > 1 && + input.jobs[tw_r.route[r - 2]].index() == previous_job.index()) || + (r == 1 && v.has_start() && + v.start.value().index() == previous_job.index()); const auto current_setup = same_location ? 0 : previous_job.setup; - Duration diff = + const Duration diff = current_setup + previous_job.service + remaining_travel_time; assert(diff <= step_start); @@ -621,7 +622,7 @@ Route format_route(const Input& input, // The whole remaining travel time is spent before this // break, not filling the whole margin. - Duration wt = margin - travel_time; + const Duration wt = margin - travel_time; forward_wt += wt; current_break.arrival = @@ -710,7 +711,7 @@ Route format_route(const Input& input, assert(j_tw != current_job.tws.end()); if (step_start < j_tw->start) { - Duration wt = j_tw->start - step_start; + const Duration wt = j_tw->start - step_start; forward_wt += wt; // Recompute user-reported waiting time rather than using @@ -778,7 +779,7 @@ Route format_route(const Input& input, // The whole remaining travel time is spent before this // break, not filling the whole margin. - Duration wt = margin - travel_time; + const Duration wt = margin - travel_time; forward_wt += wt; current_break.arrival = diff --git a/src/utils/helpers.h b/src/utils/helpers.h index e0917078..6fd2a105 100644 --- a/src/utils/helpers.h +++ b/src/utils/helpers.h @@ -77,7 +77,7 @@ inline Eval addition_cost(const Input& input, Index rank) { assert(rank <= route.size()); - Index job_index = input.jobs[job_rank].index(); + const Index job_index = input.jobs[job_rank].index(); Eval previous_eval; Eval next_eval; Eval old_edge_eval; @@ -138,8 +138,8 @@ inline Eval addition_cost(const Input& input, if (delivery_rank == pickup_rank + 1) { // Delivery is inserted just after pickup. - Index p_index = input.jobs[job_rank].index(); - Index d_index = input.jobs[job_rank + 1].index(); + const Index p_index = input.jobs[job_rank].index(); + const Index d_index = input.jobs[job_rank + 1].index(); eval += v.eval(p_index, d_index); Eval after_delivery; @@ -153,7 +153,7 @@ inline Eval addition_cost(const Input& input, } } else { // There is a job after insertion. - Index next_index = input.jobs[route[pickup_rank]].index(); + const Index next_index = input.jobs[route[pickup_rank]].index(); after_delivery = v.eval(d_index, next_index); remove_after_pickup = v.eval(p_index, next_index); } @@ -178,7 +178,7 @@ inline Eval in_place_delta_cost(const Input& input, const std::vector& route, Index rank) { assert(!route.empty()); - Index new_index = input.jobs[job_rank].index(); + const Index new_index = input.jobs[job_rank].index(); Eval new_previous_eval; Eval new_next_eval; diff --git a/src/utils/input_parser.cpp b/src/utils/input_parser.cpp index 0b01e678..9209809f 100644 --- a/src/utils/input_parser.cpp +++ b/src/utils/input_parser.cpp @@ -365,8 +365,8 @@ inline Vehicle get_vehicle(const rapidjson::Value& json_vehicle, // Check what info are available for vehicle start, then build // optional start location. - bool has_start_coords = json_vehicle.HasMember("start"); - bool has_start_index = json_vehicle.HasMember("start_index"); + const bool has_start_coords = json_vehicle.HasMember("start"); + const bool has_start_index = json_vehicle.HasMember("start_index"); if (has_start_index && !json_vehicle["start_index"].IsUint()) { throw InputException( std::format("Invalid start_index for vehicle {}.", v_id)); @@ -375,7 +375,7 @@ inline Vehicle get_vehicle(const rapidjson::Value& json_vehicle, std::optional start; if (has_start_index) { // Custom provided matrices and index. - Index start_index = json_vehicle["start_index"].GetUint(); + const Index start_index = json_vehicle["start_index"].GetUint(); if (has_start_coords) { start = Location({start_index, parse_coordinates(json_vehicle, "start")}); } else { @@ -389,8 +389,8 @@ inline Vehicle get_vehicle(const rapidjson::Value& json_vehicle, // Check what info are available for vehicle end, then build // optional end location. - bool has_end_coords = json_vehicle.HasMember("end"); - bool has_end_index = json_vehicle.HasMember("end_index"); + const bool has_end_coords = json_vehicle.HasMember("end"); + const bool has_end_index = json_vehicle.HasMember("end_index"); if (has_end_index && !json_vehicle["end_index"].IsUint()) { throw InputException( std::format("Invalid end_index for vehicle {}.", v_id)); @@ -399,7 +399,7 @@ inline Vehicle get_vehicle(const rapidjson::Value& json_vehicle, std::optional end; if (has_end_index) { // Custom provided matrices and index. - Index end_index = json_vehicle["end_index"].GetUint(); + const Index end_index = json_vehicle["end_index"].GetUint(); if (has_end_coords) { end = Location({end_index, parse_coordinates(json_vehicle, "end")}); } else { @@ -436,8 +436,8 @@ inline Vehicle get_vehicle(const rapidjson::Value& json_vehicle, inline Location get_task_location(const rapidjson::Value& v, const std::string& type) { // Check what info are available to build task location. - bool has_location_coords = v.HasMember("location"); - bool has_location_index = v.HasMember("location_index"); + const bool has_location_coords = v.HasMember("location"); + const bool has_location_index = v.HasMember("location_index"); if (has_location_index && !v["location_index"].IsUint()) { throw InputException(std::format("Invalid location_index for {} {}.", type, @@ -446,7 +446,7 @@ inline Location get_task_location(const rapidjson::Value& v, if (has_location_index) { // Custom provided matrices and index. - Index location_index = v["location_index"].GetUint(); + const Index location_index = v["location_index"].GetUint(); if (has_location_coords) { return Location({location_index, parse_coordinates(v, "location")}); } @@ -462,9 +462,9 @@ inline Job get_job(const rapidjson::Value& json_job, unsigned amount_size) { // Only for retro-compatibility: when no pickup and delivery keys // are defined and (deprecated) amount key is present, it should be // interpreted as a delivery. - bool need_amount_compat = json_job.HasMember("amount") && - !json_job.HasMember("delivery") && - !json_job.HasMember("pickup"); + const bool need_amount_compat = json_job.HasMember("amount") && + !json_job.HasMember("delivery") && + !json_job.HasMember("pickup"); return Job(json_job["id"].GetUint64(), get_task_location(json_job, "job"), @@ -484,14 +484,14 @@ template inline Matrix get_matrix(rapidjson::Value& m) { throw InputException("Invalid matrix."); } // Load custom matrix while checking if it is square. - rapidjson::SizeType matrix_size = m.Size(); + const rapidjson::SizeType matrix_size = m.Size(); Matrix matrix(matrix_size); for (rapidjson::SizeType i = 0; i < matrix_size; ++i) { if (!m[i].IsArray() || m[i].Size() != matrix_size) { throw InputException("Unexpected matrix line length."); } - rapidjson::Document::Array mi = m[i].GetArray(); + const rapidjson::Document::Array mi = m[i].GetArray(); for (rapidjson::SizeType j = 0; j < matrix_size; ++j) { if (!mi[j].IsUint()) { throw InputException("Invalid matrix entry."); @@ -509,7 +509,7 @@ void parse(Input& input, const std::string& input_str, bool geometry) { // Parsing input string to populate the input object. if (json_input.Parse(input_str.c_str()).HasParseError()) { - std::string error_msg = + const std::string error_msg = std::format("{} (offset: {})", rapidjson::GetParseError_En(json_input.GetParseError()), json_input.GetErrorOffset()); @@ -532,7 +532,7 @@ void parse(Input& input, const std::string& input_str, bool geometry) { const auto& first_vehicle = json_input["vehicles"][0]; check_id(first_vehicle, "vehicle"); - bool first_vehicle_has_capacity = + const bool first_vehicle_has_capacity = (first_vehicle.HasMember("capacity") && first_vehicle["capacity"].IsArray() && !first_vehicle["capacity"].Empty()); const unsigned amount_size = @@ -575,31 +575,31 @@ void parse(Input& input, const std::string& input_str, bool geometry) { auto& json_pickup = json_shipment["pickup"]; check_id(json_pickup, "pickup"); - Job pickup(json_pickup["id"].GetUint64(), - JOB_TYPE::PICKUP, - get_task_location(json_pickup, "pickup"), - get_duration(json_pickup, "setup"), - get_duration(json_pickup, "service"), - amount, - skills, - priority, - get_time_windows(json_pickup), - get_string(json_pickup, "description")); + const Job pickup(json_pickup["id"].GetUint64(), + JOB_TYPE::PICKUP, + get_task_location(json_pickup, "pickup"), + get_duration(json_pickup, "setup"), + get_duration(json_pickup, "service"), + amount, + skills, + priority, + get_time_windows(json_pickup), + get_string(json_pickup, "description")); // Defining delivery job. auto& json_delivery = json_shipment["delivery"]; check_id(json_delivery, "delivery"); - Job delivery(json_delivery["id"].GetUint64(), - JOB_TYPE::DELIVERY, - get_task_location(json_delivery, "delivery"), - get_duration(json_delivery, "setup"), - get_duration(json_delivery, "service"), - amount, - skills, - priority, - get_time_windows(json_delivery), - get_string(json_delivery, "description")); + const Job delivery(json_delivery["id"].GetUint64(), + JOB_TYPE::DELIVERY, + get_task_location(json_delivery, "delivery"), + get_duration(json_delivery, "setup"), + get_duration(json_delivery, "service"), + amount, + skills, + priority, + get_time_windows(json_delivery), + get_string(json_delivery, "description")); input.add_shipment(pickup, delivery); } From aca2d5ab15e1e6ed1df8d76483d58d1fa565fcad Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 10:22:11 +0100 Subject: [PATCH 20/27] Explicit lambda capture. --- src/algorithms/validation/check.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/algorithms/validation/check.cpp b/src/algorithms/validation/check.cpp index 451a0649..5e52c94e 100644 --- a/src/algorithms/validation/check.cpp +++ b/src/algorithms/validation/check.cpp @@ -62,7 +62,8 @@ check_and_set_ETA(const Input& input, std::exception_ptr ep = nullptr; std::mutex ep_m; - auto run_check = [&](const std::vector& vehicle_ranks) { + auto run_check = [&v_rank_to_actual_route_rank, &routes, &input, &ep, &ep_m]( + const std::vector& vehicle_ranks) { try { for (auto v : vehicle_ranks) { auto search = v_rank_to_actual_route_rank.find(v); From 9870e04387558bf2820374bdb4529f510a9a0be6 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 11:25:44 +0100 Subject: [PATCH 21/27] Avoid superfluous const param in function declarations. --- src/problems/cvrp/cvrp.h | 6 ++-- src/problems/vrp.h | 6 ++-- src/problems/vrptw/vrptw.h | 6 ++-- src/structures/vroom/input/input.h | 10 +++---- src/structures/vroom/raw_route.h | 34 +++++++++++----------- src/structures/vroom/solution/step.h | 2 +- src/structures/vroom/solution/violations.h | 4 +-- src/structures/vroom/tw_route.h | 30 +++++++++---------- src/utils/helpers.h | 10 +++---- 9 files changed, 52 insertions(+), 56 deletions(-) diff --git a/src/problems/cvrp/cvrp.h b/src/problems/cvrp/cvrp.h index 7f4633d0..58eb31ba 100644 --- a/src/problems/cvrp/cvrp.h +++ b/src/problems/cvrp/cvrp.h @@ -25,9 +25,9 @@ class CVRP : public VRP { explicit CVRP(const Input& input); Solution - solve(const unsigned nb_searches, - const unsigned depth, - const unsigned nb_threads, + solve(unsigned nb_searches, + unsigned depth, + unsigned nb_threads, const Timeout& timeout, const std::vector& h_param) const override; }; diff --git a/src/problems/vrp.h b/src/problems/vrp.h index ede36b9e..be6976fa 100644 --- a/src/problems/vrp.h +++ b/src/problems/vrp.h @@ -329,9 +329,9 @@ class VRP { virtual ~VRP(); virtual Solution - solve(const unsigned nb_searches, - const unsigned depth, - const unsigned nb_threads, + solve(unsigned nb_searches, + unsigned depth, + unsigned nb_threads, const Timeout& timeout, const std::vector& h_param) const = 0; }; diff --git a/src/problems/vrptw/vrptw.h b/src/problems/vrptw/vrptw.h index f7f98cdd..fc6557da 100644 --- a/src/problems/vrptw/vrptw.h +++ b/src/problems/vrptw/vrptw.h @@ -23,9 +23,9 @@ class VRPTW : public VRP { explicit VRPTW(const Input& input); Solution - solve(const unsigned nb_searches, - const unsigned depth, - const unsigned nb_threads, + solve(unsigned nb_searches, + unsigned depth, + unsigned nb_threads, const Timeout& timeout, const std::vector& h_param) const override; }; diff --git a/src/structures/vroom/input/input.h b/src/structures/vroom/input/input.h index be9479dd..5014e987 100644 --- a/src/structures/vroom/input/input.h +++ b/src/structures/vroom/input/input.h @@ -193,17 +193,17 @@ class Input { // Returns true iff both vehicles have common job candidates. bool vehicle_ok_with_vehicle(Index v1_index, Index v2_index) const; - Solution solve(const unsigned nb_searches, - const unsigned depth, - const unsigned nb_thread, + Solution solve(unsigned nb_searches, + unsigned depth, + unsigned nb_thread, const Timeout& timeout = Timeout(), const std::vector& h_param = std::vector()); // Overload designed to expose the same interface as the `-x` // command-line flag for out-of-the-box setup of exploration level. - Solution solve(const unsigned exploration_level, - const unsigned nb_thread, + Solution solve(unsigned exploration_level, + unsigned nb_thread, const Timeout& timeout = Timeout(), const std::vector& h_param = std::vector()); diff --git a/src/structures/vroom/raw_route.h b/src/structures/vroom/raw_route.h index 223db435..121dc8a1 100644 --- a/src/structures/vroom/raw_route.h +++ b/src/structures/vroom/raw_route.h @@ -75,11 +75,11 @@ class RawRoute { void update_amounts(const Input& input); - bool has_pending_delivery_after_rank(const Index rank) const; + bool has_pending_delivery_after_rank(Index rank) const; - bool has_delivery_after_rank(const Index rank) const; + bool has_delivery_after_rank(Index rank) const; - bool has_pickup_up_to_rank(const Index rank) const; + bool has_pickup_up_to_rank(Index rank) const; const Amount& fwd_peak(Index rank) const { return _fwd_peaks[rank]; @@ -110,13 +110,13 @@ class RawRoute { bool is_valid_addition_for_capacity(const Input&, const Amount& pickup, const Amount& delivery, - const Index rank) const; + Index rank) const; // Check if current load allows the addition of a pickup, just // considering capacity limitation at rank. bool is_valid_addition_for_load(const Input& input, const Amount& pickup, - const Index rank) const; + Index rank) const; // Check validity for inclusion (with regard to not breaking // capacity before and after inclusion) of some load in the existing @@ -125,8 +125,8 @@ class RawRoute { bool is_valid_addition_for_capacity_margins(const Input& input, const Amount& pickup, const Amount& delivery, - const Index first_rank, - const Index last_rank) const; + Index first_rank, + Index last_rank) const; // Check validity for inclusion (with regard to not breaking // capacity for included jobs) of the range [first_job; last_job) in @@ -135,10 +135,10 @@ class RawRoute { template bool is_valid_addition_for_capacity_inclusion(const Input& input, Amount delivery, - const Iter first_job, - const Iter last_job, - const Index first_rank, - const Index last_rank) const; + Iter first_job, + Iter last_job, + Index first_rank, + Index last_rank) const; const Amount& job_deliveries_sum() const; @@ -193,23 +193,23 @@ class RawRoute { return true; } - void add(const Input& input, const Index job_rank, const Index rank); + void add(const Input& input, Index job_rank, Index rank); bool is_valid_removal(const Input&, const Index, const unsigned) const { return true; }; - void remove(const Input& input, const Index rank, const unsigned count); + void remove(const Input& input, Index rank, unsigned count); // Add the range [first_job; last_job) in the existing route at rank // first_rank and before last_rank *in place of* the current jobs // that may be there. template void replace(const Input& input, - const Iter first_job, - const Iter last_job, - const Index first_rank, - const Index last_rank); + Iter first_job, + Iter last_job, + Index first_rank, + Index last_rank); template void replace(const Input& input, diff --git a/src/structures/vroom/solution/step.h b/src/structures/vroom/solution/step.h index 5460e29c..edb7ccdf 100644 --- a/src/structures/vroom/solution/step.h +++ b/src/structures/vroom/solution/step.h @@ -38,7 +38,7 @@ struct Step { Step(STEP_TYPE type, Location location, Amount load); - Step(const Job& job, const UserDuration setup, Amount load); + Step(const Job& job, UserDuration setup, Amount load); Step(const Break& b, Amount load); diff --git a/src/structures/vroom/solution/violations.h b/src/structures/vroom/solution/violations.h index 4006d5a3..37b2f1bc 100644 --- a/src/structures/vroom/solution/violations.h +++ b/src/structures/vroom/solution/violations.h @@ -25,8 +25,8 @@ struct Violations { // Used for route/summary. Violations( - const UserDuration lead_time, - const UserDuration delay, + UserDuration lead_time, + UserDuration delay, std::unordered_set&& types = std::unordered_set()); Violations& operator+=(const Violations& rhs); diff --git a/src/structures/vroom/tw_route.h b/src/structures/vroom/tw_route.h index 234bc63e..2834e4a3 100644 --- a/src/structures/vroom/tw_route.h +++ b/src/structures/vroom/tw_route.h @@ -51,7 +51,7 @@ struct OrderChoice { const std::vector::const_iterator b_tw; OrderChoice(const Input& input, - const Index job_rank, + Index job_rank, const Break& b, const PreviousInfo& previous); }; @@ -59,11 +59,9 @@ struct OrderChoice { class TWRoute : public RawRoute { private: PreviousInfo previous_info(const Input& input, - const Index job_rank, - const Index rank) const; - NextInfo next_info(const Input& input, - const Index job_rank, - const Index rank) const; + Index job_rank, + Index rank) const; + NextInfo next_info(const Input& input, Index job_rank, Index rank) const; void fwd_update_earliest_from(const Input& input, Index rank); void bwd_update_latest_from(const Input& input, Index rank); @@ -77,8 +75,8 @@ class TWRoute : public RawRoute { // Define global policy wrt job/break respective insertion rule. OrderChoice order_choice(const Input& input, - const Index job_rank, - const Duration job_action_time, + Index job_rank, + Duration job_action_time, const Break& b, const PreviousInfo& previous, const NextInfo& next, @@ -169,10 +167,10 @@ class TWRoute : public RawRoute { template bool is_valid_addition_for_tw(const Input& input, const Amount& delivery, - const Iter first_job, - const Iter last_job, - const Index first_rank, - const Index last_rank, + Iter first_job, + Iter last_job, + Index first_rank, + Index last_rank, bool check_max_load = true) const; void add(const Input& input, const Index job_rank, const Index rank) { @@ -220,10 +218,10 @@ class TWRoute : public RawRoute { template void replace(const Input& input, const Amount& delivery, - const Iter first_job, - const Iter last_job, - const Index first_rank, - const Index last_rank); + Iter first_job, + Iter last_job, + Index first_rank, + Index last_rank); }; } // namespace vroom diff --git a/src/utils/helpers.h b/src/utils/helpers.h index 6fd2a105..4eb10fd5 100644 --- a/src/utils/helpers.h +++ b/src/utils/helpers.h @@ -218,20 +218,18 @@ Priority priority_sum_for_route(const Input& input, Eval route_eval_for_vehicle(const Input& input, Index vehicle_rank, - const std::vector::const_iterator first_job, - const std::vector::const_iterator last_job); + std::vector::const_iterator first_job, + std::vector::const_iterator last_job); Eval route_eval_for_vehicle(const Input& input, Index vehicle_rank, const std::vector& route); void check_tws(const std::vector& tws, - const Id id, + Id id, const std::string& type); -void check_priority(const Priority priority, - const Id id, - const std::string& type); +void check_priority(Priority priority, Id id, const std::string& type); using RawSolution = std::vector; using TWSolution = std::vector; From 7328ff9b727f8b5170f041cca7a461fff89aae9b Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 11:27:43 +0100 Subject: [PATCH 22/27] Missing commas in clang-tidy config. --- .clang-tidy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index b25e2ece..c1373270 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -41,8 +41,8 @@ Checks: > -readability-named-parameter, -readability-convert-member-functions-to-static, -readability-implicit-bool-conversion, - -readability-avoid-const-params-in-decls - -readability-uppercase-literal-suffix + -readability-avoid-const-params-in-decls, + -readability-uppercase-literal-suffix, WarningsAsErrors: '*' HeaderFilterRegex: '.*' From e7ebca3dce9a430f629777dbd5a7764f620167f2 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 11:48:05 +0100 Subject: [PATCH 23/27] Remove unused function. --- src/problems/cvrp/cvrp.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/problems/cvrp/cvrp.h b/src/problems/cvrp/cvrp.h index 58eb31ba..99880e16 100644 --- a/src/problems/cvrp/cvrp.h +++ b/src/problems/cvrp/cvrp.h @@ -16,8 +16,6 @@ namespace vroom { class CVRP : public VRP { private: - bool empty_cluster(const std::vector& cluster, Index v) const; - static const std::vector homogeneous_parameters; static const std::vector heterogeneous_parameters; From 490948113ceddab512d48f139dfd5cf29e2f8a2c Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 11:49:49 +0100 Subject: [PATCH 24/27] Remove some magic numbers. --- src/algorithms/local_search/swap_star_utils.h | 3 ++- src/routing/osrm_routed_wrapper.cpp | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/algorithms/local_search/swap_star_utils.h b/src/algorithms/local_search/swap_star_utils.h index 1de6e792..c5ce0666 100644 --- a/src/algorithms/local_search/swap_star_utils.h +++ b/src/algorithms/local_search/swap_star_utils.h @@ -266,7 +266,8 @@ SwapChoice compute_best_swap_star_choice(const Input& input, s_rank); std::vector swap_choice_options; - swap_choice_options.reserve(16); + constexpr std::size_t MAX_SWAP_CHOICES = 16; + swap_choice_options.reserve(MAX_SWAP_CHOICES); // Options for in-place insertion in source route include // in-place insertion in target route and other relevant diff --git a/src/routing/osrm_routed_wrapper.cpp b/src/routing/osrm_routed_wrapper.cpp index fe502f5c..f7a4bc96 100644 --- a/src/routing/osrm_routed_wrapper.cpp +++ b/src/routing/osrm_routed_wrapper.cpp @@ -33,8 +33,9 @@ OsrmRoutedWrapper::build_query(const std::vector& locations, // Build query part for snapping restriction. std::string radiuses = "radiuses="; - radiuses.reserve(9 + locations.size() * - (DEFAULT_OSRM_SNAPPING_RADIUS.size() + 1)); + radiuses.reserve(radiuses.size() + + locations.size() * + (DEFAULT_OSRM_SNAPPING_RADIUS.size() + 1)); // Adding locations and radiuses values. for (auto const& location : locations) { From 94a0dd3c5d318e008d95b2fb3653d0074745825b Mon Sep 17 00:00:00 2001 From: jcoupey Date: Wed, 11 Dec 2024 18:58:22 +0100 Subject: [PATCH 25/27] Consistent parameter naming. --- src/routing/http_wrapper.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routing/http_wrapper.h b/src/routing/http_wrapper.h index 18deefea..772929d8 100644 --- a/src/routing/http_wrapper.h +++ b/src/routing/http_wrapper.h @@ -58,7 +58,7 @@ class HttpWrapper : public Wrapper { void update_sparse_matrix(const std::vector& route_locs, Matrices& m, std::mutex& matrix_m, - std::string& vehicles_geometry) const override; + std::string& vehicle_geometry) const override; virtual bool duration_value_is_null(const rapidjson::Value& matrix_entry) const { From 16faa6c3a00243dd280b3639f31356203b4911cb Mon Sep 17 00:00:00 2001 From: jcoupey Date: Thu, 12 Dec 2024 09:53:26 +0100 Subject: [PATCH 26/27] Remove some more magic numbers. --- src/structures/vroom/cost_wrapper.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/structures/vroom/cost_wrapper.cpp b/src/structures/vroom/cost_wrapper.cpp index 44f837e9..a19348d6 100644 --- a/src/structures/vroom/cost_wrapper.cpp +++ b/src/structures/vroom/cost_wrapper.cpp @@ -51,8 +51,11 @@ void CostWrapper::set_costs_matrix(const Matrix* matrix, UserCost CostWrapper::user_cost_from_user_metrics(UserDuration d, UserDistance m) const { assert(_cost_based_on_metrics); - return utils::round(static_cast(d * _per_hour) / 3600 + - static_cast(m * _per_km) / 1000); + constexpr auto SECONDS_PER_HOUR = 3600; + constexpr auto M_PER_KM = 1000; + return utils::round(static_cast(d * _per_hour) / + SECONDS_PER_HOUR + + static_cast(m * _per_km) / M_PER_KM); } } // namespace vroom From 485ee2bf2884ea56798116fe65dcaa6f19f32529 Mon Sep 17 00:00:00 2001 From: jcoupey Date: Thu, 12 Dec 2024 09:55:51 +0100 Subject: [PATCH 27/27] Mention tidying round in changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4191aad7..bf55170f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ #### Internals - Iterator type required by `TWRoute::replace` function (#1103) +- Address some of the SonaQube and clang-tidy reports (#1200) #### CI