From e1bb7d6be66eb65d698db7ccea643814ce5345dc Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 6 Dec 2024 09:32:00 +0100 Subject: [PATCH 1/5] Make flex linking work together with boarding locations --- .../module/StreetLinkerModule.java | 20 ++++++- .../model/vertex/TransitStopVertex.java | 26 +++++++++ .../module/StreetLinkerModuleTest.java | 56 ++++++++++++++++++- 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java b/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java index 83a6f282204..bfd35c2f025 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java @@ -103,7 +103,7 @@ public void linkTransitStops(Graph graph, TimetableRepository timetableRepositor continue; } // check if stop is already linked, to allow multiple idempotent linking cycles - if (tStop.isConnectedToGraph()) { + if (isAlreadyLinked(tStop, stopLocationsUsedForFlexTrips)) { continue; } @@ -127,6 +127,24 @@ public void linkTransitStops(Graph graph, TimetableRepository timetableRepositor LOG.info(progress.completeMessage()); } + /** + * Determines if a given transit stop vertex is already linked to the street network, taking into + * account that flex stop need special linking to both a walkable and drivable edge. + * + * @param stopVertex The transit stop vertex to be checked. + * @param stopLocationsUsedForFlexTrips A set of stop locations that are used for flexible trips. + */ + private static boolean isAlreadyLinked( + TransitStopVertex stopVertex, + Set stopLocationsUsedForFlexTrips + ) { + if (stopLocationsUsedForFlexTrips.contains(stopVertex.getStop())) { + return stopVertex.isLinkedToDrivableEdge() && stopVertex.isLinkedToWalkableEdge(); + } else { + return stopVertex.isConnectedToGraph(); + } + } + /** * Link a stop to the nearest "relevant" edges. *

