diff --git a/CHANGELOG.md b/CHANGELOG.md index df26666ba..f5969c409 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - VROOM was used incorrectly in various cases: negative quantities, vehicle duration, activity position [#223](https://github.com/Mapotempo/optimizer-api/pull/223) [#242](https://github.com/Mapotempo/optimizer-api/pull/242) - Capacity violation in periodic heuristic algorithm (`first_solution_strategy='periodic'`) [#227](https://github.com/Mapotempo/optimizer-api/pull/227) +- `total_time`, `total_travel_time` and related values are correctly calculated [#237](https://github.com/Mapotempo/optimizer-api/pull/237) ## [v1.7.1] - 2021-05-20 diff --git a/lib/interpreters/split_clustering.rb b/lib/interpreters/split_clustering.rb index 14c88faa6..7458abc0f 100644 --- a/lib/interpreters/split_clustering.rb +++ b/lib/interpreters/split_clustering.rb @@ -514,6 +514,9 @@ def self.remove_poorly_populated_routes(vrp, result, limit) if load_flag && time_flag emptied_routes = true + %i[total_distance total_time total_travel_time total_value].each{ |key| + result[key] -= route[key] if route[key] + } number_of_services_in_the_route = route[:activities].map{ |a| a[:service_id] && a.slice(:service_id, :detail).compact }.compact.size diff --git a/optimizer_wrapper.rb b/optimizer_wrapper.rb index cce33ec6e..0376300d3 100644 --- a/optimizer_wrapper.rb +++ b/optimizer_wrapper.rb @@ -155,7 +155,10 @@ def self.define_main_process(services_vrps, job = nil, &block) } } - check_result_consistency(expected_activity_count, several_results) if services_vrps.collect{ |sv| sv[:service] } != [:demo] # demo solver returns a fixed solution + # demo solver returns a fixed solution + unless services_vrps.collect{ |sv| sv[:service] }.uniq == [:demo] + check_result_consistency(expected_activity_count, several_results) + end nb_routes = several_results.sum{ |result| result[:routes].count{ |r| r[:activities].any?{ |a| a[:service_id] } } } nb_unassigned = several_results.sum{ |result| result[:unassigned].size } @@ -497,8 +500,38 @@ def self.job_remove(api_key, id) def self.check_result_consistency(expected_value, results) [results].flatten(1).each{ |result| - nb_assigned = result[:routes].sum{ |route| route[:activities].count{ |a| a[:service_id] } } - nb_unassigned = result[:unassigned].count{ |unassigned| unassigned[:service_id] } + if result[:routes].any?{ |route| route[:activities].any?{ |a| a[:waiting_time].to_i < 0 } } + log 'Computed waiting times are invalid', level: :warn + raise RuntimeError, 'Computed waiting times are invalid' if ENV['APP_ENV'] != 'production' + end + + waiting_times = result[:routes].map{ |route| route[:total_waiting_time] }.compact + durations = result[:routes].map{ |route| + route[:activities].map{ |act| act[:departure_time] && (act[:departure_time] - act[:begin_time]) }.compact + } + setup_durations = result[:routes].map{ |route| + route[:activities].map{ |act| + next if act[:type] == 'rest' + + (act[:travel_time].nil? || act[:travel_time]&.positive?) && act[:detail][:setup_duration] || 0 + }.compact + } + total_time = result[:total_time] || 0 + total_travel_time = result[:total_travel_time] || 0 + if total_time != (total_travel_time || 0) + + waiting_times.sum + + (setup_durations.flatten.reduce(&:+) || 0) + + (durations.flatten.reduce(&:+) || 0) + + log_string = "Expected #{total_time} == #{total_travel_time} +"\ + " #{waiting_times.sum} + #{setup_durations.flatten.reduce(&:+)}"\ + " + #{durations.flatten.reduce(&:+)}" + log log_string, level: :warn + raise RuntimeError, 'Computed times are invalid' if ENV['APP_ENV'] != 'production' + end + + nb_assigned = result[:routes].sum{ |route| route[:activities].count{ |a| a[:service_id] || a[:pickup_shipment_id] || a[:delivery_shipment_id] } } + nb_unassigned = result[:unassigned].count{ |unassigned| unassigned[:service_id] || unassigned[:pickup_shipment_id] || unassigned[:delivery_shipment_id] } if expected_value != nb_assigned + nb_unassigned # rubocop:disable Style/Next for error handling log "Expected: #{expected_value} Have: #{nb_assigned + nb_unassigned} activities" @@ -550,6 +583,7 @@ def self.compute_route_total_dimensions(vrp, route, matrix) activity[:current_distance] ||= total[dimension].round if dimension == :distance } end + next if point.nil? previous = point } @@ -581,31 +615,36 @@ def self.compute_result_total_dimensions_and_round_route_stats(result) end def self.compute_route_waiting_times(route) - seen = 1 - previous_end = - if route[:activities].first[:type] == 'depot' - route[:activities].first[:begin_time] + previous_end = route[:activities].first[:begin_time] + loc_index = nil + consumed_travel_time = 0 + consumed_setup_time = 0 + route[:activities].each.with_index{ |act, index| + used_travel_time = 0 + if act[:type] == 'rest' + if loc_index.nil? + next_index = route[:activities][index..-1].index{ |a| a[:type] != 'rest' } + loc_index = index + next_index if next_index + consumed_travel_time = 0 + end + shared_travel_time = loc_index && route[:activities][loc_index][:travel_time] || 0 + potential_setup = shared_travel_time > 0 && route[:activities][loc_index][:detail][:setup_duration] || 0 + left_travel_time = shared_travel_time - consumed_travel_time + used_travel_time = [act[:begin_time] - previous_end, left_travel_time].min + consumed_travel_time += used_travel_time + # As setup is considered as a transit value, it may be performed before a rest + consumed_setup_time += [act[:begin_time] - previous_end - used_travel_time, potential_setup].min else - route[:activities].first[:end_time] + used_travel_time = (act[:travel_time] || 0) - consumed_travel_time - consumed_setup_time + consumed_travel_time = 0 + consumed_setup_time = 0 + loc_index = nil end - route[:activities].first[:waiting_time] = 0 - - first_service_seen = true - while seen < route[:activities].size - considered_setup = - if route[:activities][seen][:type] == 'rest' - 0 - elsif first_service_seen || route[:activities][seen][:travel_time].positive? - route[:activities][seen][:detail][:setup_duration] || 0 - else - 0 - end - first_service_seen = false if %w[service pickup delivery].include?(route[:activities][seen][:type]) - arrival_time = previous_end + (route[:activities][seen][:travel_time] || 0) + considered_setup - route[:activities][seen][:waiting_time] = route[:activities][seen][:begin_time] - arrival_time - previous_end = route[:activities][seen][:end_time] - seen += 1 - end + considered_setup = act[:travel_time]&.positive? && (act[:detail][:setup_duration].to_i - consumed_setup_time) || 0 + arrival_time = previous_end + used_travel_time + considered_setup + consumed_setup_time + act[:waiting_time] = act[:begin_time] - arrival_time + previous_end = act[:end_time] || act[:begin_time] + } end def self.provide_day(vrp, route) diff --git a/test/real_cases_test.rb b/test/real_cases_test.rb index 2024b6b38..6b5fc4a39 100644 --- a/test/real_cases_test.rb +++ b/test/real_cases_test.rb @@ -135,7 +135,7 @@ def test_ortools_one_route_with_rest_and_waiting_time assert_equal 1, result[:routes].size # Check total travel time - assert_operator result[:routes].sum{ |r| r[:total_travel_time] }, :<=, 5289, 'Too long travel time' + assert_operator result[:routes].sum{ |r| r[:total_travel_time] }, :<=, 5394, 'Too long travel time' # Check activities assert_equal check_vrp_services_size + 2 + 1, result[:routes][0][:activities].size # Check elapsed time diff --git a/wrappers/vroom.rb b/wrappers/vroom.rb index 75e1953d7..d345b8fe5 100644 --- a/wrappers/vroom.rb +++ b/wrappers/vroom.rb @@ -105,7 +105,7 @@ def solve(vrp, job = nil, _thread_proc = nil) original_vehicle_id: vehicle.original_id, activities: activities, start_time: activities.first[:begin_time], - end_time: activities.last[:begin_time] + (activities.last[:duration] || 0), + end_time: activities.last[:begin_time] + (activities.last[:detail][:duration] || 0), } } diff --git a/wrappers/wrapper.rb b/wrappers/wrapper.rb index 208d148dc..1a08eedb0 100644 --- a/wrappers/wrapper.rb +++ b/wrappers/wrapper.rb @@ -1249,8 +1249,10 @@ def shift_route_times(route, shift_amount, shift_start_index = 0) activity[:begin_time] += shift_amount activity[:end_time] += shift_amount if activity[:end_time] + activity[:waiting_time] -= [shift_amount, activity[:waiting_time]].min if activity[:waiting_time] activity[:departure_time] += shift_amount if activity[:departure_time] } + route[:total_time] += shift_amount if route[:total_time] route[:end_time] += shift_amount if route[:end_time] end