Merge pull request #241 from senhalil/feat/study_shipment_clustering_…

…and_vroom_improvement

collect_items and Vroom can handle shipment relations

models/concerns/validate_data.rb

@@ -250,6 +250,16 @@ def check_relations(periodic_heuristic)
     # shipment relation consistency
     if @hash[:relations]&.any?{ |r| r[:type] == :shipment }
       shipment_relations = @hash[:relations].select{ |r| r[:type] == :shipment }
+      service_ids = @hash[:services].map{ |s| s[:id] }
+
+      shipments_with_invalid_linked_ids = shipment_relations.reject{ |r| r[:linked_ids].all?{ |s_id| service_ids.include?(s_id) } }
+      unless shipments_with_invalid_linked_ids.empty?
+        raise OptimizerWrapper::DiscordantProblemError.new(
+          'Shipment relations need to have two valid services -- a pickup and a delivery. ' \
+          'The following services of shipment relations are invalid: ' \
+          "#{shipments_with_invalid_linked_ids.flat_map{ |r| r[:linked_ids].select{ |s_id| service_ids.exclude?(s_id) } }.uniq.sort.join(', ')}"
+        )
+      end
 
       shipments_not_having_exactly_two_linked_ids = shipment_relations.reject{ |r| r[:linked_ids].uniq.size == 2 }
       unless shipments_not_having_exactly_two_linked_ids.empty?

models/vehicle.rb

@@ -232,7 +232,7 @@ def total_work_time_in_range(range_start, range_end)
     end
 
     def work_duration
-      raise OptimizerWrapper::DiscordantProblemError, 'Vehicle should not have sequence timewindow if there is no schedule' unless self.sequence_timewindows.empty?
+      raise 'vehicle::work_duration cannot handle sequence_timewindow' if self.sequence_timewindows.any?
 
       timewindow_duration =
         if self.timewindow&.end

test/lib/interpreters/split_clustering_test.rb

@@ -661,8 +661,9 @@ def test_collect_data_items_respects_linking_relations
       Interpreters::SplitClustering::LINKING_RELATIONS.each_with_index{ |relation, index|
         problem[:relations] << { type: relation, linked_ids: [] }
         n_service_per_relation.times.each{ |i|
-          problem[:services] << dummy_service.dup
+          problem[:services] << Oj.load(Oj.dump(dummy_service))
           problem[:services].last[:id] = "service_#{relation}_#{i}"
+          problem[:services].last[:activity][:point_id] = "point_#{i}"
           problem[:relations].last[:linked_ids] << problem[:services].last[:id]
         }
         expected_linked_items[relation] << Array.new(n_service_per_relation){ |i| index * n_service_per_relation + i }

test/models/vehicle_test.rb

@@ -27,7 +27,7 @@ def test_work_duration
 
       # work duration is only supposed to be called when there is no schedule
       # therefore, work_duration should not be called if vehicle has sequence_timewindows
-      assert_raises OptimizerWrapper::DiscordantProblemError do
+      assert_raises do
         vrp.vehicles.first.work_duration
       end
 

test/real_cases_scheduling_solver_test.rb

@@ -64,7 +64,7 @@ def test_performance_12vl_with_solver
         result = OptimizerWrapper.wrapper_vrp('ortools', { services: { vrp: [:ortools] }}, vrp, nil)
         unassigned_nb + result[:unassigned].size
       }
-      assert unassigned_visits <= 225, "Expecting less than 441 unassigned visits, have #{unassigned_visits}"
+      assert_operator unassigned_visits, :<=, 225, 'Expecting less unassigned visits'
     end
 
     def test_performance_britanny_with_solver

test/wrapper_test.rb

@@ -3026,11 +3026,14 @@ def test_assert_inapplicable_relations
       linked_ids: [],
       linked_vehicle_ids: [],
       lapse: 1
+    }, {
+      type: :shipment,
+      linked_ids: ['service_1', 'service_2']
     }]
 
     vrp = TestHelper.create(problem)
-    refute_includes OptimizerWrapper.config[:services][:vroom].inapplicable_solve?(vrp), :assert_no_relations
-    refute_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(vrp), :assert_no_relations
+    refute_includes OptimizerWrapper.config[:services][:vroom].inapplicable_solve?(vrp), :assert_no_relations_except_simple_shipments
+    refute_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(vrp), :assert_no_relations_except_simple_shipments
 
     problem[:relations] = [{
       type: :vehicle_group_duration,
@@ -3040,8 +3043,8 @@ def test_assert_inapplicable_relations
     }]
 
     vrp = TestHelper.create(problem)
