Remove trips field in vehicle model, add original info and transform …

…relation types into symbols

CHANGELOG.md

@@ -24,6 +24,7 @@
 ### Removed
 
 - `geometry_polyline` parameter now be provided through `geometry` parameter [#165](https://github.com/Mapotempo/optimizer-api/pull/165)
+- Field `trips` in vehicle model. Use `vehicle_trips` relation instead [#123](https://github.com/Mapotempo/optimizer-api/pull/123)
 
 ### Fixed
 

api/v01/entities/vrp_input.rb

@@ -488,7 +488,6 @@ module VrpVehicles
     optional(:force_start, type: Boolean, documentation: { hidden: true }, desc: '[ DEPRECATED ]')
     optional(:shift_preference, type: String, values: ['force_start', 'force_end', 'minimize_span'], desc: 'Force the vehicle to start as soon as the vehicle timewindow is open,
       as late as possible or let vehicle start at any time. Not available with periodic heuristic, it will always leave as soon as possible.')
-    optional(:trips, type: Integer, default: 1, desc: 'The number of times a vehicle is allowed to return to the depot within its route. Not available with periodic heuristic.')
 
     optional :matrix_id, type: String, desc: 'Related matrix, if already defined'
     optional :value_matrix_id, type: String, desc: 'If any value matrix defined, related matrix index'

models/vehicle.rb

@@ -57,7 +57,6 @@ class Vehicle < Base
 
     field :force_start, default: false
     field :shift_preference, default: :minimize_span
-    field :trips, default: 1
     field :duration, default: nil
     field :overall_duration, default: nil
     field :distance, default: nil
@@ -92,7 +91,6 @@ class Vehicle < Base
     # validates_numericality_of :global_day_index, allow_nil: true
     # validates_inclusion_of :router_dimension, in: %w( time distance )
     # validates_inclusion_of :shift_preference, in: %w( force_start force_end minimize_span )
-    # validates_numericality_of :trips, greater_than_or_equal_to: 0
     # validates_numericality_of :speed_multiplier
     # validates_numericality_of :duration, greater_than_or_equal_to: 0
     # validates_numericality_of :overall_duration, greater_than_or_equal_to: 0

optimizer_wrapper.rb

@@ -24,7 +24,6 @@
 
 require './lib/routers/router_wrapper.rb'
 require './lib/interpreters/multi_modal.rb'
-require './lib/interpreters/multi_trips.rb'
 require './lib/interpreters/periodic_visits.rb'
 require './lib/interpreters/split_clustering.rb'
 require './lib/interpreters/compute_several_solutions.rb'
@@ -200,8 +199,6 @@ def self.solve(service_vrp, job = nil, block = nil)
 
     tic = Time.now
 
-    Interpreters::MultiTrips.new.expand(vrp)
-
     optim_result = nil
 
     unfeasible_services = []

test/lib/interpreters/multi_trips_test.rb

@@ -19,25 +19,6 @@
 require 'date'
 
 class MultiTripsTest < Minitest::Test
-  def test_expand_vehicles_trips
-    vrp = VRP.lat_lon
-    vrp[:vehicles].first[:trips] = 2
-
-    vrp = Interpreters::MultiTrips.new.expand(TestHelper.create(vrp))
-
-    assert_equal 2, vrp.vehicles.size
-    assert_equal 1, vrp.relations.size
-    vrp.relations.each{ |relation|
-      assert_equal :vehicle_trips, relation.type
-      assert_includes relation.linked_vehicle_ids, 'vehicle_0_trip_0'
-      assert_includes relation.linked_vehicle_ids, 'vehicle_0_trip_1'
-    }
-
-    Interpreters::MultiTrips.new.expand(vrp) # consecutive MultiTrips.expand should not produce any error neither inconsistency
-    assert_equal 2, vrp.vehicles.size
-    assert_equal 1, vrp.relations.size
-  end
-
   def test_solve_vehicles_trips_capacity
     vrp = VRP.lat_lon_capacitated
     result = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:ortools] }}, TestHelper.create(vrp), nil)
@@ -46,15 +27,22 @@ def test_solve_vehicles_trips_capacity
 
     # increasing number of trips increases overall available capacity and reduces unassigned :
 
