Merge pull request #67 from senhalil/fix/dicho_infinite_loop

Fix: dicho infinite loop & vehicle without start_point_id

lib/heuristics/dichotomious_approach.rb

@@ -32,7 +32,7 @@ def self.dichotomious_candidate?(service_vrp)
           service_vrp[:vrp].services.size - service_vrp[:vrp].routes.map{ |r| r[:mission_ids].size }.sum > service_vrp[:vrp].resolution_dicho_algorithm_service_limit &&
           !service_vrp[:vrp].schedule_range_indices &&
           service_vrp[:vrp].shipments.empty? && # TODO: check dicho with a P&D instance to remove this condition
-          service_vrp[:vrp].points.all?{ |point| point&.location&.lat && point&.location&.lon } # TODO: Remove and use matrix/matrix_index in clustering
+          service_vrp[:vrp].points.all?{ |point| point&.location&.lat && point&.location&.lon }
         )
     end
 
@@ -69,10 +69,17 @@ def self.dichotomious_heuristic(service_vrp, job = nil, &block)
         if (result.nil? || result[:unassigned].size >= 0.7 * service_vrp[:vrp].services.size) && feasible_vrp(result, service_vrp) &&
            service_vrp[:vrp].vehicles.size > service_vrp[:vrp].resolution_dicho_division_vehicle_limit && service_vrp[:vrp].services.size > service_vrp[:vrp].resolution_dicho_division_service_limit
           sub_service_vrps = []
-          loop do
+
+          3.times do # TODO: move this logic inside the split function
             sub_service_vrps = split(service_vrp, job)
             break if sub_service_vrps.size == 2
           end
+
+          # TODO: instead of an error, we can just continue the optimisation in the child process
+          # by modifying the resolution_dicho_division_X_limit's here so that the child runs the
+          # optimisation instead of trying to split again
+          raise 'dichotomious_heuristic cannot split the problem into two clusters' if sub_service_vrps.size != 2
+
           results = sub_service_vrps.map.with_index{ |sub_service_vrp, index|
             sub_service_vrp[:vrp].resolution_split_number = sub_service_vrps[0][:vrp].resolution_split_number + 1 if !index.zero?
             sub_service_vrp[:vrp].resolution_total_split_number = sub_service_vrps[0][:vrp].resolution_total_split_number if !index.zero?
@@ -436,10 +443,8 @@ def self.split(service_vrp, _job = nil)
           }
         }
       else
-        raise 'Incorrect split size with kmeans' if services_by_cluster.size > 2
-
         # Kmeans return 1 vrp
-        sub_vrp = SplitClustering.build_partial_service_vrp(service_vrp, services_by_cluster[0].map(&:id))[:vrp] # this part of the code is never reached
+        sub_vrp = SplitClustering.build_partial_service_vrp(service_vrp, services_by_cluster[0].map(&:id))[:vrp]
         sub_vrp.points = vrp.points
         sub_vrp.vehicles = vrp.vehicles
         sub_vrp.vehicles.each{ |vehicle|

lib/interpreters/split_clustering.rb

@@ -361,6 +361,8 @@ def self.kmeans_process(nb_clusters, data_items, unit_symbols, limits, options =
         c.centroid_indices = [] if c.centroid_indices.size < nb_clusters
       end
 
+      raise 'Incorrect split in kmeans_process' if clusters.size > nb_clusters # it should be never more
+
       [clusters, centroids_characteristics]
     end
 
@@ -510,8 +512,8 @@ def self.collect_cluster_data(vrp, nb_clusters)
           v_id: [vehicle.id],
           days: compute_day_skills(tw),
           depot: {
-            coordinates: [vehicle.start_point.location.lat, vehicle.start_point.location.lon],
-            matrix_index: vehicle.start_point.matrix_index
+            coordinates: [vehicle.start_point&.location&.lat, vehicle.start_point&.location&.lon],
+            matrix_index: vehicle.start_point&.matrix_index
           },
           capacities: capacities,
           skills: vehicle.skills.flatten.uniq, # TODO : improve case with alternative skills. Current implementation collects all skill sets into one

test/lib/heuristics/dichotomious_test.rb

@@ -104,6 +104,38 @@ def test_dichotomious_condition_limits
       assert Interpreters::Dichotomious.dichotomious_candidate?(vrp: TestHelper.create(vrp), service: :demo, dicho_level: 0)
     end
 
+    def test_infinite_loop_due_to_impossible_to_cluster
+      vrp = VRP.lat_lon
+      vrp[:configuration][:resolution][:duration] = 10
+      vrp[:points].each{ |p| p[:location] = { lat: 45, lon: 5 } } # all at the same location (impossible to cluster)
+
+      vrp[:matrices][0][:time] = Array.new(7){ Array.new(7, 1) }
+      vrp[:matrices][0][:time].each_with_index{ |row, i| row[i] = 0 }
+      vrp[:matrices][0][:distance] = vrp[:matrices][0][:time]
+
+      vrp[:services].each{ |s| s[:activity][:duration] = 500 }
+
+      vrp[:vehicles].first[:duration] = 3600
+      vrp[:vehicles] << vrp[:vehicles].first.dup
+      vrp[:vehicles].last[:id] = 'v_1'
+
+      problem = TestHelper.create(vrp)
+
+      problem.resolution_dicho_algorithm_vehicle_limit = 1
+      problem.resolution_dicho_division_vehicle_limit = 1
+      problem.resolution_dicho_algorithm_service_limit = 5
+      problem.resolution_dicho_division_service_limit = 5
+
+      counter = 0
+      Interpreters::Dichotomious.stub(:kmeans, lambda{ |vrpi, cut_symbol|
+        assert_operator counter, :<, 3, 'Interpreters::Dichotomious::kmeans is called too many times. Either there is an infinite loop due to imposible clustering or dicho split logic is altered.'
+        counter += 1
+        Interpreters::Dichotomious.send(:__minitest_stub__kmeans, vrpi, cut_symbol)
+      }) do
+        OptimizerWrapper.wrapper_vrp('ortools', { services: { vrp: [:ortools] }}, problem, nil)
+      end
+    end
+
     def test_cluster_dichotomious_heuristic
       # Warning: This test is not enough to ensure that two services at the same point will
       # not end up in two different routes because after clustering there is tsp_simple.