fix: SearchWindow for arriveBy search is of by one

In the current version the dynamic search-window is set 1 minute too early to include a trip if
both the minimum travel time can be calculated exact, and the journey arrival-time is withing 59s
of the LAT.

src/main/java/org/opentripplanner/raptor/rangeraptor/transit/RaptorSearchWindowCalculator.java

@@ -82,23 +82,22 @@ public RaptorSearchWindowCalculator calculate() {
  }
 
  // TravelWindow is the time from the earliest-departure-time to the latest-arrival-time
- int travelWindow = searchWindowSeconds + roundUpToNearestMinute(heuristicMinTransitTime);
+ int travelWindow = searchWindowSeconds + roundDownToNearestMinute(heuristicMinTransitTime);
 
  if (!params.isEarliestDepartureTimeSet()) {
  earliestDepartureTime = latestArrivalTime - travelWindow;
  }
  return this;
  }
 
- int roundUpToNearestMinute(int minTravelTimeInSeconds) {
+ int roundDownToNearestMinute(int minTravelTimeInSeconds) {
  if (minTravelTimeInSeconds < 0) {
  throw new IllegalArgumentException(
  "This operation is not defined for negative numbers: " + minTravelTimeInSeconds
  );
  }
- // See the UnitTest for verification of this:
- // We want: 0 -> 0, 1 -> 60, 59 -> 60 ...
- return ((minTravelTimeInSeconds + 59) / 60) * 60;
+ // We want: 0 -> 0, 59 -> 0, 60 -> 60 ...
+ return (minTravelTimeInSeconds / 60) * 60;
  }
 
  /**

src/test/java/org/opentripplanner/raptor/rangeraptor/transit/RaptorSearchWindowCalculatorTest.java

@@ -2,9 +2,13 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.time.Duration;
+import java.util.List;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.opentripplanner.raptor._data.transit.TestTripSchedule;
 import org.opentripplanner.raptor.api.model.RaptorConstants;
 import org.opentripplanner.raptor.api.request.DynamicSearchWindowCoefficients;
@@ -58,19 +62,48 @@ public void calcEarliestDeparture() {
  subject.withHeuristics(minTransitTime, minWaitTime).withSearchParams(searchParams).calculate();
 
  /*
- search-window: round_N(C + T * minTransitTime + W * minWaitTime)
+ search-window: round_60(C + T * minTransitTime + W * minWaitTime)
  = round_60(600 + 0.6 * 500 + 0.4 * 200)
  = round_60(980) = 960
 
  EDT = LAT - (search-window + minTripTime)
- EDT = 3000 - (960s + roundUp_60(500))
- EDT = 3000 - (960s + 540s)
- EDT = 1500
+ EDT = 3000 - (960s + round_60(500))
+ EDT = 3000 - (960s + 480s)
+ EDT = 1560
  */
  assertEquals(960, subject.getSearchWindowSeconds());
- assertEquals(1_500, subject.getEarliestDepartureTime());
+ assertEquals(1_560, subject.getEarliestDepartureTime());
  // Given - verify not changed
  assertEquals(3_000, subject.getLatestArrivalTime());
+ assertTrue(
+ minTransitTime >
+ subject.getLatestArrivalTime() -
+ (subject.getSearchWindowSeconds() + subject.getEarliestDepartureTime())
+ );
+ }
+
+ @Test
+ public void calcEarliestDepartureExact() {
+ SearchParams searchParams = new RaptorRequestBuilder<TestTripSchedule>()
+ .searchParams()
+ .latestArrivalTime(3_000)
+ .buildSearchParam();
+
+ int minTransitTime = 600;
+ int minWaitTime = 0;
+
+ subject.withHeuristics(minTransitTime, minWaitTime).withSearchParams(searchParams).calculate();
+
+ assertEquals(960, subject.getSearchWindowSeconds());
+ assertEquals(1_440, subject.getEarliestDepartureTime());
+ // Given - verify not changed
+ assertEquals(3_000, subject.getLatestArrivalTime());
+
+ assertEquals(
+ minTransitTime,
+ subject.getLatestArrivalTime() -
+ (subject.getSearchWindowSeconds() + subject.getEarliestDepartureTime())
+ );
  }
 
  @Test
@@ -166,17 +199,23 @@ public void calculateNotDefinedIfMinTravelTimeNotSet() {
  assertThrows(IllegalArgumentException.class, subject::calculate);
  }
 
- @Test
- public void roundUpToNearestMinute() {
- assertEquals(0, subject.roundUpToNearestMinute(0));
- assertEquals(60, subject.roundUpToNearestMinute(1));
- assertEquals(60, subject.roundUpToNearestMinute(60));
- assertEquals(120, subject.roundUpToNearestMinute(61));
+ static List<V2> roundUpToNearestMinuteTestCase() {
+ return List.of(V2.of(0, 0), V2.of(0, 59), V2.of(60, 60), V2.of(60, 119), V2.of(120, 120));
+ }
+
+ @ParameterizedTest
+ @MethodSource("roundUpToNearestMinuteTestCase")
+ void roundUpToNearestMinute(V2 v2) {
+ assertEquals(v2.expected(), subject.roundDownToNearestMinute(v2.value()));
  }
 
  @Test
- public void roundUpToNearestMinuteNotDefinedForNegativeNumbers() {
- assertThrows(IllegalArgumentException.class, () -> subject.roundUpToNearestMinute(-1));
+ void roundUpToNearestMinuteNotDefinedForNegativeNumbers() {
+ var ex = assertThrows(
+ IllegalArgumentException.class,
+ () -> subject.roundDownToNearestMinute(-1)
+ );
+ assertEquals("This operation is not defined for negative numbers: -1", ex.getMessage());
  }
 
  @Test
@@ -188,4 +227,10 @@ public void roundStep() {
  assertEquals(60, subject.roundStep(30f));
  assertEquals(480, subject.roundStep(450f));
  }
+
+ record V2(int expected, int value) {
+ static V2 of(int expected, int value) {
+ return new V2(expected, value);
+ }
+ }
 }