From 073f17b0d3354fd2a7163a921d71445b88ab8c8c Mon Sep 17 00:00:00 2001
From: halilsen <halil.sen@mapotempo.com>
Date: Fri, 28 Jan 2022 11:36:42 +0100
Subject: [PATCH] Split duration among partitions correctly

---
 CHANGELOG.md                                  |  1 +
 lib/interpreters/split_clustering.rb          | 11 +++++++-
 .../lib/interpreters/split_clustering_test.rb | 25 +++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5a1132fee..780499c3c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@
 
 ### Fixed
 
+- 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)
 
 ## [v1.8.2] - 2022-01-19
diff --git a/lib/interpreters/split_clustering.rb b/lib/interpreters/split_clustering.rb
index a6f93a9dd..185433efd 100644
--- a/lib/interpreters/split_clustering.rb
+++ b/lib/interpreters/split_clustering.rb
@@ -123,9 +123,18 @@ def self.generate_split_vrps(service_vrp, _job = nil, block = nil)
           }
           current_service_vrps = generated_service_vrps.flatten
         else
-          raise OptimizerWrapper::UnsupportedProblemError, "Unknown partition method #{partition[:technique]}"
+          raise OptimizerWrapper::UnsupportedProblemError.new("Unknown partition technique #{partition[:technique]}")
         end
       }
+
+      total_size = current_service_vrps.sum{ |s_vrp| s_vrp[:vrp].services.size * [1, s_vrp[:vrp].vehicles.size].min }
+      current_service_vrps.each{ |s_vrp|
+        # If one sub vrp has no vehicle or no service, duration can be zero.
+        # We only split duration among sub_service_vrps that have at least one vehicle and one service.
+        this_sub_size = s_vrp[:vrp].services.size * [1, s_vrp[:vrp].vehicles.size].min
+        adjust_independent_duration(s_vrp[:vrp], this_sub_size, total_size)
+      }
+
       current_service_vrps
     end
 
diff --git a/test/lib/interpreters/split_clustering_test.rb b/test/lib/interpreters/split_clustering_test.rb
index fe10789b8..a049e61cf 100644
--- a/test/lib/interpreters/split_clustering_test.rb
+++ b/test/lib/interpreters/split_clustering_test.rb
@@ -269,6 +269,31 @@ def test_work_day_without_vehicle_entity_small
       assert_equal 2, generated_services_vrps.size
     end
 
+    def test_partitioning_splits_resolution_duration
+      vrp = VRP.lat_lon_periodic_two_vehicles
+      vrp[:configuration][:preprocessing][:partitions] = [TestHelper.vehicle_and_days_partitions.first]
+      vrp[:configuration][:preprocessing][:partitions][0][:restarts] = 1
+
+      service_vrp = { vrp: TestHelper.create(vrp), service: :demo }
+      generated_services_vrps = Interpreters::SplitClustering.generate_split_vrps(service_vrp)
+      generated_services_vrps.flatten!
+      generated_services_vrps.compact!
+
+      assert_equal vrp[:vehicles].size, generated_services_vrps.size
+
+      assert_in_epsilon vrp[:configuration][:resolution][:duration],
+                        generated_services_vrps.sum{ |sv| sv[:vrp].configuration.resolution.duration },
+                        vrp[:vehicles].size # due to rounding we might have overshoot the duration
+
+      s_ratio = generated_services_vrps.map{ |sv| sv[:vrp].services.size }.inject{ |ratio, service_count|
+        ratio / service_count.to_f
+      }
+      d_ratio = generated_services_vrps.map{ |sv| sv[:vrp].configuration.resolution.duration }.inject{ |ratio, duration|
+        ratio / duration.to_f
+      }
+      assert_in_epsilon s_ratio, d_ratio, 0.05 # 5% diff can be expected due to rounding in extreme cases
+    end
+
     def test_work_day_without_vehicle_entity
       vrp = VRP.lat_lon_periodic_two_vehicles
       vrp[:configuration][:preprocessing][:partitions] = TestHelper.vehicle_and_days_partitions