Merge branch 'transfer-cost-limit' into shared-stops

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java

@@ -5,6 +5,7 @@
 import java.util.List;
 import java.util.Optional;
 import org.locationtech.jts.geom.Coordinate;
+import org.opentripplanner.framework.logging.Throttle;
 import org.opentripplanner.framework.tostring.ToStringBuilder;
 import org.opentripplanner.raptor.api.model.RaptorTransfer;
 import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter;
@@ -13,9 +14,16 @@
 import org.opentripplanner.street.search.request.StreetSearchRequest;
 import org.opentripplanner.street.search.state.EdgeTraverser;
 import org.opentripplanner.street.search.state.StateEditor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class Transfer {
 
+  private static final Logger LOG = LoggerFactory.getLogger(Transfer.class);
+  private static final Throttle THROTTLE_COST_EXCEEDED = Throttle.ofOneSecond();
+
+  protected static final int MAX_TRANSFER_COST = 2_000_000;
+
   private final int toStop;
 
   private final int distanceMeters;
@@ -63,11 +71,14 @@ public Optional<RaptorTransfer> asRaptorTransfer(StreetSearchRequest request) {
     WalkPreferences walkPreferences = request.preferences().walk();
     if (edges == null || edges.isEmpty()) {
       double durationSeconds = distanceMeters / walkPreferences.speed();
+      final double domainCost = costLimitSanityCheck(
+        durationSeconds * walkPreferences.reluctance()
+      );
       return Optional.of(
         new DefaultRaptorTransfer(
           this.toStop,
           (int) Math.ceil(durationSeconds),
-          RaptorCostConverter.toRaptorCost(durationSeconds * walkPreferences.reluctance()),
+          RaptorCostConverter.toRaptorCost(domainCost),
           this
         )
       );
@@ -82,12 +93,41 @@ public Optional<RaptorTransfer> asRaptorTransfer(StreetSearchRequest request) {
       new DefaultRaptorTransfer(
         this.toStop,
         (int) s.getElapsedTimeSeconds(),
-        RaptorCostConverter.toRaptorCost(s.getWeight()),
+        RaptorCostConverter.toRaptorCost(costLimitSanityCheck(s.getWeight())),
         this
       )
     );
   }
 
+  /**
+   * Since transfer costs are not computed through a full A* with pruning they can incur an
+   * absurdly high cost that overflows the integer cost inside RAPTOR
+   * (https://github.com/opentripplanner/OpenTripPlanner/issues/5509).
+   * <p>
+   * An example would be a transfer using lots of stairs being used on a wheelchair when no
+   * wheelchair-specific one has been generated.
+   * (see https://docs.opentripplanner.org/en/dev-2.x/Accessibility/).
+   * <p>
+   * For this reason there is this sanity limit that makes sure that the transfer cost stays below a
+   * limit that is still very high (several days of transit-equivalent cost) but far away from the
+   * integer overflow.
+   *
+   * @see EdgeTraverser
+   * @see RaptorCostConverter
+   */
+  private int costLimitSanityCheck(double cost) {
+    if (cost >= 0 && cost <= MAX_TRANSFER_COST) {
+      return (int) cost;
+    } else {
+      THROTTLE_COST_EXCEEDED.throttle(() ->
+        LOG.warn(
+          "Transfer exceeded maximum cost. Please consider changing the transfer cost calculation. More information: https://github.com/opentripplanner/OpenTripPlanner/pull/5516#issuecomment-1819138078"
+        )
+      );
+      return MAX_TRANSFER_COST;
+    }
+  }
+
   @Override
   public String toString() {
     return ToStringBuilder

src/test/java/org/opentripplanner/_support/geometry/Coordinates.java

@@ -5,6 +5,8 @@
 public class Coordinates {
 
   public static final Coordinate BERLIN = new Coordinate(52.5212, 13.4105);
+  public static final Coordinate BERLIN_BRANDENBURG_GATE = new Coordinate(52.51627, 13.37770);
   public static final Coordinate HAMBURG = new Coordinate(53.5566, 10.0003);
   public static final Coordinate KONGSBERG_PLATFORM_1 = new Coordinate(59.67216, 9.65107);
+  public static final Coordinate BOSTON = new Coordinate(42.36541, -71.06129);
 }

src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java

@@ -0,0 +1,98 @@
+package org.opentripplanner.routing.algorithm.raptoradapter.transit;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex;
+
+import java.util.List;
+import java.util.Optional;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.opentripplanner._support.geometry.Coordinates;
+import org.opentripplanner.raptor.api.model.RaptorTransfer;
+import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter;
+import org.opentripplanner.street.model._data.StreetModelForTest;
+import org.opentripplanner.street.model.vertex.IntersectionVertex;
+import org.opentripplanner.street.search.request.StreetSearchRequest;
+
+class TransferTest {
+
+  private static final IntersectionVertex BERLIN_V = intersectionVertex(Coordinates.BERLIN);
+  private static final IntersectionVertex BRANDENBURG_GATE_V = intersectionVertex(
+    Coordinates.BERLIN_BRANDENBURG_GATE
+  );
+  private static final IntersectionVertex BOSTON_V = intersectionVertex(Coordinates.BOSTON);
+
+  @Nested
+  class WithEdges {
+
+    @Test
+    void limitMaxCost() {
+      // very long edge from Berlin to Boston that has of course a huge cost to traverse
+      var edge = StreetModelForTest.streetEdge(BERLIN_V, BOSTON_V);
+
+      var veryLongTransfer = new Transfer(0, List.of(edge));
+      assertTrue(veryLongTransfer.getDistanceMeters() > 1_000_000);
+      // cost would be too high, so it should not be included in RAPTOR search
+      assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get());
+    }
+
+    @Test
+    void allowLowCost() {
+      var edge = StreetModelForTest.streetEdge(BERLIN_V, BRANDENBURG_GATE_V);
+      var transfer = new Transfer(0, List.of(edge));
+      assertTrue(transfer.getDistanceMeters() < 4000);
+      final Optional<RaptorTransfer> raptorTransfer = transfer.asRaptorTransfer(
+        StreetSearchRequest.of().build()
+      );
+      // cost is below max limit and should be included in RAPTOR
+      assertBelowMaxCost(raptorTransfer.get());
+    }
+  }
+
+  @Nested
+  class WithoutEdges {
+
+    @Test
+    void overflow() {
+      var veryLongTransfer = new Transfer(0, Integer.MAX_VALUE);
+      assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get());
+    }
+
+    @Test
+    void negativeCost() {
+      var veryLongTransfer = new Transfer(0, -5);
+      assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get());
+    }
+
+    @Test
+    void limitMaxCost() {
+      var veryLongTransfer = new Transfer(0, 8_000_000);
+      // cost would be too high, so it should not be included in RAPTOR search
+      assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get());
+    }
+
+    @Test
+    void allowLowCost() {
+      var transfer = new Transfer(0, 200);
+      final Optional<RaptorTransfer> raptorTransfer = transfer.asRaptorTransfer(
+        StreetSearchRequest.of().build()
+      );
+      // cost is below max limit and should be included in RAPTOR
+      assertBelowMaxCost(raptorTransfer.get());
+    }
+  }
+
+  private static void assertMaxCost(RaptorTransfer transfer) {
+    assertEquals(
+      RaptorCostConverter.toRaptorCost(Transfer.MAX_TRANSFER_COST),
+      transfer.generalizedCost()
+    );
+  }
+
+  private static void assertBelowMaxCost(RaptorTransfer transfer) {
+    assertTrue(
+      RaptorCostConverter.toRaptorCost(Transfer.MAX_TRANSFER_COST) > transfer.generalizedCost()
+    );
+  }
+}