From f56bbebb79c1906b7a18dfc4598e9ceea203fc6f Mon Sep 17 00:00:00 2001 From: Braktar Date: Fri, 8 Apr 2022 13:36:24 +0200 Subject: [PATCH] Fix activities and relations --- CHANGELOG.md | 1 + README.md | 2 +- docker/Dockerfile | 2 +- models/concerns/validate_data.rb | 36 +++++++++-- models/relation.rb | 9 +++ models/solution/parsers/stop_parser.rb | 5 +- .../lib/interpreters/split_clustering_test.rb | 10 +++ test/test_helper.rb | 64 +++++++++++++++++++ test/wrappers/ortools_test.rb | 59 +++++++++++++++-- wrappers/ortools.rb | 7 +- wrappers/wrapper.rb | 8 ++- 11 files changed, 186 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be4e3c61f..cba4b9d16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Allow to compute geojsons for synchronous resolutions [#356](https://github.com/Mapotempo/optimizer-api/pull/356/files) - Calculate a vehicle_compatibility hash for each service and use it for unfeasible service detection [#318](https://github.com/Mapotempo/optimizer-api/pull/318) - Add an enpoint able to validate the vrp send and return it "filtered" [#349](https://github.com/Mapotempo/optimizer-api/pull/349) +- Activity positions and linking relations tolerates alternatives [#392](https://github.com/Mapotempo/optimizer-api/pull/392) ### Changed diff --git a/README.md b/README.md index a0d6cd348..bbf8240d5 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ bundle install This project requires some solver and interface projects in order to be fully functional! * [Vroom v1.8.0](https://github.com/VROOM-Project/vroom/releases/tag/v1.8.0) -* [Optimizer-ortools v1.12.0](https://github.com/Mapotempo/optimizer-ortools) & [OR-Tools v7.8](https://github.com/google/or-tools/releases/tag/v7.8) (use the version corresponding to your system operator, not source code). +* [Optimizer-ortools v1.14.0](https://github.com/Mapotempo/optimizer-ortools) & [OR-Tools v7.8](https://github.com/google/or-tools/releases/tag/v7.8) (use the version corresponding to your system operator, not source code). Note : when updating OR-Tools you should to recompile optimizer-ortools. diff --git a/docker/Dockerfile b/docker/Dockerfile index 2b608b834..2c362e9a3 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -8,7 +8,7 @@ ARG VROOM_VERSION FROM vroomvrp/vroom-docker:${VROOM_VERSION:-v1.8.0} as vroom # Rake -FROM ${REGISTRY:-registry.mapotempo.com/}mapotempo-${BRANCH:-ce}/optimizer-ortools:${OPTIMIZER_ORTOOLS_VERSION:-v1.13.0} +FROM ${REGISTRY:-registry.mapotempo.com/}mapotempo-${BRANCH:-ce}/optimizer-ortools:${OPTIMIZER_ORTOOLS_VERSION:-v1.14.0} ARG BUNDLE_WITHOUT ENV LANG C.UTF-8 diff --git a/models/concerns/validate_data.rb b/models/concerns/validate_data.rb index 0136bd771..0a803ff78 100644 --- a/models/concerns/validate_data.rb +++ b/models/concerns/validate_data.rb @@ -203,17 +203,25 @@ def check_position_relation_specificities @hash[:services].find{ |service| service[:id] == linked_id } } previous_service = nil + previous_activities = [] services.each{ |service| - if previous_service && forbidden_position_pairs.include?([previous_service[:activity][:position], service[:activity][:position]]) - inconsistent_position_services << [previous_service[:id], service[:id]] - end + activities = service[:activity] ? [service[:activity]] : service[:activities] + + previous_activities.each{ |previous_activity| + activities.each{ |activity| + next unless forbidden_position_pairs.include?([previous_activity[:position], activity[:position]]) + + inconsistent_position_services << [previous_service[:id], service[:id]] + } + } previous_service = service + previous_activities = activities } } return unless inconsistent_position_services.any? - raise OptimizerWrapper::DiscordantProblemError.new("Inconsistent positions in relations: #{inconsistent_position_services}") + raise OptimizerWrapper::DiscordantProblemError.new("Inconsistent positions in relations: #{inconsistent_position_services.uniq}") end def calculate_day_availabilities(vehicles, timewindow_arrays) @@ -405,6 +413,7 @@ def check_relations(periodic_heuristic) check_position_relation_specificities check_relation_consistent_ids check_sticky_relation_consistency + check_relation_compatibility_with_alternatives if periodic_heuristic incompatible_relation_types = @hash[:relations].collect{ |r| r[:type] }.uniq - %i[force_first never_first force_end same_vehicle] @@ -484,6 +493,25 @@ def check_sticky_relation_consistency ) end + def check_relation_compatibility_with_alternatives + not_handled_relations = [] + @hash[:relations].each{ |relation| + next if Models::Relation::ALTERNATIVE_COMPATIBLE_RELATIONS.include?(relation[:type]) + + services = @hash[:services].select{ |service| relation[:linked_service_ids]&.include?(service[:id]) } + not_handled_relations << relation if services.any?{ |service| + (service[:activities]&.size || 0) > 1 + } + } + + return unless not_handled_relations.any? + + raise OptimizerWrapper::UnsupportedProblemError.new( + "The following relations are not compatible with alternative activities: ", + not_handled_relations + ) + end + def check_routes(periodic_heuristic) @hash[:routes]&.each{ |route| route[:mission_ids].each{ |id| diff --git a/models/relation.rb b/models/relation.rb index f372aab82..147f3e357 100644 --- a/models/relation.rb +++ b/models/relation.rb @@ -20,6 +20,15 @@ module Models class Relation < Base ALL_OR_NONE_RELATIONS = %i[shipment meetup].freeze + ALTERNATIVE_COMPATIBLE_RELATIONS = %i[ + order + same_route + sequence + shipment + force_first + never_first + force_end + ].freeze # Relations that link multiple services to be on the same route LINKING_RELATIONS = %i[ diff --git a/models/solution/parsers/stop_parser.rb b/models/solution/parsers/stop_parser.rb index 68d1c47db..091792473 100644 --- a/models/solution/parsers/stop_parser.rb +++ b/models/solution/parsers/stop_parser.rb @@ -20,7 +20,8 @@ module Parsers class ServiceParser def self.parse(service, options) - activity = options[:index] && service.activities[options[:index]] || service.activity + alternative_index = options[:index] || (service.activity ? 0 : (service.activities.size - 1)) + activity = service.activity || service.activities[alternative_index] activity_hash = Models::Activity.field_names.map{ |key| next if key == :point_id @@ -36,7 +37,7 @@ def self.parse(service, options) pickup_shipment_id: service.type == :pickup ? (service.original_id || service.id) : nil, delivery_shipment_id: service.type == :delivery ? (service.original_id || service.id) : nil, type: service.type, - alternative: options[:index], + alternative: options[:index], # nil if unassigned but return by default the last activity loads: build_loads(service, options), activity: dup_activity, info: options[:info] || Models::Solution::Stop::Info.new({}), diff --git a/test/lib/interpreters/split_clustering_test.rb b/test/lib/interpreters/split_clustering_test.rb index 6871dff04..7c2c6d7dd 100644 --- a/test/lib/interpreters/split_clustering_test.rb +++ b/test/lib/interpreters/split_clustering_test.rb @@ -768,6 +768,16 @@ def test_which_relations_are_linking_and_forcing minimum_duration_lapse vehicle_trips ], Models::Relation::FORCING_RELATIONS, 'Forcing relation constant has changed' + + assert_equal %i[ + force_end + force_first + never_first + order + same_route + sequence + shipment + ], Models::Relation::ALTERNATIVE_COMPATIBLE_RELATIONS.sort, 'Forcing relation constant has changed' end def test_collect_data_items_respects_linking_relations diff --git a/test/test_helper.rb b/test/test_helper.rb index 941d97689..2c527e0ed 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -406,6 +406,70 @@ def self.basic } end + def self.basic_alternatives + { + units: [{ id: 'kg' }], + matrices: [{ + id: 'matrix_0', + time: [ + [0, 4, 5, 5], + [6, 0, 1, 5], + [1, 2, 0, 5], + [5, 5, 5, 0] + ] + }], + points: [{ + id: 'point_0', + matrix_index: 0 + }, { + id: 'point_1', + matrix_index: 1 + }, { + id: 'point_2', + matrix_index: 2 + }, { + id: 'point_3', + matrix_index: 3 + }], + vehicles: [{ + id: 'vehicle_0', + matrix_id: 'matrix_0', + start_point_id: 'point_0' + }], + services: [{ + id: 'service_1', + activities: [{ + point_id: 'point_1', + }, { + point_id: 'point_2' + }] + }, { + id: 'service_2', + activities: [{ + point_id: 'point_2' + }, { + point_id: 'point_3' + }] + }, { + id: 'service_3', + activities: [{ + point_id: 'point_3' + }, { + point_id: 'point_1' + }] + }], + configuration: { + resolution: { + duration: 100 + }, + preprocessing: {}, + restitution: { + intermediate_solutions: false, + } + } + } + end + def self.pud { matrices: [{ diff --git a/test/wrappers/ortools_test.rb b/test/wrappers/ortools_test.rb index 2b785a4f2..648c06503 100644 --- a/test/wrappers/ortools_test.rb +++ b/test/wrappers/ortools_test.rb @@ -4372,14 +4372,13 @@ def test_no_solver_with_ortools_single_heuristic def test_insert_with_order vrp = TestHelper.load_vrp(self, fixture_file: 'instance_order') + ordered_service_ids = vrp.relations.first.linked_service_ids solutions = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:ortools] }}, vrp, nil) assert solutions[0] assert_equal 0, solutions[0].unassigned_stops.size, 'All services should be planned.' - order_in_route = vrp[:relations][0][:linked_service_ids].collect{ |service_id| - solutions[0].routes.first.stops.find_index{ |activity| activity.service_id == service_id } - } - assert_equal order_in_route.sort, order_in_route, 'Services with order relation should appear in correct order in route.' + order_in_route = solutions[0].routes.first.stops.select{ |activity| ordered_service_ids.include?(activity.service_id) }.map(&:service_id) + assert_equal vrp.relations.first.linked_service_ids, order_in_route, 'Services with order relation should appear in correct order in route.' end def test_ordre_with_2_vehicles @@ -5175,4 +5174,56 @@ def test_always_one_time_window_provided_for_rests end } end + + def test_activities_positions + problem = VRP.basic_alternatives + problem[:services].first[:activities].each{ |activity| + activity[:position] = :always_last + } + OptimizerWrapper.config[:services][:ortools].solve(TestHelper.create(problem), 'test') + end + + def test_activities_build_unassigned + problem = VRP.basic_alternatives + vrp = Models::Vrp.create(problem) + + OptimizerWrapper.config[:services][:ortools].stub( + :build_unassigned, + lambda { |_, _| + unassigned_services = vrp.services.map{ |service| [service.id, service] }.to_h + unassigned_rests = [] + OptimizerWrapper.config[:services][:ortools].send(:__minitest_stub__build_unassigned, unassigned_services, unassigned_rests) + } + ) do + OptimizerWrapper.config[:services][:ortools].solve(vrp, 'test') + end + end + + def test_activities_order + problem = VRP.basic_alternatives + problem[:services][0][:activities][0][:timewindows] = [{start: 0, end: 20}] + problem[:services][0][:activities][1][:timewindows] = [{start: 100, end: 120}] + problem[:services][1][:activities][0][:timewindows] = [{start: 100, end: 120}] + problem[:services][1][:activities][1][:timewindows] = [{start: 0, end: 20}] + problem[:services][2][:activities][0][:timewindows] = [{start: 0, end: 20}] + problem[:services][2][:activities][1][:timewindows] = [{start: 100, end: 120}] + problem[:relations] = [{ + type: :order, + linked_service_ids: ['service_1', 'service_2', 'service_3'] + }] + + solution = OptimizerWrapper.config[:services][:ortools].solve(TestHelper.create(problem), 'test') + alternative_indices = solution.routes[0].stops.map(&:alternative).compact.uniq + assert_equal 2, alternative_indices.size + assert_equal 1, (solution.routes[0].stops.index{ |stop| stop.id == 'service_1' }) + assert_equal 2, (solution.routes[0].stops.index{ |stop| stop.id == 'service_2' }) + assert_equal 3, (solution.routes[0].stops.index{ |stop| stop.id == 'service_3' }) + + problem[:services][1][:activities][0][:timewindows] = [{start: 0, end: 1}] + problem[:services][1][:activities][1][:timewindows] = [{start: 0, end: 1}] + + solution = OptimizerWrapper.config[:services][:ortools].solve(TestHelper.create(problem), 'test') + unassigned_stops = solution.unassigned_stops + assert_equal 2, unassigned_stops.size + end end diff --git a/wrappers/ortools.rb b/wrappers/ortools.rb index 6c4d86aae..165ae0bdd 100644 --- a/wrappers/ortools.rb +++ b/wrappers/ortools.rb @@ -77,7 +77,7 @@ def solve(vrp, job, thread_proc = nil, &block) duplicated_begins = already_begin.uniq.select{ |linked_id| already_begin.select{ |link| link == linked_id }.size > 1 } already_end = order_relations.collect{ |relation| relation.linked_service_ids[1..-1] }.flatten duplicated_ends = already_end.uniq.select{ |linked_id| already_end.select{ |link| link == linked_id }.size > 1 } - if vrp.routes.empty? && order_relations.size == 1 + if vrp.routes.empty? && order_relations.size == 1 && vrp.services.none?{ |service| service.activities.any? } order_relations.select{ |relation| (relation.linked_service_ids[0..-2] & duplicated_begins).size == 0 && (relation.linked_service_ids[1..-1] & duplicated_ends).size == 0 }.each{ |relation| order_route = { vehicle: (vrp.vehicles.size == 1) ? vrp.vehicles.first : nil, @@ -185,7 +185,7 @@ def solve(vrp, job, thread_proc = nil, &block) services = update_services_positions(services, services_positions, service.id, service.activity.position, service_index) elsif service.activities - service.activities.each_with_index{ |possible_activity, activity_index| + service.activities.each{ |possible_activity| services << OrtoolsVrp::Service.new( time_windows: possible_activity.timewindows.collect{ |tw| OrtoolsVrp::TimeWindow.new(start: tw.start, end: tw.end || 2147483647, maximum_lateness: tw.maximum_lateness) @@ -209,7 +209,7 @@ def solve(vrp, job, thread_proc = nil, &block) matrix_index: possible_activity.point.matrix_index, vehicle_indices: vehicles_indices, setup_duration: possible_activity.setup_duration, - id: "#{service.id}_activity#{activity_index}", + id: service.id.to_s, late_multiplier: possible_activity.late_multiplier || 0, setup_quantities: vrp.units.collect{ |unit| q = service.quantities.find{ |quantity| quantity.unit == unit } @@ -336,7 +336,6 @@ def solve(vrp, job, thread_proc = nil, &block) ) log "ortools solve problem creation elapsed: #{Time.now - tic}sec", level: :debug - run_ortools(problem, vrp, thread_proc, &block) end diff --git a/wrappers/wrapper.rb b/wrappers/wrapper.rb index 1cc56f8a2..35ebdcd9d 100644 --- a/wrappers/wrapper.rb +++ b/wrappers/wrapper.rb @@ -671,7 +671,13 @@ def check_timewindow_inconsistency(vrp, unfeasible, service) max_earliest_arrival = 0 relation.linked_services.map{ |service_in| - next unless service_in.activity.timewindows.any? + if !service_in.activity && service_in.activities.any?{ |act| act.timewindows.any? } + # TODO: Should consider alternatives + log_string = 'Service activities within relations are not considered for timewindow inconsistency check' + log log_string, relation.as_json.merge(level: :warn) + next + end + next if service_in.activity.nil? || service_in.activity.timewindows.none? earliest_arrival = service_in.activity.timewindows.map{ |tw| tw.day_index.to_i * 86400 + tw.start