feat: Filter unused from stops in Transfers generation

Author: t2gran

application/src/main/java/org/opentripplanner/graph_builder/module/transfer/DirectTransferGenerator.java

@@ -152,7 +152,7 @@ public void buildGraph() {
 
     stops
       .stream()
-      .parallel()
+      //.parallel()
       .forEach(ts0 -> {
         /* Make transfers to each nearby stop that has lowest weight on some trip pattern.
          * Use map based on the list of edges, so that only distinct transfers are stored. */

application/src/main/java/org/opentripplanner/graph_builder/module/transfer/filter/CompositeNearbyStopFilter.java

@@ -6,6 +6,7 @@
 import java.util.List;
 import java.util.Set;
 import org.opentripplanner.routing.graphfinder.NearbyStop;
+import org.opentripplanner.transit.model.framework.FeedScopedId;
 
 class CompositeNearbyStopFilter implements NearbyStopFilter {
 
@@ -19,6 +20,16 @@ static Builder of() {
     return new Builder();
   }
 
+  @Override
+  public boolean includeFromStop(FeedScopedId id, boolean reverseDirection) {
+    for (NearbyStopFilter filter : filters) {
+      if (filter.includeFromStop(id, reverseDirection)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   @Override
   public Collection<NearbyStop> filterToStops(
     Collection<NearbyStop> nearbyStops,

application/src/main/java/org/opentripplanner/graph_builder/module/transfer/filter/FlexTripNearbyStopFilter.java

@@ -3,6 +3,7 @@
 import java.util.Collection;
 import org.opentripplanner.ext.flex.trip.FlexTrip;
 import org.opentripplanner.routing.graphfinder.NearbyStop;
+import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.service.TransitService;
 
 class FlexTripNearbyStopFilter implements NearbyStopFilter {
@@ -13,6 +14,12 @@ class FlexTripNearbyStopFilter implements NearbyStopFilter {
     this.transitService = transitService;
   }
 
+  @Override
+  public boolean includeFromStop(FeedScopedId id, boolean reverseDirection) {
+    var stop = transitService.getStopLocation(id);
+    return !transitService.getFlexIndex().getFlexTripsByStop(stop).isEmpty();
+  }
+
   @Override
   public Collection<NearbyStop> filterToStops(
     Collection<NearbyStop> nearbyStops,

application/src/main/java/org/opentripplanner/graph_builder/module/transfer/filter/NearbyStopFilter.java

@@ -2,14 +2,36 @@
 
 import java.util.Collection;
 import org.opentripplanner.routing.graphfinder.NearbyStop;
+import org.opentripplanner.transit.model.framework.FeedScopedId;
 
+/**
+ * Filters nearby stops for transfer generation based on trip patterns and flex trips.
+ * Returns only stops where boarding or alighting is possible, and for each pattern/trip,
+ * returns only the closest stop to minimize transfer generation.
+ */
 interface NearbyStopFilter {
+
+  /**
+   * Checks if a stop should be included as a transfer origin/destination.
+   *
+   * @param id the stop-location to check
+   * @param reverseDirection true for arrival searches (checks boarding), false for departure
+   *                        searches (checks alighting)
+   * @return true if the stop has relevant trip patterns or flex trips
+   */
+  boolean includeFromStop(FeedScopedId id, boolean reverseDirection);
+
   /**
-   * Find all unique nearby stops that are the closest stop on some trip pattern or flex trip. Note
-   * that the result will include the origin vertex if it is an instance of StopVertex. This is
-   * intentional: we don't want to return the next stop down the line for trip patterns that pass
-   * through the origin vertex. Taking the patterns into account reduces the number of transfers
-   * significantly compared to simple traverse-duration-constrained all-to-all stop linkage.
+   * Filters nearby stops to find optimal transfer points. For each trip pattern or flex trip,
+   * returns only the closest stop.
+   * <p>
+   * Note: The result will include the origin stop if it is a StopVertex. This is intentional - we
+   * don't want to return the next stop down the line for patterns passing through the origin.
+   *
+   * @param nearbyStops the stops to filter, typically from a street search
+   * @param reverseDirection true for arrival searches (filters by boarding), false for departure
+   *                        searches (filters by alighting)
+   * @return filtered stops, one per relevant trip pattern/flex trip
    */
   Collection<NearbyStop> filterToStops(
     Collection<NearbyStop> nearbyStops,

application/src/main/java/org/opentripplanner/graph_builder/module/transfer/filter/PatternConsideringNearbyStopFinder.java

@@ -7,6 +7,7 @@
 import org.opentripplanner.routing.api.request.RouteRequest;
 import org.opentripplanner.routing.api.request.request.StreetRequest;
 import org.opentripplanner.routing.graphfinder.NearbyStop;
+import org.opentripplanner.street.model.vertex.TransitStopVertex;
 import org.opentripplanner.street.model.vertex.Vertex;
 import org.opentripplanner.transit.service.TransitService;
 
@@ -36,6 +37,17 @@ public List<NearbyStop> findNearbyStops(
     StreetRequest streetRequest,
     boolean reverseDirection
   ) {
+    if (!(vertex instanceof TransitStopVertex stopVertex)) {
+      throw new IllegalArgumentException(
+        "Transfers can only be created between stops. Vertex: " + vertex
+      );
+    }
+
+    // Check if the from stop can be used in a transfer
+    if (!filter.includeFromStop(stopVertex.getId(), reverseDirection)) {
+      return List.of();
+    }
+
     // fetch nearby stops via the street network or using straight-line distance.
     var nearbyStops = delegateNearbyStopFinder.findNearbyStops(
       vertex,

application/src/main/java/org/opentripplanner/graph_builder/module/transfer/filter/PatternNearbyStopFilter.java

@@ -20,6 +20,13 @@ class PatternNearbyStopFilter implements NearbyStopFilter {
     this.transitService = transitService;
   }
 
+  @Override
+  public boolean includeFromStop(FeedScopedId id, boolean reverseDirection) {
+    var stop = transitService.getRegularStop(id);
+    boolean hasPatterns = !findPatternsForStop(stop, !reverseDirection).isEmpty();
+    return hasPatterns || includeStopUsedRealtime(stop);
+  }
+
   @Override
   public Collection<NearbyStop> filterToStops(
     Collection<NearbyStop> nearbyStops,

application/src/test/java/org/opentripplanner/graph_builder/module/transfer/DirectTransferGeneratorCarTest.java

@@ -91,10 +91,7 @@ public void testMultipleRequestsWithPatternsAndWithCarsAllowedPatterns() {
        S0 - S11, 100m
        S0 - S21, 100m
        S0 - S22, 200m
-      S11 - S21, 100m
-      S11 - S22, 110m
-      S12 - S22, 110m
-      S13 - S22, 210m""";
+      S12 - S22, 110m""";
     assertEquals(
       expected_walk_bike_results,
       pathToString(repository.findTransfers(StreetMode.WALK))
@@ -128,10 +125,7 @@ public void testBikeRequestWithPatternsAndWithCarsAllowedPatterns() {
        S0 - S11, 100m
        S0 - S21, 100m
        S0 - S22, 200m
-      S11 - S21, 100m
-      S11 - S22, 110m
-      S12 - S22, 110m
-      S13 - S22, 210m""",
+      S12 - S22, 110m""",
       pathToString(repository.getAllPathTransfers())
     );
   }
@@ -148,8 +142,6 @@ public void testBikeRequestWithPatternsAndWithCarsAllowedPatternsWithoutCarInTra
       """
        S0 - S11, 100m
        S0 - S21, 100m
-      S11 - S21, 100m
-      S11 - S22, 110m
       S12 - S22, 110m""",
       pathToString(repository.getAllPathTransfers())
     );
@@ -181,10 +173,7 @@ public void testDisableDefaultTransfersForMode() {
        S0 - S11, 100m
        S0 - S21, 100m
        S0 - S22, 200m
-      S11 - S21, 100m
-      S11 - S22, 110m
-      S12 - S22, 110m
-      S13 - S22, 210m""",
+      S12 - S22, 110m""",
       pathToString(repository.findTransfers(StreetMode.WALK))
     );
 
@@ -215,16 +204,12 @@ public void testMaxTransferDurationForMode() {
       """
        S0 - S11, 100m
        S0 - S21, 100m
-      S11 - S21, 100m
-      S11 - S22, 110m
       S12 - S22, 110m""",
       pathToString(repository.findTransfers(StreetMode.WALK))
     );
-    assertEquals(
-      """
-       S0 - S11, 100m
-       S0 - S21, 100m
-      S11 - S21, 100m""",
+    assertEquals("""
+      S0 - S11, 100m
+      S0 - S21, 100m""".indent(1).stripTrailing(),
       pathToString(repository.findTransfers(StreetMode.BIKE))
     );
     assertEquals("<Empty>", pathToString(repository.findTransfers(StreetMode.CAR)));

application/src/test/java/org/opentripplanner/graph_builder/module/transfer/DirectTransferGeneratorTest.drawio.png

@@ -101,23 +101,15 @@ public void testStraightLineTransfersWithPatternsPruning() {
 
     // Exactly one transfer is expected from each stop to the closest place to board all
     // patterns, including the same pattern used by the stop (E.g. S12 - S11).
+    // S11, S13, S21, S23 -> *   No patterns alight here
+    // * -> S0, S12, S13, S23    No patterns board here
     assertEquals(
       """
        S0 - S11, 1668m
        S0 - S21, 1829m
-      S11 -  S0, 1668m
-      S11 - S21, 751m
-      S12 -  S0, 3892m
       S12 - S11, 2224m
       S12 - S22, 751m
-      S13 - S11, 4448m
-      S13 - S22, 2347m
-      S21 -  S0, 1829m
-      S21 - S11, 751m
-      S22 -  S0, 3964m
-      S22 - S11, 2347m
-      S23 - S11, 4511m
-      S23 - S22, 2224m""",
+      S22 - S11, 2347m""",
       pathToString(repository.getAllPathTransfers())
     );
   }
@@ -131,17 +123,12 @@ public void testStraightLineTransfersWithBoardingRestrictions() {
       .build();
 
     assertEquals(
-      // * - S11 is not allowed, because of boarding constraints
+      // * -> S11 is not allowed, because of boarding constraints
+      // S11, S13, S21 -> *   No patterns alight here
+      // * -> S0, S12, S23    No patterns board here
       """
        S0 - S21, 1829m
-      S11 -  S0, 1668m
-      S11 - S21, 751m
-      S12 -  S0, 3892m
-      S12 - S22, 751m
-      S13 - S22, 2347m
-      S21 -  S0, 1829m
-      S22 -  S0, 3964m
-      S23 - S22, 2224m""",
+      S12 - S22, 751m""",
       pathToString(repository.getAllPathTransfers())
     );
   }
@@ -165,6 +152,7 @@ public void testStreetTransfersWithoutPatternsPruning() {
         .withTransferRequests(REQUEST_WITH_WALK_TRANSFER)
         .build();
 
+      // All transfers reachable in the street graph is included.
       assertEquals(
         """
          S0 - S11, 100m
@@ -195,13 +183,14 @@ public void testStreetTransfersWithPatterns() {
       .withTransferRequests(REQUEST_WITH_WALK_TRANSFER)
       .build();
 
+    // Best transfers between patterns; Hence S0 - S22 removed
+    // S11, S13, S21 -> *   No patterns alight here
+    // * -> S0, S12, S23    No patterns board here
     assertEquals(
       """
        S0 - S11, 100m
        S0 - S21, 100m
-      S11 - S21, 100m
-      S12 - S22, 110m
-      S13 - S22, 210m""",
+      S12 - S22, 110m""",
       pathToString(repository.getAllPathTransfers())
     );
   }
@@ -215,13 +204,15 @@ public void testStreetTransfersWithPatternsIncludeRealTimeUsedStops() {
         .withTransferRequests(REQUEST_WITH_WALK_TRANSFER)
         .build();
 
+      // Best transfers between patterns; Hence S0 - S22 removed
+      // S11, S21 -> *   No patterns alight here
+      // * -> S0, S12    No patterns board here
+      // S13, S23        Included, used real-time
       assertEquals(
         """
          S0 - S11, 100m
          S0 - S21, 100m
          S0 - S23, 300m
-        S11 - S21, 100m
-        S11 - S23, 210m
         S12 - S22, 110m
         S12 - S23, 210m
         S13 - S22, 210m
@@ -244,34 +235,46 @@ public void testStreetTransfersWithMultipleRequestsWithPatterns() {
     var bikeTransfers = repository.findTransfers(StreetMode.BIKE);
     var carTransfers = repository.findTransfers(StreetMode.CAR);
 
-    String expected =
+    // Best transfers between patterns; Hence S0 - S22 removed
+    // S11, S13, S21 -> *   No patterns alight here
+    // * -> S0, S12, S23    No patterns board here
+    String expectedWalkAndBike =
       """
        S0 - S11, 100m
        S0 - S21, 100m
-      S11 - S21, 100m
-      S12 - S22, 110m
-      S13 - S22, 210m""";
-    assertEquals(expected, pathToString(walkTransfers));
-    assertEquals(expected, pathToString(bikeTransfers));
+      S12 - S22, 110m""";
+    assertEquals(expectedWalkAndBike, pathToString(walkTransfers));
+    assertEquals(expectedWalkAndBike, pathToString(bikeTransfers));
     assertEquals("<Empty>", pathToString(carTransfers));
   }
 
   @Test
   public void testStreetTransfersWithStationWithTransfersNotAllowed() {
-    var repository = DirectTransferGeneratorTestData.of()
-      .withPatterns()
-      .withStreetGraph()
-      .withNoTransfersOnStationA()
-      .withTransferRequests(REQUEST_WITH_WALK_TRANSFER)
-      .build();
+    OTPFeature.IncludeStopsUsedRealtimeInTransfers.testOn(() -> {
+      var repository = DirectTransferGeneratorTestData.of()
+        .withPatterns()
+        .withStreetGraph()
+        .withNoTransfersOnStationA()
+        .withTransferRequests(REQUEST_WITH_WALK_TRANSFER)
+        .build();
 
-    assertEquals(
-      """
-       S0 - S22, 200m
-      S12 - S22, 110m
-      S13 - S22, 210m""",
-      pathToString(repository.getAllPathTransfers())
-    );
+      // Best transfers between patterns; Hence S0 - S22 removed
+      // S11, S13, S21 -> *   No patterns alight here
+      // * -> S0, S12, S23    No patterns board here
+      // Not:   S11, S21      Transfers NOT_ALLOWED for station
+      // Allow: S13, S23      Included, used real-time
+      assertEquals(
+        """
+         S0 - S22, 200m
+         S0 - S23, 300m
+        S12 - S22, 110m
+        S12 - S23, 210m
+        S13 - S22, 210m
+        S13 - S23, 310m
+        S22 - S23, 100m""",
+        pathToString(repository.getAllPathTransfers())
+      );
+    });
   }
 
   @Test

application/src/test/java/org/opentripplanner/graph_builder/module/transfer/DirectTransferGeneratorTest.java

@@ -160,7 +160,7 @@ public void build() {
         tripPattern(
           TripPattern.of(TimetableRepositoryForTest.id("TP0"))
             .withRoute(route("R0", TransitMode.RAIL, agency))
-            .withStopPattern(new StopPattern(List.of(st(S0), st(S_FAR_AWAY))))
+            .withStopPattern(new StopPattern(List.of(st(S_FAR_AWAY), st(S0))))
             .build()
         );
         tripPattern(