improve case with 2 visits

CHANGELOG.md

@@ -10,6 +10,8 @@
 
 ### Changed
 
+- Improve cases where a service has two visits in periodic heuristic: ensure that the second visit can be assigned to the right day [#227](https://github.com/Mapotempo/optimizer-api/pull/227)
+
 ### Removed
 
 - Field `trips` in vehicle model. Use `vehicle_trips` relation instead [#123](https://github.com/Mapotempo/optimizer-api/pull/123)

lib/heuristics/scheduling_heuristic.rb

@@ -206,53 +206,55 @@ def check_solution_validity
 
     private
 
+    def reject_according_to_allow_partial_assignment(service_id, vehicle_id, impacted_days, visit_number)
+      if @allow_partial_assignment
+        @uninserted["#{service_id}_#{visit_number}_#{@services_data[service_id][:raw].visits_number}"] = {
+          original_id: service_id,
+          reason: "Visit not assignable by heuristic, first visit assigned at day #{@services_data[service_id][:used_days].min}"
+        }
+        [true, impacted_days, false]
+      else
+        clean_stops(service_id, vehicle_id)
+        [false, [], true]
+      end
+    end
+
     def plan_next_visits(vehicle_id, service_id, first_unseen_visit)
       return if @services_data[service_id][:raw].visits_number == 1
 
-      days_available = @candidate_routes[vehicle_id].keys
-      next_day = @services_data[service_id][:used_days].max + @services_data[service_id][:heuristic_period]
-      day_to_insert = days_available.select{ |day| day >= next_day.round }.min
       impacted_days = []
-      if day_to_insert
-        diff = day_to_insert - next_day.round
-        next_day += diff
-      end
+      next_day = @services_data[service_id][:used_days].max + @services_data[service_id][:heuristic_period]
+      day_to_insert =
+        if @services_data[service_id][:raw].visits_number == 2
+          day_to_insert = find_day_for_second_visit(vehicle_id, @services_data[service_id][:used_days][0], service_id)
+        else
+          @candidate_routes[vehicle_id].keys.select{ |day| day >= next_day.round }.min
+        end
 
       cleaned_service = false
       need_to_add_visits = false
       (first_unseen_visit..@services_data[service_id][:raw].visits_number).each{ |visit_number|
         inserted_day = nil
         while inserted_day.nil? && day_to_insert && day_to_insert <= @schedule_end && !cleaned_service
-          inserted_day = try_to_insert_at(vehicle_id, day_to_insert, service_id, visit_number) if days_available.include?(day_to_insert)
-          impacted_days |= [inserted_day]
-
-          next_day += @services_data[service_id][:heuristic_period]
-          day_to_insert = days_available.select{ |day| day >= next_day.round }.min
-
-          next if day_to_insert.nil?
-
           diff = day_to_insert - next_day.round
           next_day += diff
-        end
 
-        next if inserted_day
+          inserted_day = try_to_insert_at(vehicle_id, day_to_insert, service_id, visit_number)
 
-        if !@allow_partial_assignment
-          clean_stops(service_id, vehicle_id)
-          cleaned_service = true
-          impacted_days = []
+          next_day += @services_data[service_id][:heuristic_period]
+          day_to_insert = @candidate_routes[vehicle_id].keys.select{ |day| day >= next_day.round }.min
+        end
+
+        if inserted_day
+          impacted_days |= [inserted_day]
         else
-          need_to_add_visits = true # only if allow_partial_assignment, do not add_missing_visits otherwise
-          @uninserted["#{service_id}_#{visit_number}_#{@services_data[service_id][:raw].visits_number}"] = {
-            original_id: service_id,
-            reason: "Visit not assignable by heuristic, first visit assigned at day #{@services_data[service_id][:used_days].min}"
-          }
+          need_to_add_visits, impacted_days, cleaned_service =
+            reject_according_to_allow_partial_assignment(service_id, vehicle_id, impacted_days, visit_number)
         end
       }
 
       @missing_visits[vehicle_id] << service_id if need_to_add_visits
-
-      impacted_days.compact
+      impacted_days
     end
 
     def adjust_candidate_routes(vehicle_id, day_finished)
@@ -527,13 +529,24 @@ def compute_costs_for_route(route_data, set = nil)
       }.compact
     end
 
-    def two_visits_and_can_not_assign_second(vehicle_id, day, service_id)
-      return false unless @services_data[service_id][:raw].visits_number == 2
+    def find_day_for_second_visit(vehicle_id, first_day, service_id)
+      return [] unless @services_data[service_id][:raw].visits_number == 2
+
+      potential_days =
+        if @unlocked.include?(service_id)
+          point = @services_data[service_id][:points_ids].first # there can be only on point in points_ids in this case
+          @points_vehicles_and_days[point][:days].dup
+        else
+          @candidate_routes[vehicle_id].keys
+        end
 
-      next_day = day + @services_data[service_id][:heuristic_period]
-      day_to_insert = @candidate_routes[vehicle_id].keys.select{ |potential_day| potential_day >= next_day.round }.min
+      potential_days.sort!
+      potential_days.delete_if{ |d|
+        !d.between?(first_day + (@services_data[service_id][:raw].minimum_lapse || 0),
+                    first_day + (@services_data[service_id][:raw].maximum_lapse || 2**32))
+      }
 
-      !(day_to_insert && find_best_index(service_id, @candidate_routes[vehicle_id][day_to_insert], false))
+      potential_days.find{ |d| find_best_index(service_id, @candidate_routes[vehicle_id][d], false) }
     end
 
     def same_point_compatibility(service_id, vehicle_id, day)
@@ -886,7 +899,7 @@ def service_compatible_with_route(service_id, route_data, first_visit)
         compatible_vehicle(service_id, route_data) &&
         service_does_not_violate_capacity(service_id, route_data, first_visit) &&
         (!first_visit ||
-          !two_visits_and_can_not_assign_second(vehicle_id, day, service_id) &&
+          find_day_for_second_visit(vehicle_id, day, service_id) &&
           route_data[:available_ids].include?(service_id) &&
           relaxed_or_same_point_day_constraint_respected(service_id, vehicle_id, day) &&
           same_point_compatibility(service_id, vehicle_id, day))
@@ -1215,7 +1228,8 @@ def select_point(insertion_costs, empty_route)
 
     def try_to_insert_at(vehicle_id, day, service_id, visit_number)
       # when adjusting routes, tries to insert [service_id] at [day] for [vehicle]
-      return if @candidate_routes[vehicle_id][day][:completed]
+      return if @candidate_routes[vehicle_id][day].nil? ||
+                @candidate_routes[vehicle_id][day][:completed]
 
       best_index = find_best_index(service_id, @candidate_routes[vehicle_id][day], false)
       return unless best_index

test/lib/heuristics/scheduling_test.rb

@@ -238,7 +238,7 @@ def test_same_cycle_more_difficult
 
       result = OptimizerWrapper.wrapper_vrp('demo', { services: { vrp: [:demo] }}, TestHelper.create(problem), nil)
       assert_equal(3, result[:routes].count{ |route| route[:activities].any?{ |stop| stop[:point_id] == 'point_1' } })
-      assert_equal(4, result[:routes].count{ |route| route[:activities].any?{ |stop| stop[:point_id] == 'point_3' } })
+      assert_equal(3, result[:routes].count{ |route| route[:activities].any?{ |stop| stop[:point_id] == 'point_3' } })
       assert_equal(3, result[:routes].count{ |route| route[:activities].any?{ |stop| stop[:point_id] == 'point_5' } })
     end
 

test/real_cases_scheduling_test.rb

@@ -157,7 +157,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 292, unassigned_visits.sum, 'Do not have the expected number of unassigned visits'
+      assert_equal 279, unassigned_visits.sum, 'Do not have the expected number of unassigned visits'
     end
 
     def test_fill_days_and_post_processing