Skip to content

Commit

Permalink
Merge pull request opentripplanner#5605 from opentripplanner/max-coun…
Browse files Browse the repository at this point in the history
…t-rental

Fix high walk reluctance leading to zero egress results for rental searches
  • Loading branch information
leonardehrenfried authored Jan 11, 2024
2 parents bdb5355 + 8a67be4 commit 75462f0
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 50 deletions.
1 change: 0 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@
--add-opens java.base/jdk.internal.loader=ALL-UNNAMED
--add-opens java.base/jdk.internal.ref=ALL-UNNAMED
--add-opens java.base/jdk.internal.util=ALL-UNNAMED
--add-opens java.base/jdk.internal.util.jar=ALL-UNNAMED
--add-opens java.base/jdk.internal.module=ALL-UNNAMED
--add-opens java.base/sun.net.www.protocol.http=ALL-UNNAMED
--add-opens java.base/sun.net.www.protocol.jar=ALL-UNNAMED
Expand Down
12 changes: 0 additions & 12 deletions src/main/java/org/opentripplanner/astar/model/BinHeap.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,6 @@ public void rekey(T e, double p) {
prio[i] = p;
}

public void dump() {
for (int i = 0; i <= capacity; i++) {
String topMarker = (i > size) ? "(UNUSED)" : "";
System.out.printf("%d\t%f\t%s\t%s\n", i, prio[i], elem[i], topMarker);
}
System.out.printf("-----------------------\n");
}

