Fix activities and relations

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
 

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.
 

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

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|

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[

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({}),

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

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: [{

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

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
 

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