From cb810498275544f0e60c68a68758d84a8a5c500e Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Fri, 3 Jan 2025 09:59:42 +0100 Subject: [PATCH 1/2] Revert "Street search: traverse edges which may return multiple states" This reverts commit 4d11307a --- .../flex/template/AbstractFlexTemplate.java | 2 +- .../flex/template/FlexDirectPathFactory.java | 2 +- .../mapping/RaptorPathToItineraryMapper.java | 30 ++- .../raptoradapter/transit/Transfer.java | 9 +- .../search/request/StreetSearchRequest.java | 4 - .../street/search/state/EdgeTraverser.java | 58 ++---- .../_support/geometry/Coordinates.java | 2 - .../search/state/EdgeTraverserTest.java | 191 ------------------ .../street/search/state/TestStateBuilder.java | 2 +- 9 files changed, 42 insertions(+), 258 deletions(-) delete mode 100644 application/src/test/java/org/opentripplanner/street/search/state/EdgeTraverserTest.java diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java b/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java index aeeab84259c..8dbcf4d785e 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/template/AbstractFlexTemplate.java @@ -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 -> { diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexDirectPathFactory.java b/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexDirectPathFactory.java index ae35c262a1e..f27a502911f 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexDirectPathFactory.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/template/FlexDirectPathFactory.java @@ -113,7 +113,7 @@ private Optional 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(); diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java b/application/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java index 15e3307b4e9..683acdd1a9e 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java @@ -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; @@ -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; @@ -361,15 +360,24 @@ private List 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 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]; + } + } + + State[] states = transferStates.toArray(new State[0]); + var graphPath = new GraphPath<>(states[states.length - 1]); Itinerary subItinerary = graphPathToItineraryMapper.generateItinerary(graphPath); diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java index 2643067398e..1cf8b015816 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java @@ -6,7 +6,6 @@ import java.util.EnumSet; import java.util.List; import java.util.Optional; -import java.util.Set; import org.locationtech.jts.geom.Coordinate; import org.opentripplanner.raptor.api.model.RaptorCostConverter; import org.opentripplanner.raptor.api.model.RaptorTransfer; @@ -15,7 +14,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; @@ -97,8 +96,10 @@ public Optional 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( diff --git a/application/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequest.java b/application/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequest.java index df8933cd22d..c93ea598256 100644 --- a/application/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequest.java +++ b/application/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequest.java @@ -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); } diff --git a/application/src/main/java/org/opentripplanner/street/search/state/EdgeTraverser.java b/application/src/main/java/org/opentripplanner/street/search/state/EdgeTraverser.java index 502d014e358..8755f014e14 100644 --- a/application/src/main/java/org/opentripplanner/street/search/state/EdgeTraverser.java +++ b/application/src/main/java/org/opentripplanner/street/search/state/EdgeTraverser.java @@ -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 @@ -17,49 +14,24 @@ */ public class EdgeTraverser { - public static Optional traverseEdges( - final Collection initialStates, - final Collection edges - ) { - return traverseEdges(initialStates.toArray(new State[0]), edges); - } - - public static Optional traverseEdges( - final State[] initialStates, - final Collection 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 traverseEdges(final State s, final Collection 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); } } diff --git a/application/src/test/java/org/opentripplanner/_support/geometry/Coordinates.java b/application/src/test/java/org/opentripplanner/_support/geometry/Coordinates.java index 33569a34b2e..5a4526012c9 100644 --- a/application/src/test/java/org/opentripplanner/_support/geometry/Coordinates.java +++ b/application/src/test/java/org/opentripplanner/_support/geometry/Coordinates.java @@ -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); diff --git a/application/src/test/java/org/opentripplanner/street/search/state/EdgeTraverserTest.java b/application/src/test/java/org/opentripplanner/street/search/state/EdgeTraverserTest.java deleted file mode 100644 index a2cd7e61b62..00000000000 --- a/application/src/test/java/org/opentripplanner/street/search/state/EdgeTraverserTest.java +++ /dev/null @@ -1,191 +0,0 @@ -package org.opentripplanner.street.search.state; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex; - -import java.time.Instant; -import java.util.ArrayList; -import java.util.List; -import java.util.Set; -import java.util.function.Function; -import org.junit.jupiter.api.Test; -import org.opentripplanner._support.geometry.Coordinates; -import org.opentripplanner.routing.api.request.StreetMode; -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.vertex.IntersectionVertex; -import org.opentripplanner.street.search.TraverseMode; -import org.opentripplanner.street.search.request.StreetSearchRequest; - -class EdgeTraverserTest { - - 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 FERNSEHTURM_V = intersectionVertex( - Coordinates.BERLIN_FERNSEHTURM - ); - private static final IntersectionVertex ADMIRALBRUCKE_V = intersectionVertex( - Coordinates.BERLIN_ADMIRALBRUCKE - ); - - @Test - void emptyEdges() { - var request = StreetSearchRequest - .of() - .withStartTime(Instant.EPOCH) - .withMode(StreetMode.WALK) - .build(); - var initialStates = State.getInitialStates(Set.of(BERLIN_V), request); - var traversedState = EdgeTraverser.traverseEdges(initialStates, List.of()); - - assertSame(initialStates.iterator().next(), traversedState.get()); - } - - @Test - void failedTraversal() { - var edge = StreetModelForTest - .streetEdge(BERLIN_V, BRANDENBURG_GATE_V) - .toBuilder() - .withPermission(StreetTraversalPermission.NONE) - .buildAndConnect(); - - var edges = List.of(edge); - var request = StreetSearchRequest - .of() - .withStartTime(Instant.EPOCH) - .withMode(StreetMode.WALK) - .build(); - var initialStates = State.getInitialStates(Set.of(edge.getFromVertex()), request); - var traversedState = EdgeTraverser.traverseEdges(initialStates, edges); - - assertTrue(traversedState.isEmpty()); - } - - @Test - void withSingleState() { - var edge = StreetModelForTest - .streetEdge(BERLIN_V, BRANDENBURG_GATE_V) - .toBuilder() - .withPermission(StreetTraversalPermission.ALL) - .buildAndConnect(); - - var edges = List.of(edge); - var request = StreetSearchRequest - .of() - .withStartTime(Instant.EPOCH) - .withMode(StreetMode.WALK) - .build(); - var initialStates = State.getInitialStates(Set.of(edge.getFromVertex()), request); - var traversedState = EdgeTraverser.traverseEdges(initialStates, edges).get(); - - assertEquals(List.of(TraverseMode.WALK), stateValues(traversedState, State::getBackMode)); - assertEquals(1719, traversedState.getElapsedTimeSeconds()); - } - - @Test - void withSingleArriveByState() { - var edge = StreetModelForTest - .streetEdge(BERLIN_V, BRANDENBURG_GATE_V) - .toBuilder() - .withPermission(StreetTraversalPermission.ALL) - .buildAndConnect(); - - var edges = List.of(edge); - var request = StreetSearchRequest - .of() - .withStartTime(Instant.EPOCH) - .withMode(StreetMode.WALK) - .withArriveBy(true) - .build(); - var initialStates = State.getInitialStates(Set.of(edge.getToVertex()), request); - var traversedState = EdgeTraverser.traverseEdges(initialStates, edges).get(); - - assertSame(BERLIN_V, traversedState.getVertex()); - assertEquals(List.of(TraverseMode.WALK), stateValues(traversedState, State::getBackMode)); - assertEquals(1719, traversedState.getElapsedTimeSeconds()); - } - - @Test - void withMultipleStates() { - // CAR_PICKUP creates parallel walking and driving states - // This tests that of the two states (WALKING, CAR) the least weight (CAR) is selected - var edge = StreetModelForTest - .streetEdge(BERLIN_V, BRANDENBURG_GATE_V) - .toBuilder() - .withPermission(StreetTraversalPermission.ALL) - .buildAndConnect(); - - var edges = List.of(edge); - var request = StreetSearchRequest - .of() - .withStartTime(Instant.EPOCH) - .withMode(StreetMode.CAR_PICKUP) - .build(); - var initialStates = State.getInitialStates(Set.of(edge.getFromVertex()), request); - var traversedState = EdgeTraverser.traverseEdges(initialStates, edges).get(); - - assertEquals(List.of(TraverseMode.CAR), stateValues(traversedState, State::getBackMode)); - assertEquals(205, traversedState.getElapsedTimeSeconds()); - } - - @Test - void withDominatedStates() { - // CAR_PICKUP creates parallel walking and driving states - // This tests that the most optimal (walking and driving the last stretch) is found after - // discarding the initial driving state for edge1 - var edge1 = StreetModelForTest - .streetEdge(FERNSEHTURM_V, BERLIN_V) - .toBuilder() - .withPermission(StreetTraversalPermission.ALL) - .buildAndConnect(); - var edge2 = StreetModelForTest - .streetEdge(BERLIN_V, BRANDENBURG_GATE_V) - .toBuilder() - .withPermission(StreetTraversalPermission.PEDESTRIAN) - .buildAndConnect(); - var edge3 = StreetModelForTest - .streetEdge(BRANDENBURG_GATE_V, ADMIRALBRUCKE_V) - .toBuilder() - .withPermission(StreetTraversalPermission.ALL) - .buildAndConnect(); - - var edges = List.of(edge1, edge2, edge3); - var request = StreetSearchRequest - .of() - .withStartTime(Instant.EPOCH) - .withMode(StreetMode.CAR_PICKUP) - .build(); - var initialStates = State.getInitialStates(Set.of(edge1.getFromVertex()), request); - var traversedState = EdgeTraverser.traverseEdges(initialStates, edges).get(); - - assertEquals( - List.of(88.103, 2286.029, 3444.28), - stateValues( - traversedState, - state -> state.getBackEdge() != null ? state.getBackEdge().getDistanceMeters() : null - ) - ); - assertEquals( - List.of(TraverseMode.WALK, TraverseMode.WALK, TraverseMode.CAR), - stateValues(traversedState, State::getBackMode) - ); - assertEquals(2169, traversedState.getElapsedTimeSeconds()); - } - - private List stateValues(State state, Function extractor) { - var values = new ArrayList(); - while (state != null) { - var value = extractor.apply(state); - if (value != null) { - values.add(value); - } - state = state.getBackState(); - } - return values.reversed(); - } -} diff --git a/application/src/test/java/org/opentripplanner/street/search/state/TestStateBuilder.java b/application/src/test/java/org/opentripplanner/street/search/state/TestStateBuilder.java index 7ea56f66145..9605d950ae0 100644 --- a/application/src/test/java/org/opentripplanner/street/search/state/TestStateBuilder.java +++ b/application/src/test/java/org/opentripplanner/street/search/state/TestStateBuilder.java @@ -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; } From 868a8dbabf76bac3ffce1e53a5711597f9df642f Mon Sep 17 00:00:00 2001 From: Vincent Paturet Date: Fri, 3 Jan 2025 10:05:46 +0100 Subject: [PATCH 2/2] Fix merge conflict --- .../routing/algorithm/raptoradapter/transit/Transfer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java index 1cf8b015816..20a36376ae7 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/Transfer.java @@ -6,6 +6,7 @@ import java.util.EnumSet; import java.util.List; import java.util.Optional; +import java.util.Set; import org.locationtech.jts.geom.Coordinate; import org.opentripplanner.raptor.api.model.RaptorCostConverter; import org.opentripplanner.raptor.api.model.RaptorTransfer;