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.

Sign up for GitHub

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

Revert allow multiple states during transfer edge traversals #6357

Open
wants to merge 2 commits into
base: dev-2.x
Choose a base branch
from
Open

Revert allow multiple states during transfer edge traversals #6357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb81049
Revert "Street search: traverse edges which may return multiple states"
vpaturet Jan 3, 2025
868a8db
Fix merge conflict
vpaturet Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ private FlexAccessEgress createFlexAccessEgress(
return null;
}

final var finalStateOpt = EdgeTraverser.traverseEdges(afterFlexState, transferEdges);
final var finalStateOpt = EdgeTraverser.traverseEdges(afterFlexState[0], transferEdges);

return finalStateOpt
.map(finalState -> {
Expand Down
2 changes: 1 addition & 1 deletion application/src/ext/java/org/opentripplanner/ext/flex/template/FlexDirectPathFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ private Optional<DirectFlexPath> createDirectGraphPath(

final State[] afterFlexState = flexEdge.traverse(accessNearbyStop.state);

var finalStateOpt = EdgeTraverser.traverseEdges(afterFlexState, egress.edges);
var finalStateOpt = EdgeTraverser.traverseEdges(afterFlexState[0], egress.edges);

if (finalStateOpt.isEmpty()) {
return Optional.empty();
Expand Down
30 changes: 19 additions & 11 deletions .../main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import org.opentripplanner.astar.model.GraphPath;
import org.opentripplanner.framework.application.OTPFeature;
import org.opentripplanner.framework.geometry.GeometryUtils;
Expand Down Expand Up @@ -39,8 +38,8 @@
import org.opentripplanner.street.search.TraverseMode;
import org.opentripplanner.street.search.request.StreetSearchRequest;
import org.opentripplanner.street.search.request.StreetSearchRequestMapper;
import org.opentripplanner.street.search.state.EdgeTraverser;
import org.opentripplanner.street.search.state.State;
import org.opentripplanner.street.search.state.StateEditor;
import org.opentripplanner.transit.model.timetable.TripIdAndServiceDate;
import org.opentripplanner.transit.model.timetable.TripOnServiceDate;
import org.opentripplanner.transit.service.TransitService;
Expand Down Expand Up @@ -361,15 +360,24 @@ private List<Leg> mapNonTransitLeg(
.build()
);
} else {
var legTransferSearchRequest = transferStreetRequest
.copyOf(createZonedDateTime(pathLeg.fromTime()).toInstant())
.build();
var initialStates = State.getInitialStates(
Set.of(edges.getFirst().getFromVertex()),
legTransferSearchRequest
);
var state = EdgeTraverser.traverseEdges(initialStates, edges);
var graphPath = new GraphPath<>(state.get());
StateEditor se = new StateEditor(edges.get(0).getFromVertex(), transferStreetRequest);
se.setTimeSeconds(createZonedDateTime(pathLeg.fromTime()).toEpochSecond());

State s = se.makeState();
ArrayList<State> transferStates = new ArrayList<>();
transferStates.add(s);
for (Edge e : edges) {
var states = e.traverse(s);
if (State.isEmpty(states)) {
s = null;
} else {
transferStates.add(states[0]);
s = states[0];
}
Comment on lines +370 to +376
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can revert this for the performance benefit. It may be useful to add a check here to avoid problems related to A* searches returning multiple states similar to EdgeTraverser.

}

State[] states = transferStates.toArray(new State[0]);
var graphPath = new GraphPath<>(states[states.length - 1]);

Itinerary subItinerary = graphPathToItineraryMapper.generateItinerary(graphPath);

Expand Down
8 changes: 5 additions & 3 deletions ...n/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.opentripplanner.street.model.edge.Edge;
import org.opentripplanner.street.search.request.StreetSearchRequest;
import org.opentripplanner.street.search.state.EdgeTraverser;
import org.opentripplanner.street.search.state.State;
import org.opentripplanner.street.search.state.StateEditor;
import org.opentripplanner.utils.logging.Throttle;
import org.opentripplanner.utils.tostring.ToStringBuilder;
import org.slf4j.Logger;
Expand Down Expand Up @@ -97,8 +97,10 @@ public Optional<RaptorTransfer> asRaptorTransfer(StreetSearchRequest request) {
);
}

var initialStates = State.getInitialStates(Set.of(edges.getFirst().getFromVertex()), request);
var state = EdgeTraverser.traverseEdges(initialStates, edges);
StateEditor se = new StateEditor(edges.get(0).getFromVertex(), request);
se.setTimeSeconds(0);

var state = EdgeTraverser.traverseEdges(se.makeState(), edges);

return state.map(s ->
new DefaultRaptorTransfer(
Expand Down
4 changes: 0 additions & 4 deletions application/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,6 @@ public DataOverlayContext dataOverlayContext() {
return dataOverlayContext;
}

public StreetSearchRequestBuilder copyOf(Instant time) {
return copyOf(this).withStartTime(time);
}

public StreetSearchRequestBuilder copyOfReversed(Instant time) {
return copyOf(this).withStartTime(time).withArriveBy(!arriveBy);
}
Expand Down
58 changes: 15 additions & 43 deletions application/src/main/java/org/opentripplanner/street/search/state/EdgeTraverser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

import java.util.Collection;
import java.util.Optional;
import org.opentripplanner.astar.model.ShortestPathTree;
import org.opentripplanner.street.model.edge.Edge;
import org.opentripplanner.street.model.vertex.Vertex;
import org.opentripplanner.street.search.strategy.DominanceFunctions;

/**
* This is a very reduced version of the A* algorithm: from an initial state a number of edges are
Expand All @@ -17,49 +14,24 @@
*/
public class EdgeTraverser {

public static Optional<State> traverseEdges(
final Collection<State> initialStates,
final Collection<Edge> edges
) {
return traverseEdges(initialStates.toArray(new State[0]), edges);
}

public static Optional<State> traverseEdges(
final State[] initialStates,
final Collection<Edge> edges
) {
if (edges.isEmpty()) {
return Optional.of(initialStates[0]);
}

// The shortest path tree is used to prune dominated parallel states. For example,
// CAR_PICKUP can return both a CAR/WALK state after each traversal of which only
// the optimal states need to be continued.
var dominanceFunction = new DominanceFunctions.MinimumWeight();
var spt = new ShortestPathTree<>(dominanceFunction);
for (State initialState : initialStates) {
spt.add(initialState);
}

Vertex lastVertex = null;
var isArriveBy = initialStates[0].getRequest().arriveBy();
public static Optional<State> traverseEdges(final State s, final Collection<Edge> edges) {
var state = s;
for (Edge e : edges) {
var vertex = isArriveBy ? e.getToVertex() : e.getFromVertex();
var fromStates = spt.getStates(vertex);
if (fromStates == null || fromStates.isEmpty()) {
return Optional.empty();
var afterTraversal = e.traverse(state);
if (afterTraversal.length > 1) {
throw new IllegalStateException(
"Expected only a single state returned from edge %s but received %s".formatted(
e,
afterTraversal.length
)
);
}

for (State fromState : fromStates) {
var newToStates = e.traverse(fromState);
for (State newToState : newToStates) {
spt.add(newToState);
}
if (State.isEmpty(afterTraversal)) {
return Optional.empty();
} else {
state = afterTraversal[0];
}

lastVertex = isArriveBy ? e.getFromVertex() : e.getToVertex();
}

return Optional.ofNullable(lastVertex).map(spt::getState);
return Optional.ofNullable(state);
}
}
2 changes: 0 additions & 2 deletions application/src/test/java/org/opentripplanner/_support/geometry/Coordinates.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ public class Coordinates {

public static final Coordinate BERLIN = of(52.5212, 13.4105);
public static final Coordinate BERLIN_BRANDENBURG_GATE = of(52.51627, 13.37770);
public static final Coordinate BERLIN_FERNSEHTURM = of(52.52084, 13.40934);
public static final Coordinate BERLIN_ADMIRALBRUCKE = of(52.49526, 13.415093);
public static final Coordinate HAMBURG = of(53.5566, 10.0003);
public static final Coordinate KONGSBERG_PLATFORM_1 = of(59.67216, 9.65107);
public static final Coordinate BOSTON = of(42.36541, -71.06129);
Expand Down
191 changes: 0 additions & 191 deletions application/src/test/java/org/opentripplanner/street/search/state/EdgeTraverserTest.java
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion application/src/test/java/org/opentripplanner/street/search/state/TestStateBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public TestStateBuilder elevator() {

currentState =
EdgeTraverser
.traverseEdges(new State[] { currentState }, List.of(link, boardEdge, hopEdge, alightEdge))
.traverseEdges(currentState, List.of(link, boardEdge, hopEdge, alightEdge))
.orElseThrow();
return this;
}
Expand Down
Loading