Merge pull request 318 from senhalil/feat/general_fixes_and_vehicle_c…

…ompatibility_check_improvement

[optim-ortools] General fixes and vehicle compatibility check improvement

.rubocop.yml

@@ -23,6 +23,9 @@ Style/FrozenStringLiteralComment:
 Style/NumericPredicate:
   Enabled: false
 
+# Style/UnlessLogicalOperators: # TODO: activate when rubocop gem is upgraded
+#   EnforcedStyle: forbid_logical_operators
+
 Layout/EmptyLines:
   Severity: warning
 

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Added
 
 - 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)
 
 ### Changed
 
@@ -24,6 +25,7 @@
 
 - Split duration among partitions correctly [#336](https://github.com/Mapotempo/optimizer-api/pull/336)
 - Fix find_best_heuristic selection logic [#337](https://github.com/Mapotempo/optimizer-api/pull/337)
+- Prevent periodic heuristic overwriting supplied initial routes [#318](https://github.com/Mapotempo/optimizer-api/pull/318)
 
 ## [v1.8.2] - 2022-01-19
 

Gemfile.lock

@@ -257,7 +257,7 @@ GEM
       rack (~> 2.2)
       rack-protection (= 2.1.0)
       tilt (~> 2.0)
-    solargraph (0.44.0)
+    solargraph (0.44.2)
       backport (~> 1.2)
       benchmark
       bundler (>= 1.17.2)

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.11.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.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).
 
 Note : when updating OR-Tools you should to recompile optimizer-ortools.
 

api/v01/entities/vrp_input.rb

@@ -283,10 +283,10 @@ module VrpMisc
   params :vrp_request_route do
     optional(:vehicle_id, type: String, desc: 'Vehicle linked to the current described route')
     optional(:indice, type: Integer, documentation: { hidden: true }, desc: '[ DEPRECATED : use day_index instead ]')
-    optional(:index, type: Integer, desc: 'Index of the route. Must be provided if first_solution_strategy is \'periodic\'.')
+    optional(:day_index, type: Integer, desc: 'Index of the route. Must be provided if first_solution_strategy is \'periodic\'.')
     optional(:date, type: Date, desc: 'Date of the route. Must be provided if first_solution_strategy is \'periodic\'.')
     requires(:mission_ids, type: Array[String], desc: 'Initial state or partial state of the current vehicle route', coerce_with: ->(val) { val.is_a?(String) ? val.split(/,/) : val })
-    mutually_exclusive :indice, :index, :day
+    mutually_exclusive :indice, :day_index, :date
   end
 
   params :vrp_request_subtour do

api/v01/vrp.rb

@@ -135,7 +135,7 @@ class Vrp < APIBase
               vrp.errors.add(:empty_vrp, message: 'VRP structure is empty') if vrp_params&.keys&.empty?
               error!("Model Validation Error: #{vrp.errors}", 400)
             else
-              vrp.router = OptimizerWrapper.router(OptimizerWrapper.access[api_key][:router_api_key] || profile[:router_api_key] || OptimizerWrapper.config[:router][:api_key])
+              vrp.router = OptimizerWrapper.router(OptimizerWrapper.access[api_key][:router_api_key] || profile[:router_api_key]) if OptimizerWrapper.access[api_key][:router_api_key] || profile[:router_api_key]
               ret = OptimizerWrapper.wrapper_vrp(api_key, profile, vrp, checksum)
               count_incr :optimize, transactions: vrp.transactions
               if ret.is_a?(String)

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.11.0}
+FROM ${REGISTRY:-registry.mapotempo.com/}mapotempo-${BRANCH:-ce}/optimizer-ortools:${OPTIMIZER_ORTOOLS_VERSION:-v1.12.0}
 ARG BUNDLE_WITHOUT
 
 ENV LANG C.UTF-8

lib/interpreters/periodic_visits.rb

@@ -57,8 +57,17 @@ def expand(vrp, job, &block)
       @periods.uniq!
       generate_relations_on_periodic_vehicles(vrp, vehicles_linking_relations)
 
-      if vrp.configuration.preprocessing.first_solution_strategy.to_a.first != 'periodic' && vrp.services.any?{ |service| service.visits_number > 1 }
+      if vrp.routes.empty? && vrp.services.any?{ |service| service.visits_number > 1 }
         vrp.routes = generate_routes(vrp)
+      elsif !vrp.periodic_heuristic?
+        expanded_service_ids = @expanded_services.transform_values{ |service| service.map(&:id) }
+        vrp.routes.sort_by(&:day_index).each{ |route|
+          # Note that we sort_by day_index so we can assume that the existing routes have the visits in the correct
+          # order. That is, the first appearance of a service id will be the visit_1_X, the next will be visit_2_X,
+          # and the last will be visit_X_X.
+          route.mission_ids.collect!{ |sid| expanded_service_ids[sid].shift }
+          route.vehicle_id += "_#{route.day_index}"
+        }
       end
 
       vrp
@@ -90,14 +99,14 @@ def generate_timewindows(timewindows_set)
 
     def generate_relations(vrp)
       vrp.relations.flat_map{ |relation|
-        unless relation.linked_ids.uniq{ |s_id| @expanded_services[s_id].size }.size <= 1
+        unless relation.linked_ids.uniq{ |s_id| @expanded_services[s_id]&.size.to_i }.size <= 1
           raise "Cannot expand relations of #{relation.linked_ids} because they have different visits_number"
         end
 
         # keep the original relation if it is another type of relation or if it doesn't belong to an unexpanded service.
-        next relation if relation.linked_services.empty? || @expanded_services[relation.linked_ids.first].empty?
+        next relation if relation.linked_services.empty? || @expanded_services[relation.linked_ids.first]&.size.to_i == 0
 
-        Array.new(@expanded_services[relation.linked_ids.first].size){ |visit_index|
+        Array.new(@expanded_services[relation.linked_ids.first]&.size.to_i){ |visit_index|
           linked_ids = relation.linked_ids.collect{ |s_id| @expanded_services[s_id][visit_index].id }
 
           Models::Relation.create(
@@ -111,47 +120,45 @@ def generate_relations_between_visits(vrp, mission)
       # TODO : need to uniformize generated relations whether mission has minimum AND maximum lapse or only one of them
       return unless mission.visits_number > 1
 
-      if mission.minimum_lapse && mission.maximum_lapse
-        (2..mission.visits_number).each{ |index|
+      if mission.minimum_lapse && mission.maximum_lapse && (mission.minimum_lapse == mission.maximum_lapse)
+        2.upto(mission.visits_number){ |index|
           current_lapse = (index - 1) * mission.minimum_lapse.to_i
           vrp.relations << Models::Relation.create(
             type: :minimum_day_lapse,
             linked_ids: ["#{mission.id}_1_#{mission.visits_number}", "#{mission.id}_#{index}_#{mission.visits_number}"],
             lapses: [current_lapse]
           )
         }
-        (2..mission.visits_number).each{ |index|
+        2.upto(mission.visits_number){ |index|
           current_lapse = (index - 1) * mission.maximum_lapse.to_i
           vrp.relations << Models::Relation.create(
             type: :maximum_day_lapse,
             linked_ids: ["#{mission.id}_1_#{mission.visits_number}", "#{mission.id}_#{index}_#{mission.visits_number}"],
             lapses: [current_lapse]
           )
         }
-      elsif mission.minimum_lapse
-        (2..mission.visits_number).each{ |index|
-          current_lapse = mission.minimum_lapse.to_i
+      else
+        if mission.minimum_lapse
           vrp.relations << Models::Relation.create(
             type: :minimum_day_lapse,
-            linked_ids: ["#{mission.id}_#{index - 1}_#{mission.visits_number}", "#{mission.id}_#{index}_#{mission.visits_number}"],
-            lapses: [current_lapse]
+            linked_ids: 1.upto(mission.visits_number).map{ |index| "#{mission.id}_#{index}_#{mission.visits_number}" },
+            lapses: [mission.minimum_lapse.to_i]
           )
-        }
-      elsif mission.maximum_lapse
-        (2..mission.visits_number).each{ |index|
-          current_lapse = mission.maximum_lapse.to_i
+        end
+        if mission.maximum_lapse
           vrp.relations << Models::Relation.create(
             type: :maximum_day_lapse,
-            linked_ids: ["#{mission.id}_#{index - 1}_#{mission.visits_number}", "#{mission.id}_#{index}_#{mission.visits_number}"],
-            lapses: [current_lapse]
+            linked_ids: 1.upto(mission.visits_number).map{ |index| "#{mission.id}_#{index}_#{mission.visits_number}" },
+            lapses: [mission.maximum_lapse.to_i]
           )
-        }
+        end
       end
     end
 
     def generate_services(vrp)
-      @expanded_services = Hash.new{ |h, k| h[k] = [] }
-      vrp.services.collect{ |service|
+      @expanded_services = {}
+      new_services = []
+      vrp.services.each{ |service|
         # transform service data into periodic data
         (service.activity ? [service.activity] : service.activities).each{ |activity|
           activity.timewindows = generate_timewindows(activity.timewindows)
@@ -161,7 +168,7 @@ def generate_services(vrp)
         # TODO : create visit in model
         @periods << service.visits_number
 
-        visits = (0..service.visits_number - 1).collect{ |visit_index|
+        0.upto(service.visits_number - 1){ |visit_index|
           next if service.unavailable_visit_indices.include?(visit_index)
 
           new_service = duplicate_safe(
@@ -174,15 +181,16 @@ def generate_services(vrp)
           )
           new_service.skills += ["#{visit_index + 1}_f_#{service.visits_number}"] if !service.minimum_lapse && !service.maximum_lapse && service.visits_number > 1
 
+          @expanded_services[service.id] ||= []
           @expanded_services[service.id] << new_service
 
-          new_service
-        }.compact
+          new_services << new_service
+        }
 
         generate_relations_between_visits(vrp, service)
+      }
 
-        visits
-      }.flatten
+      new_services
     end
 
     def build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
@@ -204,40 +212,39 @@ def build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
 
     def generate_vehicles(vrp)
       rests_durations = Array.new(vrp.vehicles.size, 0)
-      new_vehicles = vrp.vehicles.collect{ |vehicle|
-        @equivalent_vehicles[vehicle.id] = []
+      new_vehicles = []
+      vrp.vehicles.each{ |vehicle|
+        @equivalent_vehicles[vehicle.id] = [] # equivalent_vehicle_ids !
         @equivalent_vehicles[vehicle.original_id] = []
-        vehicles = (vrp.configuration.schedule.range_indices[:start]..vrp.configuration.schedule.range_indices[:end]).collect{ |vehicle_day_index|
+        vrp.configuration.schedule.range_indices[:start].upto(vrp.configuration.schedule.range_indices[:end]){ |vehicle_day_index|
           next if vehicle.unavailable_days.include?(vehicle_day_index)
 
           timewindows = [vehicle.timewindow || vehicle.sequence_timewindows].flatten
           if timewindows.empty?
-            new_vehicle = build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
-            new_vehicle
+            new_vehicles << build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
           else
-            timewindows.select{ |timewindow| timewindow.day_index.nil? || timewindow.day_index == vehicle_day_index % 7 }.collect{ |associated_timewindow|
+            timewindows.each{ |associated_timewindow|
+              next unless associated_timewindow.day_index.nil? || associated_timewindow.day_index == vehicle_day_index % 7
+
               new_vehicle = build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
               new_vehicle.timewindow = Models::Timewindow.create(start: associated_timewindow.start || 0, end: associated_timewindow.end || 86400)
               if @have_day_index
                 new_vehicle.timewindow.start += vehicle_day_index * 86400
                 new_vehicle.timewindow.end += vehicle_day_index * 86400
               end
-              new_vehicle
-            }.compact
+              new_vehicles << new_vehicle
+            }
           end
-        }.compact
+        }
 
-        if vehicle.overall_duration
-          new_relation = Models::Relation.create(
-            type: :vehicle_group_duration,
-            linked_vehicle_ids: @equivalent_vehicles[vehicle.original_id],
-            lapses: [vehicle.overall_duration + rests_durations[index]]
-          )
-          vrp.relations << new_relation
-        end
+        next unless vehicle.overall_duration
 
-        vehicles
-      }.flatten
+        vrp.relations << Models::Relation.create(
+          type: :vehicle_group_duration,
+          linked_vehicle_ids: @equivalent_vehicles[vehicle.original_id],
+          lapses: [vehicle.overall_duration + rests_durations[index]]
+        )
+      }
 
       new_vehicles
     end

models/activity.rb

@@ -47,5 +47,27 @@ def vrp_result(options = {})
       end
       hash
     end
+
+    def duration_on(vehicle = nil)
+      case vehicle
+      when nil
+        duration
+      when Models::Vehicle
+        duration * vehicle.coef_service + vehicle.additional_service
+      else
+        raise 'Unknown object type for activity duration calculation'
+      end
+    end
+
+    def setup_duration_on(vehicle = nil)
+      case vehicle
+      when nil
+        setup_duration
+      when Models::Vehicle
+        setup_duration > 0 ? setup_duration * vehicle.coef_setup + vehicle.additional_setup : 0
+      else
+        raise 'Unknown object type for activity setup_duration calculation'
+      end
+    end
   end
 end

models/concerns/distance_matrix.rb

@@ -45,12 +45,6 @@ def compute_need_matrix(&block)
         point.matrix_index = index
         [point.location.lat, point.location.lon]
       }
-      vehicles.select(&:start_point).each{ |v|
-        v.start_point.matrix_index = points.find{ |p| p.id == v.start_point.id }.matrix_index
-      }
-      vehicles.select(&:end_point).each{ |v|
-        v.end_point.matrix_index = points.find{ |p| p.id == v.end_point.id }.matrix_index
-      }
 
       uniq_need_matrix = need_matrix.collect{ |vehicle, dimensions|
         [vehicle.router_mode.to_sym, dimensions | vrp_need_matrix, vehicle.router_options]

models/concerns/periodic_service.rb

@@ -21,9 +21,13 @@ module PeriodicService
   extend ActiveSupport::Concern
 
   def can_affect_all_visits?(service)
+    return true unless self.schedule?
+
     return true if service.visits_number == 1
 
     self.vehicles.any?{ |vehicle|
+      next unless service.vehicle_compatibility.nil? || service.vehicle_compatibility[vehicle.id] # vehicle already eliminated
+
       current_day = self.configuration.schedule.range_indices[:start]
       decimal_day = current_day
       current_visit = 0

models/service.rb

@@ -54,6 +54,8 @@ class Service < Base
     field :skills
     field :original_skills
 
+    field :vehicle_compatibility, as_json: :none # vehicle_compatibility[v_id] == {true -> compatible, false -> incompatible, nil -> not checked yet}
+
     ## has_many :period_activities, class_name: 'Models::Activity' # Need alternatives visits
     belongs_to :activity, class_name: 'Models::Activity'
     has_many :activities, class_name: 'Models::Activity'

models/solution/parsers/route_parser.rb

@@ -127,11 +127,10 @@ def self.compute_time_info(stop, previous_departure, travel_time)
           previous_departure + travel_time
         ].max || 0
       if travel_time > 0
-        earliest_arrival += stop.activity.setup_duration * @route.vehicle.coef_setup + @route.vehicle.additional_setup
+        earliest_arrival += stop.activity.setup_duration_on(@route.vehicle)
       end
       stop.info.begin_time = earliest_arrival
-      stop.info.end_time = earliest_arrival +
-        (stop.activity.duration * @route.vehicle.coef_service + @route.vehicle.additional_service)
+      stop.info.end_time = earliest_arrival + stop.activity.duration_on(@route.vehicle)
       stop.info.departure_time = stop.info.end_time
       earliest_arrival
     end

models/timewindow.rb

@@ -40,5 +40,15 @@ def self.create(hash, _options = {})
 
       super(hash)
     end
+
+    def safe_end(lateness_allowed = false)
+      if self.end
+        self.end + (lateness_allowed ? self.maximum_lateness : 0)
+      elsif self.day_index
+        86399 # 24h - 1sec
+      else
+        2147483647 # 2**31 - 1
+      end
+    end
   end
 end

optimizer_wrapper.rb

@@ -21,6 +21,8 @@
 require_all 'util'
 
 module OptimizerWrapper
+  @zip_condition = false
+
   def self.wrapper_vrp(api_key, profile, vrp, checksum, job_id = nil)
     inapplicable_services = []
     apply_zones(vrp)
@@ -185,10 +187,14 @@ def self.solve(service_vrp, job = nil, block = nil)
     else
       unfeasible_services = config[:services][service].detect_unfeasible_services(vrp)
 
+      # TODO: Eliminate the points which has no feasible vehicle or service
+
       vrp.compute_matrix(&block)
 
       config[:services][service].check_distances(vrp, unfeasible_services)
 
+      # TODO: Eliminate the vehicles which cannot serve any service vrp.services.all?{ |s| s.vehicle_compatibility[v.id] == false }
+
       # Remove infeasible services
       services_to_reinject = []
       unfeasible_services.each_key{ |una_service_id|

test/lib/heuristics/periodic_instance_validity_test.rb

@@ -139,17 +139,5 @@ def test_not_too_many_visits_provided_in_route
       assert_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(TestHelper.create(problem)), :assert_not_too_many_visits_in_route
     end
 
-    def test_reject_if_periodic_route_without_periodic_heuristic
-      problem = VRP.periodic
-      problem[:routes] = [{
-        vehicle_id: 'vehicle_0',
-        indice: 0,
-        mission_ids: ['service_1']
-      }]
-      refute_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(TestHelper.create(problem)), :assert_no_route_if_schedule_without_periodic_heuristic
-
-      problem[:configuration][:preprocessing][:first_solution_strategy] = []
-      assert_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(TestHelper.create(problem)), :assert_no_route_if_schedule_without_periodic_heuristic
-    end
   end
 end