Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transfer cost limit #5516

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
)
);
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
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);
private static final int MAX_RAPTOR_TRANSFER_COST = RaptorCostConverter.toRaptorCost(
Transfer.MAX_TRANSFER_COST
);

@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 be capped to a maximum value
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 included as is in RAPTOR unchanged
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 will be capped before passing to RAPTOR
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 as is in RAPTOR
assertBelowMaxCost(raptorTransfer.get());
}
}

private static void assertMaxCost(RaptorTransfer transfer) {
assertEquals(MAX_RAPTOR_TRANSFER_COST, transfer.generalizedCost());
}

private static void assertBelowMaxCost(RaptorTransfer transfer) {
assertTrue(MAX_RAPTOR_TRANSFER_COST > transfer.generalizedCost());
}
}
Loading