fix(drt): missing insertions for prebooking (#3112)

* fix missing insertions

* fix test

contribs/drt-extensions/src/test/java/org/matsim/contrib/drt/extension/insertion/DrtInsertionExtensionIT.java

@@ -283,7 +283,7 @@ void testRangeConstraintWithCustomInstances() {
 		}
 
 		assertEquals(1470, distanceCalculator.calculatedDistances);
-		assertEquals(5288, distanceApproximator.calculatedDistances);
+		assertEquals(5280, distanceApproximator.calculatedDistances);
 	}
 
 	@Test
@@ -319,7 +319,7 @@ protected void configureQSim() {
 		}
 
 		assertEquals(1470, distanceCalculator.calculatedDistances);
-		assertEquals(5288, distanceApproximator.calculatedDistances);
+		assertEquals(5280, distanceApproximator.calculatedDistances);
 	}
 
 	static class CustomDistanceCalculator extends CustomCalculator {
@@ -464,9 +464,9 @@ void testVehicleDistanceObjective() {
 		controller.run();
 
 		assertEquals(22, handler.rejectedRequests);
-		assertEquals(2066658.0, handler.fleetDistance, 1e-3);
-		assertEquals(694149.0, handler.activeTime(), 1e-3);
-		assertEquals(280.61475, handler.meanWaitTime(), 1e-3);
+		assertEquals(2070663.0, handler.fleetDistance, 1e-3);
+		assertEquals(699076.0, handler.activeTime(), 1e-3);
+		assertEquals(279.37704, handler.meanWaitTime(), 1e-3);
 	}
 
 	@Test

contribs/drt/src/main/java/org/matsim/contrib/drt/optimizer/insertion/InsertionGenerator.java

@@ -186,7 +186,8 @@ public List<InsertionWithDetourData> generateInsertions(DrtRequest drtRequest, V
 					// task here on the same link (maybe a pickup followed by its dropoff) but much
 					// earlier. In that case it is actually a valid insertion.
 
-					if (drtRequest.getEarliestStartTime() < nextStop.getArrivalTime()) {
+					boolean viableSameLink = vEntry.getPrecedingStayTime(i) > 0.0;
+					if (viableSameLink && drtRequest.getEarliestStartTime() < nextStop.getArrivalTime()) {
 						// the new request wants to depart before the start of the next stop, which may
 						// be a viable insertion. Note that if the requested wanted to depart after the
 						// start of the next stop, but before its end, this is a special case that is
@@ -235,6 +236,16 @@ private void generateDropoffInsertions(DrtRequest request, VehicleEntry vEntry,
 				addInsertion(insertions,
 						createInsertionWithDetourData(request, vEntry, pickupInsertion, fromPickupTT, pickupDetourInfo,
 								j));
+			} else {
+				// special case: inserting dropoff before prebooked task on the same link
+				// see the reasoning in generateInsertions
+
+				boolean viableSameLink = vEntry.getPrecedingStayTime(j) > 0.0;
+				if (viableSameLink && earliestPickupStartTime + fromPickupTT < nextStop(vEntry, j).getArrivalTime()) {
+					addInsertion(insertions,
+							createInsertionWithDetourData(request, vEntry, pickupInsertion, fromPickupTT, pickupDetourInfo,
+									j));
+				}
 			}
 		}
 
@@ -274,6 +285,16 @@ private void generateDropoffInsertions(DrtRequest request, VehicleEntry vEntry,
 				addInsertion(insertions,
 						createInsertionWithDetourData(request, vEntry, pickupInsertion, fromPickupTT, pickupDetourInfo,
 								j));
+			} else {
+				// special case: inserting dropoff before prebooked task on the same link
+				// see the reasoning in generateInsertions
+
+				boolean viableSameLink = vEntry.getPrecedingStayTime(j) > 0.0;
+				if (viableSameLink && earliestPickupStartTime + fromPickupTT < nextStop(vEntry, j).getArrivalTime()) {
+					addInsertion(insertions,
+							createInsertionWithDetourData(request, vEntry, pickupInsertion, fromPickupTT, pickupDetourInfo,
+									j));
+				}
 			}
 		}
 

contribs/drt/src/test/java/org/matsim/contrib/drt/optimizer/insertion/InsertionGeneratorTest.java

@@ -378,7 +378,8 @@ void startEmpty_onlineRequest_beforeAlreadyPrebookedOtherRequest() {
 		Waypoint.Start start = new Waypoint.Start(null, link("start"), 0, 0);
 		Waypoint.Stop stop0 = stop(200, fromLink, 1);
 		Waypoint.Stop stop1 = stop(400, link("stop"), 0);
-		VehicleEntry entry = entry(start, stop0, stop1);
+		List<Double> precedingStayTimes = Arrays.asList(100.0, 0.0);
+		VehicleEntry entry = entry(start, precedingStayTimes, stop0, stop1);
 		assertInsertionsOnly(drtRequest, entry,
 			new Insertion(drtRequest, entry, 0, 0),
 			new Insertion(drtRequest, entry, 0, 1),
@@ -524,9 +525,13 @@ private Waypoint.Stop stop(double beginTime, Link link, int outgoingOccupancy) {
 	}
 
 	private VehicleEntry entry(Waypoint.Start start, Waypoint.Stop... stops) {
+		List<Double> precedingStayTimes = Collections.nCopies(stops.length, 0.0);
+		return entry(start, precedingStayTimes, stops);
+	}
+
+	private VehicleEntry entry(Waypoint.Start start, List<Double> precedingStayTimes, Waypoint.Stop... stops) {
 		var slackTimes = new double[stops.length + 2];
 		Arrays.fill(slackTimes, Double.POSITIVE_INFINITY);
-		List<Double> precedingStayTimes = Collections.nCopies(stops.length, 0.0);
 		return new VehicleEntry(vehicle, start, ImmutableList.copyOf(stops), slackTimes, precedingStayTimes, 0);
 	}
 }

contribs/drt/src/test/java/org/matsim/contrib/drt/prebooking/PrebookingTest.java

@@ -528,4 +528,90 @@ public void install() {
 		assertEquals(2060.0, taskInfo.get(3).endTime, 1e-3); // Ending stop (260s duration)
 		assertEquals(2060.0, taskInfo.get(4).startTime, 1e-3); // Starting drive (ending stop)
 	}
+
+	@Test
+	void destinationEqualsPrebookedOrigin_twoRequests() {
+		/*-
+		 * In this test, we have two prebooked requests:
+		 * P[A] ---------> D[A]     P[B] --------> D[B]
+		 * 
+		 * The dropoff of A happens at the same place as the pickup of B. Then we dispatch a new request C 
+		 * traveling the same trip as A. Without an implemented fix, inserting the dropfof between D[A] and P[B] 
+		 * was not an option as P[B] was the same link as the destination of C. The only viable dropoff insertion 
+		 * was after P[B], which, however, was too late.
+		 */
+
+		PrebookingTestEnvironment environment = new PrebookingTestEnvironment(utils) //
+				.addVehicle("vehicleA", 1, 1) //
+				.addRequest("requestA", 1, 1, 4, 4, 1000.0, 0.0) //
+				.addRequest("requestB", 4, 4, 8, 8, 8000.0, 1.0) //
+				.addRequest("requestC", 1, 1, 4, 4, 1000.0, 2.0) //
+				.configure(300.0, 2.0, 1800.0, 60.0) //
+				.endTime(12.0 * 3600.0);
+
+		Controler controller = environment.build();
+		installPrebooking(controller);
+		controller.run();
+
+		{
+			RequestInfo requestInfo = environment.getRequestInfo().get("requestA");
+			assertEquals(0.0, requestInfo.submissionTime, 1e-3);
+			assertEquals(1000.0 + 60.0 + 1.0, requestInfo.pickupTime, 1e-3);
+			assertEquals(1188.0, requestInfo.dropoffTime, 1e-3);
+		}
+
+		{
+			RequestInfo requestInfo = environment.getRequestInfo().get("requestB");
+			assertEquals(1.0, requestInfo.submissionTime, 1e-3);
+			assertEquals(8000.0 + 60.0 + 1.0, requestInfo.pickupTime, 1e-3);
+			assertEquals(8230.0, requestInfo.dropoffTime, 1e-3);
+		}
+
+		{
+			RequestInfo requestInfo = environment.getRequestInfo().get("requestC");
+			assertEquals(2.0, requestInfo.submissionTime, 1e-3);
+			assertEquals(1000.0 + 60.0 + 1.0, requestInfo.pickupTime, 1e-3);
+			assertEquals(1188.0, requestInfo.dropoffTime, 1e-3);
+		}
+
+		assertEquals(4, environment.getTaskInfo().get("vehicleA").stream().filter(t -> t.type.equals("STOP")).count());
+	}
+
+	@Test
+	void destinationEqualsPrebookedOrigin_oneRequest() {
+		/*-
+		 * In this test, we aprebooked requests:
+		 * P[A] ---------> D[A]
+		 * 
+		 * Then we dispatch a new request C before A. The destination of C is the origin of A. Without an implemented fix, 
+		 * inserting the dropoff before P[A] was not allowed as it is the same link, but inserting after D[A] was too late.
+		 */
+
+		PrebookingTestEnvironment environment = new PrebookingTestEnvironment(utils) //
+				.addVehicle("vehicleA", 1, 1) //
+				.addRequest("requestA", 4, 4, 8, 8, 4000.0, 1.0) //
+				.addRequest("requestB", 1, 1, 4, 4, 1000.0, 2.0) //
+				.configure(300.0, 2.0, 1800.0, 60.0) //
+				.endTime(12.0 * 3600.0);
+
+		Controler controller = environment.build();
+		installPrebooking(controller);
+		controller.run();
+
+		{
+			RequestInfo requestInfo = environment.getRequestInfo().get("requestA");
+			assertEquals(1.0, requestInfo.submissionTime, 1e-3);
+			assertEquals(4000.0 + 60.0 + 1.0, requestInfo.pickupTime, 1e-3);
+			assertEquals(4230.0, requestInfo.dropoffTime, 1e-3);
+		}
+
+		{
+			RequestInfo requestInfo = environment.getRequestInfo().get("requestB");
+			assertEquals(2.0, requestInfo.submissionTime, 1e-3);
+			assertEquals(1000.0 + 60.0 + 1.0, requestInfo.pickupTime, 1e-3);
+			assertEquals(1188.0, requestInfo.dropoffTime, 1e-3);
+		}
+
+		assertEquals(4, environment.getTaskInfo().get("vehicleA").stream().filter(t -> t.type.equals("STOP")).count());
+	}
 }

contribs/drt/src/test/java/org/matsim/contrib/drt/run/examples/RunDrtExampleIT.java

@@ -304,16 +304,12 @@ public void install() {
 
 		controller.run();
 
-		// sh, 11/08/2023: updated after introducing prebookg, basically we generate a
-		// new feasible insertion (see InsertionGenerator) that previously did not
-		// exist, but has the same cost (pickup loss + drop-off loss) as the original
-		// one
 		var expectedStats = Stats.newBuilder()
 				.rejectionRate(0.04)
 				.rejections(16)
-				.waitAverage(278.76)
-				.inVehicleTravelTimeMean(384.93)
-				.totalTravelTimeMean(663.68)
+				.waitAverage(278.92)
+				.inVehicleTravelTimeMean(384.6)
+				.totalTravelTimeMean(663.52)
 				.build();
 
 		verifyDrtCustomerStatsCloseToExpectedStats(utils.getOutputDirectory(), expectedStats);
@@ -350,9 +346,9 @@ void testRunDrtWithPrebooking() {
 		var expectedStats = Stats.newBuilder()
 				.rejectionRate(0.04)
 				.rejections(14)
-				.waitAverage(232.45)
-				.inVehicleTravelTimeMean(388.99)
-				.totalTravelTimeMean(621.44)
+				.waitAverage(232.47)
+				.inVehicleTravelTimeMean(389.16)
+				.totalTravelTimeMean(621.63)
 				.build();
 
 		verifyDrtCustomerStatsCloseToExpectedStats(utils.getOutputDirectory(), expectedStats);