Provide rest timewindows compatible with vehicle

CHANGELOG.md

@@ -17,6 +17,7 @@
 - The internal solution object now use a single model for all the resolution methods. This improve the consistency and the completness of the solutions returned. [#321](https://github.com/Mapotempo/optimizer-api/pull/321)
 - Mapotempo github wiki is now directly part of the project [#351](https://github.com/Mapotempo/optimizer-api/pull/351)
 - Empty/Fill behavior changed so that the loads are naturally managed within optimizer-ortools [#370](https://github.com/Mapotempo/optimizer-api/pull/370)
+- Allow multiple rests with the same day index in periodic heuristic algorithm (`first_solution_strategy='periodic'`) [#273](https://github.com/Mapotempo/optimizer-api/pull/273)
 
 ### Removed
 
@@ -27,6 +28,7 @@
 - Split duration among partitions correctly [#336](https://github.com/Mapotempo/optimizer-api/pull/336)
 - Fix find_best_heuristic selection logic [#337](https://github.com/Mapotempo/optimizer-api/pull/337)
 - Prevent periodic heuristic overwriting supplied initial routes [#318](https://github.com/Mapotempo/optimizer-api/pull/318)
+- Rests now have a correct timewindow according to the vehicle in periodic heuristic algorithm (`first_solution_strategy='periodic'`) [#273](https://github.com/Mapotempo/optimizer-api/pull/273)
 - Split independent VRP respects the skills of services in relations  [#379](https://github.com/Mapotempo/optimizer-api/pull/379)
 
 ## [v1.8.2] - 2022-01-19

lib/interpreters/periodic_visits.rb

@@ -73,11 +73,14 @@ def expand(vrp, job, &block)
       vrp
     end
 
-    def generate_timewindows(timewindows_set)
+    def generate_timewindows(timewindows_set, fixed_day_index = nil)
       return nil if timewindows_set.empty?
 
       timewindows_set.flat_map{ |timewindow|
-        if @have_day_index
+        if @have_day_index && fixed_day_index
+          [Models::Timewindow.create(start: timewindow.start + fixed_day_index * 86400,
+                                    end: (timewindow.end || 86400) + fixed_day_index * 86400)]
+        elsif @have_day_index
           first_day =
             if timewindow.day_index
               (@schedule_start..@schedule_end).find{ |day| day % 7 == timewindow.day_index }
@@ -193,12 +196,12 @@ def generate_services(vrp)
       new_services
     end
 
-    def build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
+    def build_vehicle(vrp, vehicle, vehicle_day_index, vehicle_timewindow, rests_durations)
       new_vehicle_hash = vehicle.as_json(except: [:id, :start_point_id, :end_point_id, :sequence_timewindows])
       new_vehicle_hash[:global_day_index] = vehicle_day_index
       new_vehicle_hash[:skills] = associate_skills(vehicle, vehicle_day_index)
-      new_vehicle_hash[:rests] = generate_rests(vehicle, vehicle_day_index, rests_durations)
-
+      new_vehicle_hash[:rests] = generate_rests(vehicle, vehicle_day_index, vehicle_timewindow, rests_durations)
+      new_vehicle_hash[:timewindow] = vehicle_timewindow.as_json
       new_vehicle = Models::Vehicle.create(new_vehicle_hash)
 
     # Current depot points may not be currently in the active_hash base due to
@@ -225,13 +228,13 @@ def generate_vehicles(vrp)
 
           timewindows = [vehicle.timewindow || vehicle.sequence_timewindows].flatten
           if timewindows.empty?
-            new_vehicles << build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
+            new_vehicles << build_vehicle(vrp, vehicle, vehicle_day_index, nil, rests_durations)
           else
             timewindows.each{ |associated_timewindow|
               next unless associated_timewindow.day_index.nil? || associated_timewindow.day_index == vehicle_day_index % 7
 
-              new_vehicle = build_vehicle(vrp, vehicle, vehicle_day_index, rests_durations)
-              new_vehicle.timewindow = Models::Timewindow.create(start: associated_timewindow.start || 0, end: associated_timewindow.end || 86400)
+              new_timewindow = Models::Timewindow.create(start: associated_timewindow.start || 0, end: associated_timewindow.end || 86400)
+              new_vehicle = build_vehicle(vrp, vehicle, vehicle_day_index, new_timewindow, rests_durations)
               if @have_day_index
                 new_vehicle.timewindow.start += vehicle_day_index * 86400
                 new_vehicle.timewindow.end += vehicle_day_index * 86400
@@ -376,15 +379,18 @@ def generate_routes(vrp)
       routes
     end
 
-    def generate_rests(vehicle, day_index, rests_durations)
+    def generate_rests(vehicle, day_index, vehicle_timewindow, rests_durations)
       vehicle.rests.collect{ |rest|
-        next unless rest.timewindows.empty? || rest.timewindows.any?{ |timewindow| timewindow.day_index.nil? || timewindow.day_index == day_index % 7 }
-
-        # rest is compatible with this vehicle day
-        new_rest = Marshal.load(Marshal.dump(rest))
-        new_rest.id = "#{new_rest.id}_#{day_index + 1}"
+        # Rests can not have more than one timewindow for now
+        next if (vehicle_timewindow && rest.timewindows.first &&
+                !rest.timewindows.first.compatible_with?(vehicle_timewindow)) ||
+                (rest.timewindows.first&.day_index && rest.timewindows.first.day_index != day_index % 7)
+
+        # rest is compatible with this vehicle day and timewindow
+        new_rest = Models::Rest.create(rest.as_json(except: [:id]))
+        new_rest.original_id = rest.original_id || rest.id
         rests_durations[-1] += new_rest.duration
-        new_rest.timewindows = generate_timewindows(rest.timewindows)
+        new_rest.timewindows = generate_timewindows(rest.timewindows, day_index)
         new_rest
       }.compact
     end

models/rest.rb

@@ -20,6 +20,7 @@
 module Models
   class Rest < Activity
     field :id
+    field :original_id
     field :duration, default: 0
     field :late_multiplier, default: 0, vrp_result: :hide
     field :exclusion_cost, default: nil, vrp_result: :hide

models/solution/parsers/stop_parser.rb

@@ -80,8 +80,8 @@ def self.parse(point, options)
   class RestParser
     def self.parse(rest, options)
       {
-        id: rest.id,
-        rest_id: rest.id,
+        id: rest.original_id || rest.id,
+        rest_id: rest.original_id || rest.id,
         type: :rest,
         activity: Models::Rest.new(rest.as_json),
         info: options[:info] || Models::Solution::Stop::Info.new({})

models/timewindow.rb

@@ -50,5 +50,16 @@ def safe_end(lateness_allowed = false)
         2147483647 # 2**31 - 1
       end
     end
+
+    def compatible_with?(timewindow, check_days = true)
+      raise unless timewindow.is_a?(Models::Timewindow)
+
+      return false if check_days &&
+                      self.day_index && timewindow.day_index &&
+                      self.day_index != timewindow.day_index
+
+      (self.end.nil? || timewindow.start.nil? || timewindow.start <= self.end + (self.maximum_lateness || 0)) &&
+        (self.start.nil? || timewindow.end.nil? || timewindow.end + (timewindow.maximum_lateness || 0) >= self.start)
+    end
   end
 end

test/lib/interpreters/interpreter_test.rb

@@ -1617,4 +1617,27 @@ def test_expand_multitrips_relations
     assert_equal 4, expanded_vrp.vehicles.size
     assert_equal 2, (expanded_vrp.relations.count{ |r| r[:type] == :vehicle_trips })
   end
+
+  def test_expand_rests_timewindows
+    problem = VRP.lat_lon_two_vehicles
+    problem[:rests] = [{ id: 'rest', duration: 10 }]
+    problem[:vehicles].each{ |v| v[:rest_ids] = ['rest'] }
+    problem[:configuration][:schedule] = { range_indices: { start: 0, end: 3 }}
+
+    expanded_vrp = periodic_expand(problem)
+    assert_equal 8, expanded_vrp.vehicles.size
+    assert_equal 8, expanded_vrp.vehicles.flat_map(&:rest_ids).uniq.size
+
+    problem[:rests].first[:timewindows] = [{ start: 2, end: 3 }]
+    expanded_vrp = periodic_expand(problem)
+    assert_equal 8, expanded_vrp.vehicles.flat_map(&:rest_ids).uniq.size
+    assert(expanded_vrp.vehicles.all?{ |v| v.rests.first.timewindows.first.start == 2 })
+    assert(expanded_vrp.vehicles.all?{ |v| v.rests.first.timewindows.first.end == 3 })
+
+    problem[:rests].first[:timewindows].first[:day_index] = 1
+    expanded_vrp = periodic_expand(problem)
+    assert_equal 2, expanded_vrp.vehicles.flat_map(&:rest_ids).uniq.size
+    vehicle_at_day_one = expanded_vrp.vehicles.find{ |v| v.global_day_index == 1 }
+    assert_equal 86402, vehicle_at_day_one.rests.first.timewindows.first.start
+  end
 end

test/models/timewindow_test.rb

@@ -0,0 +1,44 @@
+# Copyright © Mapotempo, 2021
+#
+# This file is part of Mapotempo.
+#
+# Mapotempo is free software. You can redistribute it and/or
+# modify since you respect the terms of the GNU Affero General
+# Public License as published by the Free Software Foundation,
+# either version 3 of the License, or (at your option) any later version.
+#
+# Mapotempo is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+# or FITNESS FOR A PARTICULAR PURPOSE.  See the Licenses for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with Mapotempo. If not, see:
+# <http://www.gnu.org/licenses/agpl.html>
+#
+require './test/test_helper'
+
+module Models
+  class TimewindowTest < Minitest::Test
+    def test_compatibility_between_timewindows
+      tw1 = Models::Timewindow.new(start: 10, end: 20, day_index: 0)
+      assert_raises RuntimeError do
+        tw1.compatible_with?([0, 10], false)
+      end
+      assert tw1.compatible_with?(tw1, false)
+
+      tw2 = Models::Timewindow.new(start: 10, end: 20, day_index: 1)
+      assert tw1.compatible_with?(tw2, false)
+      refute tw1.compatible_with?(tw2)
+      refute tw1.compatible_with?(tw2, true) # lateness has no impact on days incompatibility
+
+      # ignore days :
+      tw2.start = 21
+      tw2.end = 25
+      refute tw1.compatible_with?(tw2, false)
+      tw2.start = 5
+      assert tw1.compatible_with?(tw2, false)
+      tw2.end = 8
+      refute tw1.compatible_with?(tw2, false)
+    end
+  end
+end

test/wrappers/ortools_test.rb

@@ -5299,4 +5299,27 @@ def test_relations_sent_to_ortools_when_different_lapses
       end
     }
   end
+
+  def test_always_one_time_window_provided_for_rests
+    problem = VRP.basic
+    problem[:rests] = [{ id: 'rest', duration: 10 }]
+    problem[:vehicles][0][:rest_ids] = ['rest']
+
+    [[], [{ start: 2, end: 3 }]].each{ |rest_tws|
+      OptimizerWrapper.config[:services][:ortools].stub(
+        :run_ortools,
+        lambda { |pb, _, _|
+          assert_equal rest_tws.any? ? rest_tws[0][:start] : 0,
+                       pb.vehicles.first.rests.first.time_window.start
+          assert_equal rest_tws.any? ? rest_tws[0][:end] : 2**31 - 1,
+                       pb.vehicles.first.rests.first.time_window.end
+
+          'Job killed' # Return "Job killed" to stop gracefully
+        }
+      ) do
+        problem[:rests].first[:timewindows] = rest_tws
+        OptimizerWrapper.config[:services][:ortools].solve(TestHelper.create(problem), 'test')
+      end
+    }
+  end
 end