-    vrp[:vehicles].first[:trips] = 2
+    vrp[:vehicles] << vrp[:vehicles].first.dup
+    vrp[:vehicles].last[:id] += '_second_trip'
+    vrp[:relations] = [{
+      type: 'vehicle_trips',
+      linked_vehicle_ids: [vrp[:vehicles].first[:id], vrp[:vehicles].last[:id]]
+    }]
     result = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:ortools] }}, TestHelper.create(vrp), nil)
     assert_equal 2, result[:routes].size
     assert_equal 2, result[:unassigned].size
     routes_start = result[:routes].collect{ |route| route[:activities].first[:begin_time] }
     routes_end = result[:routes].collect{ |route| route[:activities].last[:begin_time] }
     assert_operator routes_end[0], :<=, routes_start[1]
 
-    vrp[:vehicles].first[:trips] = 3
+    vrp[:vehicles] << vrp[:vehicles].first.dup
+    vrp[:vehicles].last[:id] += '_third_trip'
+    vrp[:relations].first[:linked_vehicle_ids] << vrp[:vehicles].last[:id]
     result = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:ortools] }}, TestHelper.create(vrp), nil)
     assert_equal 3, result[:routes].size
     assert_empty result[:unassigned]
@@ -68,22 +56,30 @@ def test_solve_vehicles_trips_capacity
   def test_solve_vehicles_trips_duration
     vrp = VRP.basic
     vrp[:matrices].first[:time] = vrp[:matrices].first[:time].collect{ |l| l.collect{ |v| v.positive? ? 4 : 0 } }
-    vrp[:vehicles].first[:duration] = 4
+    vrp[:vehicles].first[:end_point_id] = vrp[:vehicles].first[:start_point_id]
+    vrp[:vehicles].first[:duration] = 10
     result = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:ortools] }}, TestHelper.create(vrp), nil)
     assert_equal 1, result[:routes].size
     assert_equal 2, result[:unassigned].size
 
     # increasing number of trips increases overall available duration and reduces unassigned :
 
-    vrp[:vehicles].first[:trips] = 2
+    vrp[:vehicles] << vrp[:vehicles].first.dup
+    vrp[:vehicles].last[:id] += '_second_trip'
+    vrp[:relations] = [{
+      type: 'vehicle_trips',
+      linked_vehicle_ids: [vrp[:vehicles].first[:id], vrp[:vehicles].last[:id]]
+    }]
     result = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:ortools] }}, TestHelper.create(vrp), nil)
     assert_equal 2, result[:routes].size
     assert_equal 1, result[:unassigned].size
     routes_start = result[:routes].collect{ |route| route[:activities].first[:begin_time] }
     routes_end = result[:routes].collect{ |route| route[:activities].last[:begin_time] }
     assert_operator routes_end[0], :<=, routes_start[1]
 
-    vrp[:vehicles].first[:trips] = 3
+    vrp[:vehicles] << vrp[:vehicles].first.dup
+    vrp[:vehicles].last[:id] += '_third_trip'
+    vrp[:relations].first[:linked_vehicle_ids] << vrp[:vehicles].last[:id]
     result = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:ortools] }}, TestHelper.create(vrp), nil)
     assert_equal 3, result[:routes].size
     assert_empty result[:unassigned]

test/real_cases_scheduling_test.rb

@@ -75,7 +75,7 @@ def test_without_same_point_day
       result = OptimizerWrapper.wrapper_vrp('ortools', { services: { vrp: [:ortools] }}, vrp, nil)
       assert_equal expecting, result[:routes].sum{ |route| route[:activities].count{ |stop| stop[:service_id] } } + result[:unassigned].size
       unassigned = result[:unassigned].size
-      assert_equal 46, unassigned
+      assert_equal 49, unassigned
 
       vrp = TestHelper.load_vrp(self)
       vrp.resolution_solver = true
@@ -235,7 +235,7 @@ def test_performance_12vl
 
       # voluntarily equal to watch evolution of scheduling algorithm performance
       assert_equal expected, seen, 'Do not have the expected number of total visits'
-      assert_equal 280, unassigned_visits.sum, 'Do not have the expected number of unassigned visits'
+      assert_equal 315, unassigned_visits.sum, 'Do not have the expected number of unassigned visits'
     end
 
     def test_minimum_stop_in_route