public void reset() {
// empties the queue in one operation
size = 0;
Expand Down Expand Up @@ -135,8 +127,4 @@ public void resize(int capacity) {
prio = Arrays.copyOf(prio, capacity + 1);
elem = Arrays.copyOf(elem, capacity + 1);
}

public int getCapacity() {
return capacity;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,6 @@ public void setAborted() {
aborted = true;
}

public boolean isAborted() {
return aborted;
}

public String toString() {
return "ShortestPathTree(" + this.stateSets.size() + " vertices)";
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
package org.opentripplanner.astar.strategy;

import java.util.function.Function;
import java.util.function.Predicate;
import org.opentripplanner.astar.spi.AStarEdge;
import org.opentripplanner.astar.spi.AStarState;
import org.opentripplanner.astar.spi.SkipEdgeStrategy;

/**
* Skips edges when the specified number of desired vertices have been visited
* Skips edges when the specified number of desired vertices have been visited.
*/
public class MaxCountSkipEdgeStrategy<
State extends AStarState<State, Edge, ?>, Edge extends AStarEdge<State, Edge, ?>
>
implements SkipEdgeStrategy<State, Edge> {

private final int maxCount;
private final Function<State, Boolean> shouldIncreaseCount;
private final Predicate<State> shouldIncreaseCount;

private int visited;

public MaxCountSkipEdgeStrategy(int count, Function<State, Boolean> shouldIncreaseCount) {
public MaxCountSkipEdgeStrategy(int count, Predicate<State> shouldIncreaseCount) {
this.maxCount = count;
this.shouldIncreaseCount = shouldIncreaseCount;
this.visited = 0;
}

@Override
public boolean shouldSkipEdge(State current, Edge edge) {
if (this.shouldIncreaseCount.apply(current)) {
if (shouldIncreaseCount.test(current)) {
visited++;
}
return visited > maxCount;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,8 @@
import org.opentripplanner.transit.service.TransitService;

/**
* These library functions are used by the streetless and streetful stop linkers, and in profile
* transfer generation.
* TODO OTP2 Fold these into org.opentripplanner.routing.graphfinder.StreetGraphFinder
* These are not library functions, this is instantiated as an object. Define lifecycle of the object (reuse?).
* Because AStar instances should only be used once, NearbyStopFinder should only be used once.
* Ideally they could also be used in long distance mode and profile routing for the street segments.
* For each stop, it finds the closest stops on all other patterns. This reduces the number of transfer edges
* significantly compared to simple radius-constrained all-to-all stop linkage.
* This class contains code for finding nearby stops from a given vertex. It is being used by access
* and egress searches as well as transfer generation.
*/
public class NearbyStopFinder {

Expand Down Expand Up @@ -100,6 +94,8 @@ public NearbyStopFinder(
* 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.
*/
public Set<NearbyStop> findNearbyStopsConsideringPatterns(
Vertex vertex,
Expand Down Expand Up @@ -227,15 +223,12 @@ public List<NearbyStop> findNearbyStopsViaStreets(
for (State state : spt.getAllStates()) {
Vertex targetVertex = state.getVertex();
if (originVertices.contains(targetVertex)) continue;
if (targetVertex instanceof TransitStopVertex && state.isFinal()) {
stopsFound.add(
NearbyStop.nearbyStopForState(state, ((TransitStopVertex) targetVertex).getStop())
);
if (targetVertex instanceof TransitStopVertex tsv && state.isFinal()) {
stopsFound.add(NearbyStop.nearbyStopForState(state, tsv.getStop()));
}
if (
OTPFeature.FlexRouting.isOn() &&
targetVertex instanceof StreetVertex &&
!((StreetVertex) targetVertex).areaStops().isEmpty()
targetVertex instanceof StreetVertex streetVertex && !streetVertex.areaStops().isEmpty()
) {
for (AreaStop areaStop : ((StreetVertex) targetVertex).areaStops()) {
// This is for a simplification, so that we only return one vertex from each
Expand Down Expand Up @@ -314,7 +307,7 @@ private SkipEdgeStrategy<State, Edge> getSkipEdgeStrategy(
if (maxStopCount > 0) {
var strategy = new MaxCountSkipEdgeStrategy<>(
maxStopCount,
NearbyStopFinder::isTransitVertex
NearbyStopFinder::hasReachedStop
);
return new ComposingSkipEdgeStrategy<>(strategy, durationSkipEdgeStrategy);
}
Expand Down Expand Up @@ -360,15 +353,23 @@ private boolean canBoardFlex(State state, boolean reverse) {

return edges
.stream()
.anyMatch(e ->
e instanceof StreetEdge && ((StreetEdge) e).getPermission().allows(TraverseMode.CAR)
);
.anyMatch(e -> e instanceof StreetEdge se && se.getPermission().allows(TraverseMode.CAR));
}

/**
* Checks if the {@code state} as at a transit vertex.
* Checks if the {@code state} is at a transit vertex and if it's final, which means that the state
* can actually board a vehicle.
* <p>
* This is important because there can be cases where states that cannot actually board the vehicle
* can dominate those that can thereby leading to zero found stops when this predicate is used with
* the {@link MaxCountSkipEdgeStrategy}.
* <p>
* An example of this would be an egress/reverse search with a very high walk reluctance where
* the states that speculatively rent a vehicle move the walk states down the A* priority queue
* until the required number of stops are reached to abort the search, leading to zero egress
* results.
*/
public static boolean isTransitVertex(State state) {
return state.getVertex() instanceof TransitStopVertex;
public static boolean hasReachedStop(State state) {
return state.getVertex() instanceof TransitStopVertex && state.isFinal();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ protected void prepareInitialStates(Collection<State> initialStates) {
@Override
protected void initializeHeuristic(
RemainingWeightHeuristic<State> heuristic,
Set<Vertex> origin,
Set<Vertex> ignored,
Set<Vertex> destination,
boolean arriveBy
) {
Expand All @@ -111,7 +111,6 @@ protected void initializeHeuristic(
} else if (heuristic instanceof EuclideanRemainingWeightHeuristic euclideanHeuristic) {
euclideanHeuristic.initialize(
streetRequest.mode(),
origin,
destination,
arriveBy,
routeRequest.preferences()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public class EuclideanRemainingWeightHeuristic implements RemainingWeightHeurist
// not work correctly.
public void initialize(
StreetMode streetMode,
Set<Vertex> fromVertices,
Set<Vertex> toVertices,
boolean arriveBy,
RoutingPreferences preferences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,25 @@ class MaxCountSkipEdgeStrategyTest {
@Test
void countStops() {
var state = TestStateBuilder.ofWalking().stop().build();
var strategy = new MaxCountSkipEdgeStrategy<>(1, NearbyStopFinder::isTransitVertex);
var strategy = new MaxCountSkipEdgeStrategy<>(1, NearbyStopFinder::hasReachedStop);
assertFalse(strategy.shouldSkipEdge(state, null));
assertTrue(strategy.shouldSkipEdge(state, null));
}

@Test
void doNotCountStop() {
var state = TestStateBuilder.ofWalking().build();
var strategy = new MaxCountSkipEdgeStrategy<>(1, NearbyStopFinder::isTransitVertex);
var strategy = new MaxCountSkipEdgeStrategy<>(1, NearbyStopFinder::hasReachedStop);
assertFalse(strategy.shouldSkipEdge(state, null));
assertFalse(strategy.shouldSkipEdge(state, null));
assertFalse(strategy.shouldSkipEdge(state, null));
}

@Test
void nonFinalState() {
var state = TestStateBuilder.ofScooterRentalArriveBy().stop().build();
assertFalse(state.isFinal());
var strategy = new MaxCountSkipEdgeStrategy<>(1, NearbyStopFinder::hasReachedStop);
assertFalse(strategy.shouldSkipEdge(state, null));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.street.search.state;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.opentripplanner.routing.algorithm.raptoradapter.router.street.AccessEgressType.EGRESS;
import static org.opentripplanner.transit.model.site.PathwayMode.WALKWAY;

import java.time.Instant;
Expand All @@ -10,6 +11,7 @@
import javax.annotation.Nonnull;
import org.opentripplanner.framework.i18n.I18NString;
import org.opentripplanner.framework.i18n.NonLocalizedString;
import org.opentripplanner.routing.algorithm.raptoradapter.router.street.AccessEgressType;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.service.vehiclerental.model.TestFreeFloatingRentalVehicleBuilder;
import org.opentripplanner.service.vehiclerental.model.TestVehicleRentalStationBuilder;
Expand All @@ -20,6 +22,7 @@
import org.opentripplanner.street.model.RentalFormFactor;
import org.opentripplanner.street.model.StreetTraversalPermission;
import org.opentripplanner.street.model._data.StreetModelForTest;
import org.opentripplanner.street.model.edge.Edge;
import org.opentripplanner.street.model.edge.ElevatorAlightEdge;
import org.opentripplanner.street.model.edge.ElevatorBoardEdge;
import org.opentripplanner.street.model.edge.ElevatorHopEdge;
Expand Down Expand Up @@ -51,10 +54,19 @@ public class TestStateBuilder {
private State currentState;

private TestStateBuilder(StreetMode mode) {
this(mode, AccessEgressType.ACCESS);
}

private TestStateBuilder(StreetMode mode, AccessEgressType type) {
currentState =
new State(
StreetModelForTest.intersectionVertex(count, count),
StreetSearchRequest.of().withMode(mode).withStartTime(DEFAULT_START_TIME).build()
StreetSearchRequest
.of()
.withArriveBy(type.isEgress())
.withMode(mode)
.withStartTime(DEFAULT_START_TIME)
.build()
);
}

Expand All @@ -80,6 +92,14 @@ public static TestStateBuilder ofScooterRental() {
return new TestStateBuilder(StreetMode.SCOOTER_RENTAL);
}

/**
* Creates a state that starts the scooter rental in arriveBy mode, so starting with
* a rental scooter and going backwards until it finds a rental vertex where to drop it.
*/
public static TestStateBuilder ofScooterRentalArriveBy() {
return new TestStateBuilder(StreetMode.SCOOTER_RENTAL, EGRESS);
}

public static TestStateBuilder ofBikeRental() {
return new TestStateBuilder(StreetMode.BIKE_RENTAL);
}
Expand Down Expand Up @@ -248,7 +268,12 @@ private TestStateBuilder arriveAtStop(RegularStop stop) {
var from = (StreetVertex) currentState.vertex;
var to = new TransitStopVertexBuilder().withStop(stop).build();

var edge = StreetTransitStopLink.createStreetTransitStopLink(from, to);
Edge edge;
if (currentState.getRequest().arriveBy()) {
edge = StreetTransitStopLink.createStreetTransitStopLink(to, from);
} else {
edge = StreetTransitStopLink.createStreetTransitStopLink(from, to);
}
var states = edge.traverse(currentState);
if (states.length != 1) {
throw new IllegalStateException("Only single state transitions are supported.");
Expand Down

0 comments on commit 75462f0

Please sign in to comment.