-    assert_includes OptimizerWrapper.config[:services][:vroom].inapplicable_solve?(vrp), :assert_no_relations
-    refute_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(vrp), :assert_no_relations
+    assert_includes OptimizerWrapper.config[:services][:vroom].inapplicable_solve?(vrp), :assert_no_relations_except_simple_shipments
+    refute_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(vrp), :assert_no_relations_except_simple_shipments
   end
 
   def test_solver_needed
@@ -3062,13 +3065,14 @@ def test_first_solution_acceptance_with_solvers
     refute_includes OptimizerWrapper.config[:services][:ortools].inapplicable_solve?(vrp), :assert_no_first_solution_strategy
   end
 
-  def test_vroom_not_called_if_max_split_lower_thant_services_size
+  def test_vroom_cannot_be_called_synchronously_if_max_split_lower_than_services_size
     problem = VRP.lat_lon_two_vehicles
     assert_empty OptimizerWrapper.config[:services][:vroom].inapplicable_solve?(TestHelper.create(problem))
+    assert OptimizerWrapper.config[:services][:vroom].solve_synchronous?(TestHelper.create(problem))
 
     problem[:configuration][:preprocessing] = { max_split_size: 2 }
-    assert_includes OptimizerWrapper.config[:services][:vroom].inapplicable_solve?(TestHelper.create(problem)),
-                    :assert_not_a_split_solve_candidate
+    assert_empty OptimizerWrapper.config[:services][:vroom].inapplicable_solve?(TestHelper.create(problem))
+    refute OptimizerWrapper.config[:services][:vroom].solve_synchronous?(TestHelper.create(problem))
   end
 
   def test_simplify_constraints_simplifies_pause
@@ -3179,4 +3183,51 @@ def test_simplified_pause_returns_the_same_cost
                    'Manually inserting the simplified pause should give the same route total time'
     }
   end
+
+  def test_simplify_service_setup_duration
+    problem = VRP.basic
+
+    original_time_matrix = Oj.load(Oj.dump(problem[:matrices][0][:time]))
+    problem[:services].each_with_index{ |service, index|
+      service[:activity][:setup_duration] = 600 + index
+    }
+
+    vrp = TestHelper.create(problem)
+
+    OptimizerWrapper.config[:services][:demo].simplify_service_setup_duration_and_vehicle_setup_modifiers(vrp)
+
+    vrp.matrices[0][:time].each_with_index{ |row, row_index|
+      row[1..-1].each_with_index{ |value, col_index|
+        original_value = original_time_matrix[row_index][col_index + 1] # the first column is skipped
+        if original_value.zero?
+          assert_equal 0, value, 'A zero time should stay zero after setup_duration simplification'
+        else
+          assert_equal original_value + 600 + col_index, value, 'Time should have been increased with the setup duration'
+        end
+      }
+    }
+  end
+
+  def test_cannot_simplify_service_setup_duration
+    problem = VRP.basic
+
+    original_time_matrix = Oj.load(Oj.dump(problem[:matrices][0][:time]))
+    problem[:services] << Oj.load(Oj.dump(problem[:services].last)) # the same point_id
+    problem[:services].last[:id] += '_dup'
+
+    problem[:services].each_with_index{ |service, index|
+      service[:activity][:setup_duration] = 600 + index # but different setup_duration
+    }
+
+    vrp = TestHelper.create(problem)
+
+    OptimizerWrapper.config[:services][:demo].simplify_service_setup_duration_and_vehicle_setup_modifiers(vrp)
+
+    dup_service_matrix_index = vrp.services.last.activity.point.matrix_index
+    vrp.matrices[0][:time].each_with_index{ |row, row_index|
+      assert_equal original_time_matrix[row_index][dup_service_matrix_index],
+                   row[dup_service_matrix_index],
+                   'Cannot simplify the pause if the point has multiple setup durations'
+    }
+  end
 end

test/wrappers/ortools_test.rb

@@ -441,7 +441,7 @@ def test_duration_adjusted_by_presence_of_rest
       }],
       configuration: {
         resolution: {
-          duration: 100,
+          duration: 200,
         }
       }
     }
@@ -4379,7 +4379,7 @@ def test_rest_with_exclusion_cost
       }],
       configuration: {
         resolution: {
-          duration: 100,
+          duration: 200,
         }
       }
     }