@@ -251,7 +251,7 @@ def test_minimum_stop_in_route
       result = OptimizerWrapper.wrapper_vrp('ortools', { services: { vrp: [:ortools] }}, vrp, nil)
       assert result[:routes].all?{ |r| (r[:activities].size - 2).zero? || r[:activities].size - 2 >= 5 }, 'Expecting no route with less than 5 stops unless it is empty'
       assert_operator(should_remain_assigned, :<=, result[:routes].sum{ |r| r[:activities].size - 2 })
-      assert_equal 19, result[:unassigned].size
+      assert_equal 21, result[:unassigned].size
       assert_equal vrp.visits, result[:routes].sum{ |r| r[:activities].count{ |a| a[:service_id] } } + result[:unassigned].size
 
       all_ids = (result[:routes].flat_map{ |route| route[:activities].collect{ |stop| stop[:service_id] } }.compact + result[:unassigned].collect{ |un| un[:service_id] }).uniq
@@ -274,7 +274,7 @@ def test_performance_13vl
 
       # voluntarily equal to watch evolution of scheduling algorithm performance
       assert_equal expected, seen, 'Do not have the expected number of total visits'
-      assert_equal 294, unassigned_visits.sum, 'Do not have the expected number of unassigned visits'
+      assert_equal 298, unassigned_visits.sum, 'Do not have the expected number of unassigned visits'
     end
 
     def test_fill_days_and_post_processing
@@ -287,7 +287,7 @@ def test_fill_days_and_post_processing
       assert_equal vrp.visits,
                    result[:routes].sum{ |r| r[:activities].count{ |s| s[:service_id] } } + result[:unassigned].size,
                    'Do not have the expected number of total visits'
-      assert_equal 74, result[:unassigned].size, 'Do not have the expected number of unassigned visits'
+      assert_equal 39, result[:unassigned].size, 'Do not have the expected number of unassigned visits'
     end
 
     def test_treatment_site

test/test_helper.rb

@@ -68,16 +68,21 @@ def self.coerce(vrp)
 
     [vrp[:services], vrp[:shipments]].each{ |group|
       group&.each{ |s|
-        next if s.nil?
-        next if vrp.is_a?(Hash) && !s.key?(:visits_number)
-
-        raise StandardError, "Service/Shipment #{s[:id]} visits_number (#{s[:visits_number]}) is invalid." unless s[:visits_number].is_a?(Integer) && s[:visits_number].positive?
+        s[:skills]&.map!(&:to_sym)
 
         [s[:activity] || s[:activities] || s[:pickup] || s[:delivery]].flatten.each{ |activity|
           next unless activity[:position]
 
           activity[:position] = activity[:position].to_sym
         }
+
+        next if vrp.is_a?(Hash) && !s.key?(:visits_number)
+
+        unless s[:visits_number].is_a?(Integer) && s[:visits_number].positive?
+          raise StandardError.new(
+            "Service/Shipment #{s[:id]} visits_number (#{s[:visits_number]}) is invalid."
+          )
+        end
       }
     }
 
@@ -102,9 +107,17 @@ def self.coerce(vrp)
     vrp[:configuration][:preprocessing][:partitions]&.each{ |partition| partition[:entity] = partition[:entity].to_sym } if vrp[:configuration] && vrp[:configuration][:preprocessing]
     vrp.preprocessing_partitions&.each{ |partition| partition[:entity] = partition[:entity].to_sym } if vrp.is_a?(Models::Vrp)
 
-    vrp.provide_original_info unless vrp.is_a?(Hash) # TODO: re-dump with this modification
+    vrp[:relations]&.each{ |r| r[:type] = r[:type]&.to_sym }
 
-    vrp[:relations]&.each{ |r| r[:type] = r[:type]&.to_sym } # TODO: re-dump with this modification
+    vrp[:vehicles]&.each{ |v|
+      next if v[:skills].to_a.empty?
+
+      if v[:skills].first&.is_a?(Array)
+        v[:skills].each{ |set| set.map!(&:to_sym) }
+      else
+        v[:skills].map!(&:to_sym)
+      end
+    }
 
     vrp
   end