From 33189c22fd4fc74407f9c5b3f20d7425ed9ea580 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Tue, 26 Nov 2024 11:09:33 +0200 Subject: [PATCH 01/16] Rename 'transferByStopIndex' variable to 'transfersByStopIndex'. --- .../raptoradapter/transit/mappers/TransfersMapper.java | 6 +++--- .../raptoradapter/transit/mappers/TransitLayerMapper.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java index ff47cb3ea74..3de07da9e19 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java @@ -18,7 +18,7 @@ static List> mapTransfers( SiteRepository siteRepository, TransitService transitService ) { - List> transferByStopIndex = new ArrayList<>(); + List> transfersByStopIndex = new ArrayList<>(); for (int i = 0; i < siteRepository.stopIndexSize(); ++i) { var stop = siteRepository.stopByIndex(i); @@ -45,10 +45,10 @@ static List> mapTransfers( } // Create a copy to compact and make the inner lists immutable - transferByStopIndex.add(List.copyOf(list)); + transfersByStopIndex.add(List.copyOf(list)); } // Return an immutable copy - return List.copyOf(transferByStopIndex); + return List.copyOf(transfersByStopIndex); } } diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java index d6f9c0709c5..edcfa3f9dc8 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java @@ -62,7 +62,7 @@ public static TransitLayer map( private TransitLayer map(TransitTuningParameters tuningParameters) { HashMap> tripPatternsByStopByDate; - List> transferByStopIndex; + List> transfersByStopIndex; ConstrainedTransfersForPatterns constrainedTransfers = null; LOG.info("Mapping transitLayer from TimetableRepository..."); @@ -71,7 +71,7 @@ private TransitLayer map(TransitTuningParameters tuningParameters) { tripPatternsByStopByDate = mapTripPatterns(allTripPatterns); - transferByStopIndex = mapTransfers(siteRepository, transitService); + transfersByStopIndex = mapTransfers(siteRepository, transitService); TransferIndexGenerator transferIndexGenerator = null; if (OTPFeature.TransferConstraints.isOn()) { @@ -86,7 +86,7 @@ private TransitLayer map(TransitTuningParameters tuningParameters) { return new TransitLayer( tripPatternsByStopByDate, - transferByStopIndex, + transfersByStopIndex, transitService.getTransferService(), siteRepository, transferCache, From aa4927ed83734c9af78df7caacb6235aa647db3f Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 29 Nov 2024 15:09:43 +0200 Subject: [PATCH 02/16] Add changes for CAR transfers. --- .../api/parameter/QualifiedModeSet.java | 7 ++++--- .../mapping/RaptorPathToItineraryMapper.java | 2 +- ...treetModeToTransferTraverseModeMapper.java | 21 +++++++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 application/src/main/java/org/opentripplanner/routing/algorithm/mapping/StreetModeToTransferTraverseModeMapper.java diff --git a/application/src/main/java/org/opentripplanner/api/parameter/QualifiedModeSet.java b/application/src/main/java/org/opentripplanner/api/parameter/QualifiedModeSet.java index c6f1a3d74ec..98ce58c6d21 100644 --- a/application/src/main/java/org/opentripplanner/api/parameter/QualifiedModeSet.java +++ b/application/src/main/java/org/opentripplanner/api/parameter/QualifiedModeSet.java @@ -126,9 +126,10 @@ public RequestModes getRequestModes() { mBuilder.withEgressMode(StreetMode.CAR_HAILING); mBuilder.withDirectMode(StreetMode.WALK); } else { - mBuilder.withAccessMode(StreetMode.WALK); - mBuilder.withTransferMode(StreetMode.WALK); - mBuilder.withEgressMode(StreetMode.WALK); + // This is necessary for transfer calculations. + mBuilder.withAccessMode(StreetMode.CAR); + mBuilder.withTransferMode(StreetMode.CAR); + mBuilder.withEgressMode(StreetMode.CAR); mBuilder.withDirectMode(StreetMode.CAR); } } 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 64c59b71603..15e3307b4e9 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 @@ -126,7 +126,7 @@ else if (pathLeg.isTransferLeg()) { legs.addAll( mapTransferLeg( pathLeg.asTransferLeg(), - transferMode == StreetMode.BIKE ? TraverseMode.BICYCLE : TraverseMode.WALK + StreetModeToTransferTraverseModeMapper.map(transferMode) ) ); } diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/mapping/StreetModeToTransferTraverseModeMapper.java b/application/src/main/java/org/opentripplanner/routing/algorithm/mapping/StreetModeToTransferTraverseModeMapper.java new file mode 100644 index 00000000000..65db8e87832 --- /dev/null +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/mapping/StreetModeToTransferTraverseModeMapper.java @@ -0,0 +1,21 @@ +package org.opentripplanner.routing.algorithm.mapping; + +import org.opentripplanner.routing.api.request.StreetMode; +import org.opentripplanner.street.search.TraverseMode; + +/** + * Maps street mode to transfer traverse mode. + */ +public class StreetModeToTransferTraverseModeMapper { + + public static TraverseMode map(StreetMode mode) { + return switch (mode) { + case WALK -> TraverseMode.WALK; + case BIKE -> TraverseMode.BICYCLE; + case CAR -> TraverseMode.CAR; + default -> throw new IllegalArgumentException( + String.format("StreetMode %s can not be mapped to a TraverseMode for transfers.", mode) + ); + }; + } +} From 4c882f42b22c20c582ca67438171b725523ff7d4 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 29 Nov 2024 15:13:10 +0200 Subject: [PATCH 03/16] Add EnumSet of StreetModes to PathTransfers and implement mode-specific transfers. --- .../opentripplanner/ext/flex/FlexIndex.java | 14 ++++++- .../opentripplanner/ext/flex/FlexRouter.java | 2 +- .../module/DirectTransferGenerator.java | 42 +++++++++++++++---- .../opentripplanner/model/PathTransfer.java | 25 +++++++++-- .../transit/RaptorTransferIndex.java | 3 ++ .../raptoradapter/transit/Transfer.java | 13 +++++- .../transit/mappers/TransfersMapper.java | 9 +++- .../RaptorPathToItineraryMapperTest.java | 4 +- .../raptoradapter/transit/TransferTest.java | 14 ++++--- 9 files changed, 101 insertions(+), 25 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java index cd047de6d04..ccc4f7fe554 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java @@ -5,9 +5,11 @@ import com.google.common.collect.Multimap; import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.opentripplanner.ext.flex.trip.FlexTrip; import org.opentripplanner.model.PathTransfer; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.network.Route; import org.opentripplanner.transit.model.site.GroupStop; @@ -18,6 +20,8 @@ public class FlexIndex { private final Multimap transfersToStop = ArrayListMultimap.create(); + private final Multimap transfersFromStop = ArrayListMultimap.create(); + private final Multimap> flexTripsByStop = HashMultimap.create(); private final Map routeById = new HashMap<>(); @@ -25,8 +29,12 @@ public class FlexIndex { private final Map> tripById = new HashMap<>(); public FlexIndex(TimetableRepository timetableRepository) { - for (PathTransfer transfer : timetableRepository.getAllPathTransfers()) { + // Flex transfers should only use WALK mode transfers. + StreetMode mode = StreetMode.WALK; + List filteredPathTransfers = timetableRepository.getAllPathTransfers().stream().filter(pathTransfer -> pathTransfer.getModes().contains(mode)).toList(); + for (PathTransfer transfer : filteredPathTransfers) { transfersToStop.put(transfer.to, transfer); + transfersFromStop.put(transfer.from, transfer); } for (FlexTrip flexTrip : timetableRepository.getAllFlexTrips()) { routeById.put(flexTrip.getTrip().getRoute().getId(), flexTrip.getTrip().getRoute()); @@ -47,6 +55,10 @@ public Collection getTransfersToStop(StopLocation stopLocation) { return transfersToStop.get(stopLocation); } + public Collection getTransfersFromStop(StopLocation stopLocation) { + return transfersFromStop.get(stopLocation); + } + public Collection> getFlexTripsByStop(StopLocation stopLocation) { return flexTripsByStop.get(stopLocation); } diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java index 46d9e527980..e7f3d3544ab 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexRouter.java @@ -192,7 +192,7 @@ public TransitStopVertex getStopVertexForStopId(FeedScopedId stopId) { @Override public Collection getTransfersFromStop(StopLocation stop) { - return transitService.findPathTransfers(stop); + return transitService.getFlexIndex().getTransfersFromStop(stop); } @Override diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index c0deb932418..4c26808ab80 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -3,9 +3,12 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimaps; import java.time.Duration; +import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; @@ -17,6 +20,7 @@ import org.opentripplanner.graph_builder.module.nearbystops.StreetNearbyStopFinder; import org.opentripplanner.model.PathTransfer; import org.opentripplanner.routing.api.request.RouteRequest; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.routing.graphfinder.NearbyStop; import org.opentripplanner.street.model.edge.Edge; @@ -102,6 +106,7 @@ public void buildGraph() { LOG.debug("Linking stop '{}' {}", stop, ts0); for (RouteRequest transferProfile : transferRequests) { + StreetMode mode = transferProfile.journey().transfer().mode(); for (NearbyStop sd : nearbyStopFinder.findNearbyStops( ts0, transferProfile, @@ -115,12 +120,12 @@ public void buildGraph() { if (sd.stop.transfersNotAllowed()) { continue; } - distinctTransfers.put( - new TransferKey(stop, sd.stop, sd.edges), - new PathTransfer(stop, sd.stop, sd.distance, sd.edges) - ); + createPathTransfer(distinctTransfers, sd, stop, mode); } - if (OTPFeature.FlexRouting.isOn()) { + } + if (OTPFeature.FlexRouting.isOn()) { + for (RouteRequest transferProfile : transferRequests) { + StreetMode mode = transferProfile.journey().transfer().mode(); // This code is for finding transfers from AreaStops to Stops, transfers // from Stops to AreaStops and between Stops are already covered above. for (NearbyStop sd : nearbyStopFinder.findNearbyStops( @@ -136,10 +141,7 @@ public void buildGraph() { if (sd.stop instanceof RegularStop) { continue; } - distinctTransfers.put( - new TransferKey(sd.stop, stop, sd.edges), - new PathTransfer(sd.stop, stop, sd.distance, sd.edges) - ); + createPathTransfer(distinctTransfers, sd, stop, mode); } } } @@ -174,6 +176,28 @@ public void buildGraph() { ); } + /** + * Factory method for creating a PathTransfer. + */ + private void createPathTransfer( + Map distinctTransfers, + NearbyStop sd, + RegularStop stop, + StreetMode mode + ) { + TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges); + PathTransfer pathTransfer = distinctTransfers.get(transferKey); + if (pathTransfer == null) { + EnumSet modes = EnumSet.of(mode); + distinctTransfers.put( + transferKey, + new PathTransfer(stop, sd.stop, sd.distance, sd.edges, modes) + ); + } else { + pathTransfer.addMode(mode); + } + } + /** * Factory method for creating a NearbyStopFinder. Will create different finders depending on * whether the graph has a street network and if ConsiderPatternsForDirectTransfers feature is diff --git a/application/src/main/java/org/opentripplanner/model/PathTransfer.java b/application/src/main/java/org/opentripplanner/model/PathTransfer.java index 5b42a8149e3..ed4696ab11c 100644 --- a/application/src/main/java/org/opentripplanner/model/PathTransfer.java +++ b/application/src/main/java/org/opentripplanner/model/PathTransfer.java @@ -1,19 +1,20 @@ package org.opentripplanner.model; import java.io.Serializable; +import java.util.EnumSet; import java.util.List; import org.opentripplanner.model.transfer.ConstrainedTransfer; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.street.model.edge.Edge; import org.opentripplanner.transit.model.site.StopLocation; import org.opentripplanner.utils.tostring.ToStringBuilder; /** - * Represents a transfer between stops with the street network path attatched to it. + * Represents a transfer for a set of modes between stops with the street network path attached to it. *

* Do not confuse this with {@link ConstrainedTransfer}. * *

- * TODO these should really have a set of valid modes in case bike vs. walk transfers are different * TODO Should we just store the NearbyStop as a field here, or even switch to using it instead * where this class is used */ @@ -27,11 +28,20 @@ public class PathTransfer implements Serializable { private final List edges; - public PathTransfer(StopLocation from, StopLocation to, double distanceMeters, List edges) { + private EnumSet modes; + + public PathTransfer( + StopLocation from, + StopLocation to, + double distanceMeters, + List edges, + EnumSet modes + ) { this.from = from; this.to = to; this.distanceMeters = distanceMeters; this.edges = edges; + this.modes = modes; } public String getName() { @@ -46,6 +56,14 @@ public List getEdges() { return this.edges; } + public EnumSet getModes() { + return this.modes; + } + + public boolean addMode(StreetMode mode) { + return this.modes.add(mode); + } + @Override public String toString() { return ToStringBuilder @@ -54,6 +72,7 @@ public String toString() { .addObj("to", to) .addNum("distance", distanceMeters) .addColSize("edges", edges) + .addColSize("modes", modes) .toString(); } } diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java index 1d9b804067c..941e1296838 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.function.Function; import org.opentripplanner.raptor.api.model.RaptorTransfer; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.street.search.request.StreetSearchRequest; public class RaptorTransferIndex { @@ -29,6 +30,7 @@ public static RaptorTransferIndex create( ) { var forwardTransfers = new ArrayList>(transfersByStopIndex.size()); var reversedTransfers = new ArrayList>(transfersByStopIndex.size()); + StreetMode mode = request.mode(); for (int i = 0; i < transfersByStopIndex.size(); i++) { forwardTransfers.add(new ArrayList<>()); @@ -41,6 +43,7 @@ public static RaptorTransferIndex create( var transfers = transfersByStopIndex .get(fromStop) .stream() + .filter(transfer -> transfer.getModes().contains(mode)) .flatMap(s -> s.asRaptorTransfer(request).stream()) .collect( toMap(RaptorTransfer::stop, Function.identity(), (a, b) -> a.c1() < b.c1() ? a : b) 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 32d54787e6f..c0c923228bf 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 @@ -8,6 +8,7 @@ import org.locationtech.jts.geom.Coordinate; import org.opentripplanner.raptor.api.model.RaptorCostConverter; import org.opentripplanner.raptor.api.model.RaptorTransfer; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.routing.api.request.preference.WalkPreferences; import org.opentripplanner.street.model.edge.Edge; import org.opentripplanner.street.search.request.StreetSearchRequest; @@ -31,16 +32,20 @@ public class Transfer { private final List edges; - public Transfer(int toStop, List edges) { + private final Set modes; + + public Transfer(int toStop, List edges, Set modes) { this.toStop = toStop; this.edges = edges; this.distanceMeters = (int) edges.stream().mapToDouble(Edge::getDistanceMeters).sum(); + this.modes = modes; } - public Transfer(int toStopIndex, int distanceMeters) { + public Transfer(int toStopIndex, int distanceMeters, Set modes) { this.toStop = toStopIndex; this.distanceMeters = distanceMeters; this.edges = null; + this.modes = modes; } public List getCoordinates() { @@ -68,6 +73,10 @@ public List getEdges() { return edges; } + public Set getModes() { + return modes; + } + public Optional asRaptorTransfer(StreetSearchRequest request) { WalkPreferences walkPreferences = request.preferences().walk(); if (edges == null || edges.isEmpty()) { diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java index 3de07da9e19..9fc49c5c5e1 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java @@ -34,10 +34,15 @@ static List> mapTransfers( int toStopIndex = pathTransfer.to.getIndex(); Transfer newTransfer; if (pathTransfer.getEdges() != null) { - newTransfer = new Transfer(toStopIndex, pathTransfer.getEdges()); + newTransfer = + new Transfer(toStopIndex, pathTransfer.getEdges(), pathTransfer.getModes()); } else { newTransfer = - new Transfer(toStopIndex, (int) Math.ceil(pathTransfer.getDistanceMeters())); + new Transfer( + toStopIndex, + (int) Math.ceil(pathTransfer.getDistanceMeters()), + pathTransfer.getModes() + ); } list.add(newTransfer); diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java index 8f2fc45b875..c397cc74877 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -54,6 +55,7 @@ import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitLayer; import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.DefaultCostCalculator; import org.opentripplanner.routing.api.request.RouteRequest; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.routing.graph.Graph; import org.opentripplanner.street.search.state.State; import org.opentripplanner.street.search.state.TestStateBuilder; @@ -199,7 +201,7 @@ void createItineraryWithOnBoardFlexAccess() { flexAccessEgress, AccessEgressType.ACCESS ); - Transfer transfer = new Transfer(S2.getIndex(), 0); + Transfer transfer = new Transfer(S2.getIndex(), 0, Set.of(StreetMode.WALK)); RaptorTransfer raptorTransfer = new DefaultRaptorTransfer(S1.getIndex(), 0, 0, transfer); RaptorAccessEgress egress = new DefaultAccessEgress(S2.getIndex(), state); PathLeg egressLeg = new EgressPathLeg<>(egress, 0, 0, 0); diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java index 8dc4d548c18..ca846148dea 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java @@ -6,11 +6,13 @@ import java.util.List; import java.util.Optional; +import java.util.Set; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.opentripplanner._support.geometry.Coordinates; import org.opentripplanner.raptor.api.model.RaptorCostConverter; import org.opentripplanner.raptor.api.model.RaptorTransfer; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.street.model._data.StreetModelForTest; import org.opentripplanner.street.model.vertex.IntersectionVertex; import org.opentripplanner.street.search.request.StreetSearchRequest; @@ -34,7 +36,7 @@ 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)); + var veryLongTransfer = new Transfer(0, List.of(edge), Set.of(StreetMode.WALK)); 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()); @@ -43,7 +45,7 @@ void limitMaxCost() { @Test void allowLowCost() { var edge = StreetModelForTest.streetEdge(BERLIN_V, BRANDENBURG_GATE_V); - var transfer = new Transfer(0, List.of(edge)); + var transfer = new Transfer(0, List.of(edge), Set.of(StreetMode.WALK)); assertTrue(transfer.getDistanceMeters() < 4000); final Optional raptorTransfer = transfer.asRaptorTransfer( StreetSearchRequest.of().build() @@ -58,26 +60,26 @@ class WithoutEdges { @Test void overflow() { - var veryLongTransfer = new Transfer(0, Integer.MAX_VALUE); + var veryLongTransfer = new Transfer(0, Integer.MAX_VALUE, Set.of(StreetMode.WALK)); assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get()); } @Test void negativeCost() { - var veryLongTransfer = new Transfer(0, -5); + var veryLongTransfer = new Transfer(0, -5, Set.of(StreetMode.WALK)); assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get()); } @Test void limitMaxCost() { - var veryLongTransfer = new Transfer(0, 8_000_000); + var veryLongTransfer = new Transfer(0, 8_000_000, Set.of(StreetMode.WALK)); // 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); + var transfer = new Transfer(0, 200, Set.of(StreetMode.WALK)); final Optional raptorTransfer = transfer.asRaptorTransfer( StreetSearchRequest.of().build() ); From 44e4d31e0f7d7945513c2d465035eb173e482f57 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 29 Nov 2024 15:13:55 +0200 Subject: [PATCH 04/16] Add test for mode-specific transfers. --- .../module/DirectTransferGeneratorTest.java | 59 ++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java index 8a17e5258d8..310e474ae99 100644 --- a/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java +++ b/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java @@ -196,7 +196,7 @@ public void testMultipleRequestsWithoutPatterns() { reqWalk.journey().transfer().setMode(StreetMode.WALK); var reqBike = new RouteRequest(); - reqWalk.journey().transfer().setMode(StreetMode.BIKE); + reqBike.journey().transfer().setMode(StreetMode.BIKE); var transferRequests = List.of(reqWalk, reqBike); @@ -223,7 +223,7 @@ public void testMultipleRequestsWithPatterns() { reqWalk.journey().transfer().setMode(StreetMode.WALK); var reqBike = new RouteRequest(); - reqWalk.journey().transfer().setMode(StreetMode.BIKE); + reqBike.journey().transfer().setMode(StreetMode.BIKE); var transferRequests = List.of(reqWalk, reqBike); @@ -250,6 +250,61 @@ public void testMultipleRequestsWithPatterns() { ); } + @Test + public void testPathTransfersWithModesForMultipleRequestsWithPatterns() { + var reqWalk = new RouteRequest(); + reqWalk.journey().transfer().setMode(StreetMode.WALK); + + var reqBike = new RouteRequest(); + reqBike.journey().transfer().setMode(StreetMode.BIKE); + + var transferRequests = List.of(reqWalk, reqBike); + + TestOtpModel model = model(true); + var graph = model.graph(); + graph.hasStreets = true; + var timetableRepository = model.timetableRepository(); + + new DirectTransferGenerator( + graph, + timetableRepository, + DataImportIssueStore.NOOP, + MAX_TRANSFER_DURATION, + transferRequests + ) + .buildGraph(); + + var walkTransfers = timetableRepository + .getAllPathTransfers() + .stream() + .filter(pathTransfer -> pathTransfer.getModes().contains(StreetMode.WALK)) + .toList(); + var bikeTransfers = timetableRepository + .getAllPathTransfers() + .stream() + .filter(pathTransfer -> pathTransfer.getModes().contains(StreetMode.BIKE)) + .toList(); + var carTransfers = timetableRepository + .getAllPathTransfers() + .stream() + .filter(pathTransfer -> pathTransfer.getModes().contains(StreetMode.CAR)) + .toList(); + + assertTransfers( + walkTransfers, + tr(S0, 100, List.of(V0, V11), S11), + tr(S0, 100, List.of(V0, V21), S21), + tr(S11, 100, List.of(V11, V21), S21) + ); + assertTransfers( + bikeTransfers, + tr(S0, 100, List.of(V0, V11), S11), + tr(S0, 100, List.of(V0, V21), S21), + tr(S11, 110, List.of(V11, V22), S22) + ); + assertTransfers(carTransfers); + } + @Test public void testTransferOnIsolatedStations() { var otpModel = model(true, false, true); From 8448e6e91539ae9015269cc8de1ecef453dbc64e Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 29 Nov 2024 16:25:55 +0200 Subject: [PATCH 05/16] Remove unused imports and small changes. --- .../module/DirectTransferGenerator.java | 2 -- .../algorithm/raptoradapter/transit/Transfer.java | 9 +++++---- .../mapping/RaptorPathToItineraryMapperTest.java | 4 ++-- .../raptoradapter/transit/TransferTest.java | 13 +++++++------ 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index 4c26808ab80..27fc87d9fff 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -5,10 +5,8 @@ import java.time.Duration; import java.util.EnumSet; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; 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 c0c923228bf..6ee0dee83c0 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 @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.EnumSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -32,16 +33,16 @@ public class Transfer { private final List edges; - private final Set modes; + private final EnumSet modes; - public Transfer(int toStop, List edges, Set modes) { + public Transfer(int toStop, List edges, EnumSet modes) { this.toStop = toStop; this.edges = edges; this.distanceMeters = (int) edges.stream().mapToDouble(Edge::getDistanceMeters).sum(); this.modes = modes; } - public Transfer(int toStopIndex, int distanceMeters, Set modes) { + public Transfer(int toStopIndex, int distanceMeters, EnumSet modes) { this.toStop = toStopIndex; this.distanceMeters = distanceMeters; this.edges = null; @@ -73,7 +74,7 @@ public List getEdges() { return edges; } - public Set getModes() { + public EnumSet getModes() { return modes; } diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java index c397cc74877..1c43234bd4d 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapperTest.java @@ -12,9 +12,9 @@ import java.time.LocalDateTime; import java.time.Month; import java.util.ArrayList; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; -import java.util.Set; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -201,7 +201,7 @@ void createItineraryWithOnBoardFlexAccess() { flexAccessEgress, AccessEgressType.ACCESS ); - Transfer transfer = new Transfer(S2.getIndex(), 0, Set.of(StreetMode.WALK)); + Transfer transfer = new Transfer(S2.getIndex(), 0, EnumSet.of(StreetMode.WALK)); RaptorTransfer raptorTransfer = new DefaultRaptorTransfer(S1.getIndex(), 0, 0, transfer); RaptorAccessEgress egress = new DefaultAccessEgress(S2.getIndex(), state); PathLeg egressLeg = new EgressPathLeg<>(egress, 0, 0, 0); diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java index ca846148dea..7edda8fba9c 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex; +import java.util.EnumSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -36,7 +37,7 @@ 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), Set.of(StreetMode.WALK)); + var veryLongTransfer = new Transfer(0, List.of(edge), EnumSet.of(StreetMode.WALK)); 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()); @@ -45,7 +46,7 @@ void limitMaxCost() { @Test void allowLowCost() { var edge = StreetModelForTest.streetEdge(BERLIN_V, BRANDENBURG_GATE_V); - var transfer = new Transfer(0, List.of(edge), Set.of(StreetMode.WALK)); + var transfer = new Transfer(0, List.of(edge), EnumSet.of(StreetMode.WALK)); assertTrue(transfer.getDistanceMeters() < 4000); final Optional raptorTransfer = transfer.asRaptorTransfer( StreetSearchRequest.of().build() @@ -60,26 +61,26 @@ class WithoutEdges { @Test void overflow() { - var veryLongTransfer = new Transfer(0, Integer.MAX_VALUE, Set.of(StreetMode.WALK)); + var veryLongTransfer = new Transfer(0, Integer.MAX_VALUE, EnumSet.of(StreetMode.WALK)); assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get()); } @Test void negativeCost() { - var veryLongTransfer = new Transfer(0, -5, Set.of(StreetMode.WALK)); + var veryLongTransfer = new Transfer(0, -5, EnumSet.of(StreetMode.WALK)); assertMaxCost(veryLongTransfer.asRaptorTransfer(StreetSearchRequest.of().build()).get()); } @Test void limitMaxCost() { - var veryLongTransfer = new Transfer(0, 8_000_000, Set.of(StreetMode.WALK)); + var veryLongTransfer = new Transfer(0, 8_000_000, EnumSet.of(StreetMode.WALK)); // 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, Set.of(StreetMode.WALK)); + var transfer = new Transfer(0, 200, EnumSet.of(StreetMode.WALK)); final Optional raptorTransfer = transfer.asRaptorTransfer( StreetSearchRequest.of().build() ); From 3c46be363a6912f6f221eba1aebf9a9ebbbf1a39 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Tue, 3 Dec 2024 08:10:56 +0200 Subject: [PATCH 06/16] Add logging for mode-specific transfers. --- .../module/DirectTransferGenerator.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index 27fc87d9fff..0219445bb3d 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -8,6 +8,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.graph_builder.issues.StopNotLinkedForTransfers; @@ -172,6 +173,20 @@ public void buildGraph() { nTransfersTotal, nLinkedStops ); + for (StreetMode mode : transferRequests + .stream() + .map(transferProfile -> transferProfile.journey().transfer().mode()) + .collect(Collectors.toSet())) { + LOG.info( + "Created {} transfers for mode {}.", + transfersByStop + .values() + .stream() + .filter(pathTransfer -> pathTransfer.getModes().contains(mode)) + .count(), + mode + ); + } } /** From 244c5371bc8319981906afc8ce5cca97ebd57829 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Wed, 4 Dec 2024 10:42:54 +0200 Subject: [PATCH 07/16] Fix flex routing transfer calculations and only calculate flex transfer requests for WALK mode. --- .../module/DirectTransferGenerator.java | 95 +++++++++++-------- 1 file changed, 53 insertions(+), 42 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index 0219445bb3d..19e2c8227d0 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -3,6 +3,7 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimaps; import java.time.Duration; +import java.util.ArrayList; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -89,6 +90,17 @@ public void buildGraph() { HashMultimap.create() ); + List flexTransferRequests = new ArrayList<>(); + // Flex transfer requests only use the WALK mode. + if (OTPFeature.FlexRouting.isOn()) { + flexTransferRequests.addAll( + transferRequests + .stream() + .filter(transferProfile -> transferProfile.journey().transfer().mode() == StreetMode.WALK) + .toList() + ); + } + stops .stream() .parallel() @@ -104,6 +116,7 @@ public void buildGraph() { LOG.debug("Linking stop '{}' {}", stop, ts0); + // Calculate default transfers. for (RouteRequest transferProfile : transferRequests) { StreetMode mode = transferProfile.journey().transfer().mode(); for (NearbyStop sd : nearbyStopFinder.findNearbyStops( @@ -119,28 +132,48 @@ public void buildGraph() { if (sd.stop.transfersNotAllowed()) { continue; } - createPathTransfer(distinctTransfers, sd, stop, mode); + TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges); + PathTransfer pathTransfer = distinctTransfers.get(transferKey); + if (pathTransfer == null) { + EnumSet modes = EnumSet.of(mode); + distinctTransfers.put( + transferKey, + new PathTransfer(stop, sd.stop, sd.distance, sd.edges, modes) + ); + } else { + pathTransfer.addMode(mode); + } } } - if (OTPFeature.FlexRouting.isOn()) { - for (RouteRequest transferProfile : transferRequests) { - StreetMode mode = transferProfile.journey().transfer().mode(); - // This code is for finding transfers from AreaStops to Stops, transfers - // from Stops to AreaStops and between Stops are already covered above. - for (NearbyStop sd : nearbyStopFinder.findNearbyStops( - ts0, - transferProfile, - transferProfile.journey().transfer(), - true - )) { - // Skip the origin stop, loop transfers are not needed. - if (sd.stop == stop) { - continue; - } - if (sd.stop instanceof RegularStop) { - continue; - } - createPathTransfer(distinctTransfers, sd, stop, mode); + // Calculate flex transfers if flex routing is enabled. + for (RouteRequest transferProfile : flexTransferRequests) { + StreetMode mode = transferProfile.journey().transfer().mode(); + // This code is for finding transfers from AreaStops to Stops, transfers + // from Stops to AreaStops and between Stops are already covered above. + for (NearbyStop sd : nearbyStopFinder.findNearbyStops( + ts0, + transferProfile, + transferProfile.journey().transfer(), + true + )) { + // Skip the origin stop, loop transfers are not needed. + if (sd.stop == stop) { + continue; + } + if (sd.stop instanceof RegularStop) { + continue; + } + // The TransferKey and PathTransfer are created differently for flex routing. + TransferKey transferKey = new TransferKey(sd.stop, stop, sd.edges); + PathTransfer pathTransfer = distinctTransfers.get(transferKey); + if (pathTransfer == null) { + EnumSet modes = EnumSet.of(mode); + distinctTransfers.put( + transferKey, + new PathTransfer(sd.stop, stop, sd.distance, sd.edges, modes) + ); + } else { + pathTransfer.addMode(mode); } } } @@ -189,28 +222,6 @@ public void buildGraph() { } } - /** - * Factory method for creating a PathTransfer. - */ - private void createPathTransfer( - Map distinctTransfers, - NearbyStop sd, - RegularStop stop, - StreetMode mode - ) { - TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges); - PathTransfer pathTransfer = distinctTransfers.get(transferKey); - if (pathTransfer == null) { - EnumSet modes = EnumSet.of(mode); - distinctTransfers.put( - transferKey, - new PathTransfer(stop, sd.stop, sd.distance, sd.edges, modes) - ); - } else { - pathTransfer.addMode(mode); - } - } - /** * Factory method for creating a NearbyStopFinder. Will create different finders depending on * whether the graph has a street network and if ConsiderPatternsForDirectTransfers feature is From 012f149376eb5ae5453a3c33e852665224bff1b5 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Mon, 9 Dec 2024 13:55:57 +0200 Subject: [PATCH 08/16] Add changes based on review comments. --- .../module/DirectTransferGenerator.java | 20 +++++++++++-------- .../opentripplanner/model/PathTransfer.java | 12 ++++++----- .../raptoradapter/transit/Transfer.java | 2 +- .../transit/service/TimetableRepository.java | 9 +++++++++ .../module/DirectTransferGeneratorTest.java | 18 +++-------------- .../raptoradapter/transit/TransferTest.java | 1 - 6 files changed, 32 insertions(+), 30 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index 19e2c8227d0..626c85312a7 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -135,19 +135,21 @@ public void buildGraph() { TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges); PathTransfer pathTransfer = distinctTransfers.get(transferKey); if (pathTransfer == null) { - EnumSet modes = EnumSet.of(mode); + // If the PathTransfer can't be found, it is created. distinctTransfers.put( transferKey, - new PathTransfer(stop, sd.stop, sd.distance, sd.edges, modes) + new PathTransfer(stop, sd.stop, sd.distance, sd.edges, EnumSet.of(mode)) ); } else { - pathTransfer.addMode(mode); + // If the PathTransfer is found, a new PathTransfer with the added mode is created. + distinctTransfers.put(transferKey, pathTransfer.withAddedMode(mode)); } } } // Calculate flex transfers if flex routing is enabled. for (RouteRequest transferProfile : flexTransferRequests) { - StreetMode mode = transferProfile.journey().transfer().mode(); + // Flex transfer requests only use the WALK mode. + StreetMode mode = StreetMode.WALK; // This code is for finding transfers from AreaStops to Stops, transfers // from Stops to AreaStops and between Stops are already covered above. for (NearbyStop sd : nearbyStopFinder.findNearbyStops( @@ -167,13 +169,14 @@ public void buildGraph() { TransferKey transferKey = new TransferKey(sd.stop, stop, sd.edges); PathTransfer pathTransfer = distinctTransfers.get(transferKey); if (pathTransfer == null) { - EnumSet modes = EnumSet.of(mode); + // If the PathTransfer can't be found, it is created. distinctTransfers.put( transferKey, - new PathTransfer(sd.stop, stop, sd.distance, sd.edges, modes) + new PathTransfer(sd.stop, stop, sd.distance, sd.edges, EnumSet.of(mode)) ); } else { - pathTransfer.addMode(mode); + // If the PathTransfer is found, a new PathTransfer with the added mode is created. + distinctTransfers.put(transferKey, pathTransfer.withAddedMode(mode)); } } } @@ -209,7 +212,8 @@ public void buildGraph() { for (StreetMode mode : transferRequests .stream() .map(transferProfile -> transferProfile.journey().transfer().mode()) - .collect(Collectors.toSet())) { + .distinct() + .toList()) { LOG.info( "Created {} transfers for mode {}.", transfersByStop diff --git a/application/src/main/java/org/opentripplanner/model/PathTransfer.java b/application/src/main/java/org/opentripplanner/model/PathTransfer.java index ed4696ab11c..a6b66548e64 100644 --- a/application/src/main/java/org/opentripplanner/model/PathTransfer.java +++ b/application/src/main/java/org/opentripplanner/model/PathTransfer.java @@ -28,7 +28,7 @@ public class PathTransfer implements Serializable { private final List edges; - private EnumSet modes; + private final EnumSet modes; public PathTransfer( StopLocation from, @@ -53,15 +53,17 @@ public double getDistanceMeters() { } public List getEdges() { - return this.edges; + return edges; } public EnumSet getModes() { - return this.modes; + return modes.clone(); } - public boolean addMode(StreetMode mode) { - return this.modes.add(mode); + public PathTransfer withAddedMode(StreetMode mode) { + EnumSet newModes = EnumSet.copyOf(modes); + newModes.add(mode); + return new PathTransfer(from, to, distanceMeters, edges, newModes); } @Override 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 6ee0dee83c0..2440321ab1f 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 @@ -75,7 +75,7 @@ public List getEdges() { } public EnumSet getModes() { - return modes; + return modes.clone(); } public Optional asRaptorTransfer(StreetSearchRequest request) { diff --git a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java index b81dd7d4f14..d83c3f9b996 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java @@ -35,6 +35,7 @@ import org.opentripplanner.model.transfer.DefaultTransferService; import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitLayer; import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.TransitLayerUpdater; +import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.routing.impl.DelegatingTransitAlertServiceImpl; import org.opentripplanner.routing.services.TransitAlertService; import org.opentripplanner.routing.util.ConcurrentPublished; @@ -433,6 +434,14 @@ public Collection getTransfersByStop(StopLocation stop) { return transfersByStop.get(stop); } + public Collection getTransfersByMode(StreetMode mode) { + return transfersByStop + .values() + .stream() + .filter(pathTransfer -> pathTransfer.getModes().contains(mode)) + .toList(); + } + public SiteRepository getSiteRepository() { return siteRepository; } diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java index 310e474ae99..482cb0afd13 100644 --- a/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java +++ b/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java @@ -274,21 +274,9 @@ public void testPathTransfersWithModesForMultipleRequestsWithPatterns() { ) .buildGraph(); - var walkTransfers = timetableRepository - .getAllPathTransfers() - .stream() - .filter(pathTransfer -> pathTransfer.getModes().contains(StreetMode.WALK)) - .toList(); - var bikeTransfers = timetableRepository - .getAllPathTransfers() - .stream() - .filter(pathTransfer -> pathTransfer.getModes().contains(StreetMode.BIKE)) - .toList(); - var carTransfers = timetableRepository - .getAllPathTransfers() - .stream() - .filter(pathTransfer -> pathTransfer.getModes().contains(StreetMode.CAR)) - .toList(); + var walkTransfers = timetableRepository.getTransfersByMode(StreetMode.WALK); + var bikeTransfers = timetableRepository.getTransfersByMode(StreetMode.BIKE); + var carTransfers = timetableRepository.getTransfersByMode(StreetMode.CAR); assertTransfers( walkTransfers, diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java index 7edda8fba9c..ebae06f5063 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TransferTest.java @@ -7,7 +7,6 @@ import java.util.EnumSet; import java.util.List; import java.util.Optional; -import java.util.Set; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.opentripplanner._support.geometry.Coordinates; From 984d34eeebfdacb8469e89705b83bfb10fd9965a Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Mon, 9 Dec 2024 14:33:37 +0200 Subject: [PATCH 09/16] Change how EnumSet is copied and make EnumSet immutable in Transfer. --- .../java/org/opentripplanner/model/PathTransfer.java | 2 +- .../algorithm/raptoradapter/transit/Transfer.java | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/model/PathTransfer.java b/application/src/main/java/org/opentripplanner/model/PathTransfer.java index a6b66548e64..01aa9af02f5 100644 --- a/application/src/main/java/org/opentripplanner/model/PathTransfer.java +++ b/application/src/main/java/org/opentripplanner/model/PathTransfer.java @@ -57,7 +57,7 @@ public List getEdges() { } public EnumSet getModes() { - return modes.clone(); + return EnumSet.copyOf(modes); } public PathTransfer withAddedMode(StreetMode mode) { 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 2440321ab1f..648343b9b0f 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 @@ -1,5 +1,7 @@ package org.opentripplanner.routing.algorithm.raptoradapter.transit; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; @@ -33,20 +35,20 @@ public class Transfer { private final List edges; - private final EnumSet modes; + private final ImmutableSet modes; public Transfer(int toStop, List edges, EnumSet modes) { this.toStop = toStop; this.edges = edges; this.distanceMeters = (int) edges.stream().mapToDouble(Edge::getDistanceMeters).sum(); - this.modes = modes; + this.modes = Sets.immutableEnumSet(modes); } public Transfer(int toStopIndex, int distanceMeters, EnumSet modes) { this.toStop = toStopIndex; this.distanceMeters = distanceMeters; this.edges = null; - this.modes = modes; + this.modes = Sets.immutableEnumSet(modes); } public List getCoordinates() { @@ -74,8 +76,8 @@ public List getEdges() { return edges; } - public EnumSet getModes() { - return modes.clone(); + public ImmutableSet getModes() { + return modes; } public Optional asRaptorTransfer(StreetSearchRequest request) { From 3ae6eedf3c2b006cbbb3bcd9ceb07a46e5c677f2 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Wed, 11 Dec 2024 16:40:38 +0200 Subject: [PATCH 10/16] Change logging in transfer calculations. --- .../module/DirectTransferGenerator.java | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index 626c85312a7..dcd56822a09 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -209,21 +209,17 @@ public void buildGraph() { nTransfersTotal, nLinkedStops ); - for (StreetMode mode : transferRequests + transferRequests .stream() .map(transferProfile -> transferProfile.journey().transfer().mode()) .distinct() - .toList()) { - LOG.info( - "Created {} transfers for mode {}.", - transfersByStop - .values() - .stream() - .filter(pathTransfer -> pathTransfer.getModes().contains(mode)) - .count(), - mode + .forEach(mode -> + LOG.info( + "Created {} transfers for mode {}.", + timetableRepository.getTransfersByMode(mode).size(), + mode + ) ); - } } /** From 4a01328ab31f1c49060d3df33c4b390d90cc3a01 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Thu, 12 Dec 2024 12:34:11 +0200 Subject: [PATCH 11/16] Refactor to use method. --- .../src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java | 4 +--- .../opentripplanner/transit/service/TimetableRepository.java | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java index ccc4f7fe554..73259fbc80e 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java @@ -30,9 +30,7 @@ public class FlexIndex { public FlexIndex(TimetableRepository timetableRepository) { // Flex transfers should only use WALK mode transfers. - StreetMode mode = StreetMode.WALK; - List filteredPathTransfers = timetableRepository.getAllPathTransfers().stream().filter(pathTransfer -> pathTransfer.getModes().contains(mode)).toList(); - for (PathTransfer transfer : filteredPathTransfers) { + for (PathTransfer transfer : timetableRepository.getTransfersByMode(StreetMode.WALK)) { transfersToStop.put(transfer.to, transfer); transfersFromStop.put(transfer.from, transfer); } diff --git a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java index d83c3f9b996..7ef6da379f1 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java @@ -16,6 +16,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -434,7 +435,7 @@ public Collection getTransfersByStop(StopLocation stop) { return transfersByStop.get(stop); } - public Collection getTransfersByMode(StreetMode mode) { + public List getTransfersByMode(StreetMode mode) { return transfersByStop .values() .stream() From 95e9ca770bac8f0751f951f3223ffb669d66baa4 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Thu, 12 Dec 2024 13:43:09 +0200 Subject: [PATCH 12/16] Rename getTransfersByMode to findTransfers. --- .../ext/java/org/opentripplanner/ext/flex/FlexIndex.java | 2 +- .../graph_builder/module/DirectTransferGenerator.java | 2 +- .../transit/service/TimetableRepository.java | 2 +- .../graph_builder/module/DirectTransferGeneratorTest.java | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java index 73259fbc80e..86d9766178f 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java @@ -30,7 +30,7 @@ public class FlexIndex { public FlexIndex(TimetableRepository timetableRepository) { // Flex transfers should only use WALK mode transfers. - for (PathTransfer transfer : timetableRepository.getTransfersByMode(StreetMode.WALK)) { + for (PathTransfer transfer : timetableRepository.findTransfers(StreetMode.WALK)) { transfersToStop.put(transfer.to, transfer); transfersFromStop.put(transfer.from, transfer); } diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java index dcd56822a09..37ac69d88ff 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/DirectTransferGenerator.java @@ -216,7 +216,7 @@ public void buildGraph() { .forEach(mode -> LOG.info( "Created {} transfers for mode {}.", - timetableRepository.getTransfersByMode(mode).size(), + timetableRepository.findTransfers(mode).size(), mode ) ); diff --git a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java index 7ef6da379f1..5c57724d90d 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java @@ -435,7 +435,7 @@ public Collection getTransfersByStop(StopLocation stop) { return transfersByStop.get(stop); } - public List getTransfersByMode(StreetMode mode) { + public List findTransfers(StreetMode mode) { return transfersByStop .values() .stream() diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java index 482cb0afd13..39d2f4b5684 100644 --- a/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java +++ b/application/src/test/java/org/opentripplanner/graph_builder/module/DirectTransferGeneratorTest.java @@ -274,9 +274,9 @@ public void testPathTransfersWithModesForMultipleRequestsWithPatterns() { ) .buildGraph(); - var walkTransfers = timetableRepository.getTransfersByMode(StreetMode.WALK); - var bikeTransfers = timetableRepository.getTransfersByMode(StreetMode.BIKE); - var carTransfers = timetableRepository.getTransfersByMode(StreetMode.CAR); + var walkTransfers = timetableRepository.findTransfers(StreetMode.WALK); + var bikeTransfers = timetableRepository.findTransfers(StreetMode.BIKE); + var carTransfers = timetableRepository.findTransfers(StreetMode.CAR); assertTransfers( walkTransfers, From df64c5a7f5ffc4453db5f74d21536770c27cb3ff Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 13 Dec 2024 16:03:01 +0200 Subject: [PATCH 13/16] Changes based on review comments and small optimization. --- .../org/opentripplanner/ext/flex/FlexIndex.java | 1 - .../raptoradapter/transit/RaptorTransferIndex.java | 5 ++--- .../algorithm/raptoradapter/transit/Transfer.java | 13 ++++++------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java index 86d9766178f..8097bd05c6e 100644 --- a/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java +++ b/application/src/ext/java/org/opentripplanner/ext/flex/FlexIndex.java @@ -5,7 +5,6 @@ import com.google.common.collect.Multimap; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import org.opentripplanner.ext.flex.trip.FlexTrip; import org.opentripplanner.model.PathTransfer; diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java index 941e1296838..8676e863911 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/RaptorTransferIndex.java @@ -33,7 +33,6 @@ public static RaptorTransferIndex create( StreetMode mode = request.mode(); for (int i = 0; i < transfersByStopIndex.size(); i++) { - forwardTransfers.add(new ArrayList<>()); reversedTransfers.add(new ArrayList<>()); } @@ -43,14 +42,14 @@ public static RaptorTransferIndex create( var transfers = transfersByStopIndex .get(fromStop) .stream() - .filter(transfer -> transfer.getModes().contains(mode)) + .filter(transfer -> transfer.allowsMode(mode)) .flatMap(s -> s.asRaptorTransfer(request).stream()) .collect( toMap(RaptorTransfer::stop, Function.identity(), (a, b) -> a.c1() < b.c1() ? a : b) ) .values(); - forwardTransfers.get(fromStop).addAll(transfers); + forwardTransfers.add(new ArrayList<>(transfers)); for (RaptorTransfer forwardTransfer : transfers) { reversedTransfers 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 648343b9b0f..7e66498f349 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 @@ -1,9 +1,8 @@ package org.opentripplanner.routing.algorithm.raptoradapter.transit; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Optional; @@ -35,20 +34,20 @@ public class Transfer { private final List edges; - private final ImmutableSet modes; + private final Set modes; public Transfer(int toStop, List edges, EnumSet modes) { this.toStop = toStop; this.edges = edges; this.distanceMeters = (int) edges.stream().mapToDouble(Edge::getDistanceMeters).sum(); - this.modes = Sets.immutableEnumSet(modes); + this.modes = Collections.unmodifiableSet(modes); } public Transfer(int toStopIndex, int distanceMeters, EnumSet modes) { this.toStop = toStopIndex; this.distanceMeters = distanceMeters; this.edges = null; - this.modes = Sets.immutableEnumSet(modes); + this.modes = Collections.unmodifiableSet(modes); } public List getCoordinates() { @@ -76,8 +75,8 @@ public List getEdges() { return edges; } - public ImmutableSet getModes() { - return modes; + public boolean allowsMode(StreetMode mode) { + return modes.contains(mode); } public Optional asRaptorTransfer(StreetSearchRequest request) { From 9b44df082e097cd13f591c89bbfc1633983a837a Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 13 Dec 2024 16:12:23 +0200 Subject: [PATCH 14/16] Add comments. --- .../src/main/java/org/opentripplanner/model/PathTransfer.java | 1 + .../org/opentripplanner/transit/service/TimetableRepository.java | 1 + 2 files changed, 2 insertions(+) diff --git a/application/src/main/java/org/opentripplanner/model/PathTransfer.java b/application/src/main/java/org/opentripplanner/model/PathTransfer.java index 01aa9af02f5..c4676fdf06f 100644 --- a/application/src/main/java/org/opentripplanner/model/PathTransfer.java +++ b/application/src/main/java/org/opentripplanner/model/PathTransfer.java @@ -60,6 +60,7 @@ public EnumSet getModes() { return EnumSet.copyOf(modes); } + /** Create a new PathTransfer based on the current one with the mode added to the valid modes. */ public PathTransfer withAddedMode(StreetMode mode) { EnumSet newModes = EnumSet.copyOf(modes); newModes.add(mode); diff --git a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java index 5c57724d90d..004391602ae 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java @@ -435,6 +435,7 @@ public Collection getTransfersByStop(StopLocation stop) { return transfersByStop.get(stop); } + /** Pre-generated transfers between all stops filtered based on the modes in the PathTransfer's mode field. */ public List findTransfers(StreetMode mode) { return transfersByStop .values() From c75b0a2dee17484a99bc8dc4fe2c3eeb86dbace2 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 13 Dec 2024 16:14:39 +0200 Subject: [PATCH 15/16] Fix spelling. --- .../opentripplanner/transit/service/TimetableRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java index 004391602ae..ff8607f3818 100644 --- a/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java +++ b/application/src/main/java/org/opentripplanner/transit/service/TimetableRepository.java @@ -435,7 +435,7 @@ public Collection getTransfersByStop(StopLocation stop) { return transfersByStop.get(stop); } - /** Pre-generated transfers between all stops filtered based on the modes in the PathTransfer's mode field. */ + /** Pre-generated transfers between all stops filtered based on the modes in the PathTransfer. */ public List findTransfers(StreetMode mode) { return transfersByStop .values() From 18215a995a15875e39a20ad29cbf6262bc6291a1 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Sun, 15 Dec 2024 18:03:46 +0200 Subject: [PATCH 16/16] Add comment. --- .../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 7e66498f349..2643067398e 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 @@ -75,6 +75,7 @@ public List getEdges() { return edges; } + /** Check if the given mode is a valid mode for the transfer. */ public boolean allowsMode(StreetMode mode) { return modes.contains(mode); }