diff --git a/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java b/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java index 82c977902b0..d43f587280c 100644 --- a/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java +++ b/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java @@ -1,9 +1,14 @@ package org.opentripplanner.street.model.vertex; +import static org.opentripplanner.street.search.TraverseMode.CAR; +import static org.opentripplanner.street.search.TraverseMode.WALK; + import java.util.HashSet; import java.util.Set; import org.opentripplanner.street.model.edge.Edge; import org.opentripplanner.street.model.edge.PathwayEdge; +import org.opentripplanner.street.model.edge.StreetTransitEntityLink; +import org.opentripplanner.street.search.TraverseMode; import org.opentripplanner.transit.model.basic.Accessibility; import org.opentripplanner.transit.model.basic.TransitMode; import org.opentripplanner.transit.model.site.RegularStop; @@ -94,4 +99,25 @@ public StationElement getStationElement() { public boolean isConnectedToGraph() { return getDegreeOut() + getDegreeIn() > 0; } + + public boolean isLinkedToDrivableEdge() { + return isLinkedToEdgeWhichAllows(CAR); + } + + public boolean isLinkedToWalkableEdge() { + return isLinkedToEdgeWhichAllows(WALK); + } + + private boolean isLinkedToEdgeWhichAllows(TraverseMode traverseMode) { + return getOutgoing() + .stream() + .anyMatch(edge -> + edge instanceof StreetTransitEntityLink link && + link + .getToVertex() + .getOutgoingStreetEdges() + .stream() + .anyMatch(se -> se.canTraverse(traverseMode)) + ); + } } diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java index 4f54581fdb8..3a9452e022a 100644 --- a/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java +++ b/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java @@ -11,6 +11,7 @@ import java.util.Arrays; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Test; import org.opentripplanner.ext.flex.trip.UnscheduledTrip; import org.opentripplanner.framework.application.OTPFeature; @@ -20,8 +21,10 @@ import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.service.vehicleparking.internal.DefaultVehicleParkingRepository; import org.opentripplanner.street.model._data.StreetModelForTest; +import org.opentripplanner.street.model.edge.BoardingLocationToStopLink; import org.opentripplanner.street.model.edge.Edge; import org.opentripplanner.street.model.edge.StreetTransitStopLink; +import org.opentripplanner.street.model.vertex.OsmBoardingLocationVertex; import org.opentripplanner.street.model.vertex.SplitterVertex; import org.opentripplanner.street.model.vertex.TransitStopVertex; import org.opentripplanner.transit.model._data.TimetableRepositoryForTest; @@ -103,6 +106,38 @@ void linkFlexStop() { }); } + @Test + void linkFlexStopWithBoardingLocation() { + OTPFeature.FlexRouting.testOn(() -> { + var model = new TestModel().withStopLinkedToBoardingLocation(); + var flexTrip = TimetableRepositoryForTest.of().unscheduledTrip("flex", model.stop()); + model.withFlexTrip(flexTrip); + + var module = model.streetLinkerModule(); + + module.buildGraph(); + + assertTrue(model.stopVertex().isConnectedToGraph()); + + // stop is used by a flex trip, needs to be linked to both the walk and car edge, + // also linked to the boarding location + assertThat(model.stopVertex().getOutgoing()).hasSize(3); + var links = model.outgoingLinks(); + assertInstanceOf(BoardingLocationToStopLink.class, links.getFirst()); + var linkToWalk = links.get(1); + SplitterVertex walkSplit = (SplitterVertex) linkToWalk.getToVertex(); + + assertTrue(walkSplit.isConnectedToWalkingEdge()); + assertFalse(walkSplit.isConnectedToDriveableEdge()); + + var linkToCar = links.getLast(); + SplitterVertex carSplit = (SplitterVertex) linkToCar.getToVertex(); + + assertFalse(carSplit.isConnectedToWalkingEdge()); + assertTrue(carSplit.isConnectedToDriveableEdge()); + }); + } + @Test void linkCarsAllowedStop() { var model = new TestModel(); @@ -140,6 +175,7 @@ private static class TestModel { private final StreetLinkerModule module; private final RegularStop stop; private final TimetableRepository timetableRepository; + private final Graph graph; public TestModel() { var from = StreetModelForTest.intersectionVertex( @@ -151,7 +187,7 @@ public TestModel() { KONGSBERG_PLATFORM_1.x + DELTA ); - Graph graph = new Graph(); + this.graph = new Graph(); graph.addVertex(from); graph.addVertex(to); @@ -232,5 +268,23 @@ public void withCarsAllowedTrip(Trip trip, StopLocation... stops) { timetableRepository.addTripPattern(tripPattern.getId(), tripPattern); } + + /** + * Links the stop to a boarding location as can happen during regular graph build. + */ + public TestModel withStopLinkedToBoardingLocation() { + var boardingLocation = new OsmBoardingLocationVertex( + "boarding-location", + KONGSBERG_PLATFORM_1.x - 0.0001, + KONGSBERG_PLATFORM_1.y - 0.0001, + null, + Set.of(stop.getId().getId()) + ); + graph.addVertex(boardingLocation); + + BoardingLocationToStopLink.createBoardingLocationToStopLink(boardingLocation, stopVertex); + BoardingLocationToStopLink.createBoardingLocationToStopLink(stopVertex, boardingLocation); + return this; + } } } From 22a414cc9f445448df663d5be7e06deed31f22be Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 6 Dec 2024 09:53:50 +0100 Subject: [PATCH 2/5] Add javadoc --- .../street/model/vertex/TransitStopVertex.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java b/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java index d43f587280c..7d5d52f02b3 100644 --- a/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java +++ b/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java @@ -100,10 +100,18 @@ public boolean isConnectedToGraph() { return getDegreeOut() + getDegreeIn() > 0; } + /** + * Determines if this vertex is linked (via a {@link StreetTransitEntityLink}) to a drivable edge + * in the street network. + */ public boolean isLinkedToDrivableEdge() { return isLinkedToEdgeWhichAllows(CAR); } + /** + * Determines if this vertex is linked (via a {@link StreetTransitEntityLink}) to a walkable edge + * in the street network. + */ public boolean isLinkedToWalkableEdge() { return isLinkedToEdgeWhichAllows(WALK); } From 47d444165dcd0f911314071d73f8f3ae55d24c0d Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 6 Dec 2024 15:02:26 +0100 Subject: [PATCH 3/5] Add warnings --- .../street/model/vertex/TransitStopVertex.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java b/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java index 7d5d52f02b3..5b1b1dfa6a8 100644 --- a/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java +++ b/application/src/main/java/org/opentripplanner/street/model/vertex/TransitStopVertex.java @@ -103,6 +103,8 @@ public boolean isConnectedToGraph() { /** * Determines if this vertex is linked (via a {@link StreetTransitEntityLink}) to a drivable edge * in the street network. + *

+ * This method is slow: only use this during graph build. */ public boolean isLinkedToDrivableEdge() { return isLinkedToEdgeWhichAllows(CAR); @@ -111,6 +113,8 @@ public boolean isLinkedToDrivableEdge() { /** * Determines if this vertex is linked (via a {@link StreetTransitEntityLink}) to a walkable edge * in the street network. + *

+ * This method is slow: only use this during graph build. */ public boolean isLinkedToWalkableEdge() { return isLinkedToEdgeWhichAllows(WALK); From ee5b8421309be1c674ecf08e82ca5341409c8e64 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 9 Dec 2024 08:11:44 +0100 Subject: [PATCH 4/5] Improve comment --- .../graph_builder/module/StreetLinkerModule.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java b/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java index bfd35c2f025..b59b2aee20f 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java @@ -129,7 +129,9 @@ public void linkTransitStops(Graph graph, TimetableRepository timetableRepositor /** * Determines if a given transit stop vertex is already linked to the street network, taking into - * account that flex stop need special linking to both a walkable and drivable edge. + * account that flex stop need special linking to both a walkable and drivable edge. For example, + * the {@link OsmBoardingLocationsModule}, which runs before this one, links stops often to + * walkable edges only. * * @param stopVertex The transit stop vertex to be checked. * @param stopLocationsUsedForFlexTrips A set of stop locations that are used for flexible trips. From ebbba525dd2d40d550a7e1ffb2d3f664b9cdfd06 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 9 Dec 2024 16:20:42 +0100 Subject: [PATCH 5/5] Apply review feedback --- .../graph_builder/module/StreetLinkerModule.java | 4 ++-- .../graph_builder/module/StreetLinkerModuleTest.java | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java b/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java index b59b2aee20f..d50659f275c 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/StreetLinkerModule.java @@ -129,8 +129,8 @@ public void linkTransitStops(Graph graph, TimetableRepository timetableRepositor /** * Determines if a given transit stop vertex is already linked to the street network, taking into - * account that flex stop need special linking to both a walkable and drivable edge. For example, - * the {@link OsmBoardingLocationsModule}, which runs before this one, links stops often to + * account that flex stops need special linking to both a walkable and drivable edge. For example, + * the {@link OsmBoardingLocationsModule}, which runs before this one, often links stops to * walkable edges only. * * @param stopVertex The transit stop vertex to be checked. diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java index 3a9452e022a..10427812ac2 100644 --- a/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java +++ b/application/src/test/java/org/opentripplanner/graph_builder/module/StreetLinkerModuleTest.java @@ -122,14 +122,23 @@ void linkFlexStopWithBoardingLocation() { // stop is used by a flex trip, needs to be linked to both the walk and car edge, // also linked to the boarding location assertThat(model.stopVertex().getOutgoing()).hasSize(3); + + // while the order of the link doesn't matter, it _is_ deterministic. + // first we have the link to the boarding location where the passengers are expected + // to wait. var links = model.outgoingLinks(); assertInstanceOf(BoardingLocationToStopLink.class, links.getFirst()); + + // the second link is the link to the walkable street network. this is not really necessary + // because the boarding location is walkable. this will be refactored away in the future. var linkToWalk = links.get(1); SplitterVertex walkSplit = (SplitterVertex) linkToWalk.getToVertex(); assertTrue(walkSplit.isConnectedToWalkingEdge()); assertFalse(walkSplit.isConnectedToDriveableEdge()); + // lastly we have the link to the drivable street network because vehicles also need to + // reach the stop if it's part of a flex trip. var linkToCar = links.getLast(); SplitterVertex carSplit = (SplitterVertex) linkToCar.getToVertex();