-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add 'transferParametersForMode' build config field #6215
Changes from 4 commits
36336bf
f1380e3
2175e64
6418e26
e1e9bbb
89e617f
a1e0807
21bb307
573078a
3fd5bf5
35c98bc
3a34aa4
97238a3
41a0674
1a3f846
2e048f9
c90c3ec
30cc925
2e57c2f
f2296cd
0ac172e
bff77a5
9f5d63c
4f84804
4d599a2
2eb0912
57e03b4
7de4a39
8e87974
d0fbae9
f385061
afa1b16
c649ab7
91782ef
186d20c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,14 @@ | |
import com.google.common.collect.HashMultimap; | ||
import com.google.common.collect.Multimaps; | ||
import java.time.Duration; | ||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
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; | ||
|
@@ -17,10 +21,13 @@ | |
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.api.request.framework.DurationForEnum; | ||
import org.opentripplanner.routing.graph.Graph; | ||
import org.opentripplanner.routing.graphfinder.NearbyStop; | ||
import org.opentripplanner.street.model.edge.Edge; | ||
import org.opentripplanner.street.model.vertex.TransitStopVertex; | ||
import org.opentripplanner.street.model.vertex.Vertex; | ||
import org.opentripplanner.transit.model.site.RegularStop; | ||
import org.opentripplanner.transit.model.site.StopLocation; | ||
import org.opentripplanner.transit.service.DefaultTransitService; | ||
|
@@ -44,6 +51,7 @@ public class DirectTransferGenerator implements GraphBuilderModule { | |
private final Duration radiusByDuration; | ||
|
||
private final List<RouteRequest> transferRequests; | ||
private final DurationForEnum<StreetMode> carsAllowedStopMaxTransferDurationsForMode; | ||
private final Graph graph; | ||
private final TimetableRepository timetableRepository; | ||
private final DataImportIssueStore issueStore; | ||
|
@@ -60,6 +68,23 @@ public DirectTransferGenerator( | |
this.issueStore = issueStore; | ||
this.radiusByDuration = radiusByDuration; | ||
this.transferRequests = transferRequests; | ||
this.carsAllowedStopMaxTransferDurationsForMode = DurationForEnum.of(StreetMode.class).build(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should always include this with some default value that is the same as the normal transfer duration limit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has changed quite a lot, but you can look at how I handle an empty parameter |
||
} | ||
|
||
public DirectTransferGenerator( | ||
Graph graph, | ||
TimetableRepository timetableRepository, | ||
DataImportIssueStore issueStore, | ||
Duration radiusByDuration, | ||
List<RouteRequest> transferRequests, | ||
DurationForEnum<StreetMode> carsAllowedStopMaxTransferDurationsForMode | ||
) { | ||
this.graph = graph; | ||
this.timetableRepository = timetableRepository; | ||
this.issueStore = issueStore; | ||
this.radiusByDuration = radiusByDuration; | ||
this.transferRequests = transferRequests; | ||
this.carsAllowedStopMaxTransferDurationsForMode = carsAllowedStopMaxTransferDurationsForMode; | ||
} | ||
|
||
@Override | ||
|
@@ -68,9 +93,16 @@ public void buildGraph() { | |
timetableRepository.index(); | ||
|
||
/* The linker will use streets if they are available, or straight-line distance otherwise. */ | ||
NearbyStopFinder nearbyStopFinder = createNearbyStopFinder(); | ||
NearbyStopFinder nearbyStopFinder = createNearbyStopFinder(radiusByDuration, Set.of()); | ||
|
||
List<TransitStopVertex> stops = graph.getVerticesOfType(TransitStopVertex.class); | ||
Set<TransitStopVertex> carsAllowedStops = timetableRepository | ||
.getStopLocationsUsedForCarsAllowedTrips() | ||
.stream() | ||
.map(StopLocation::getId) | ||
.map(graph::getStopVertexForStopId) | ||
.filter(TransitStopVertex.class::isInstance) // filter out null values if no TransitStopVertex is found for ID | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inline comments are not allowed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
.collect(Collectors.toSet()); | ||
|
||
ProgressTracker progress = ProgressTracker.track( | ||
"Create transfer edges for stops", | ||
|
@@ -86,6 +118,37 @@ public void buildGraph() { | |
HashMultimap.create() | ||
); | ||
|
||
List<RouteRequest> filteredTransferRequests = new ArrayList<RouteRequest>(); | ||
List<RouteRequest> carsAllowedStopTransferRequests = new ArrayList<RouteRequest>(); | ||
HashMap<StreetMode, NearbyStopFinder> carsAllowedStopNearbyStopFinders = new HashMap<>(); | ||
|
||
// Split transfer requests into normal and carsAllowedStop requests. | ||
for (RouteRequest transferProfile : transferRequests) { | ||
StreetMode mode = transferProfile.journey().transfer().mode(); | ||
if (carsAllowedStopMaxTransferDurationsForMode.containsKey(mode)) { | ||
carsAllowedStopNearbyStopFinders.put( | ||
mode, | ||
createNearbyStopFinder( | ||
carsAllowedStopMaxTransferDurationsForMode.valueOf(mode), | ||
Collections.<Vertex>unmodifiableSet(carsAllowedStops) | ||
) | ||
); | ||
|
||
carsAllowedStopTransferRequests.add(transferProfile); | ||
// For bikes, also normal transfer requests are wanted. | ||
if (mode == StreetMode.BIKE) { | ||
filteredTransferRequests.add(transferProfile); | ||
} | ||
} else if (mode == StreetMode.CAR) { | ||
// Special transfers are always created for cars. | ||
// If a duration is not specified for cars, the default is used. | ||
carsAllowedStopNearbyStopFinders.put(mode, nearbyStopFinder); | ||
carsAllowedStopTransferRequests.add(transferProfile); | ||
} else { | ||
filteredTransferRequests.add(transferProfile); | ||
} | ||
} | ||
|
||
stops | ||
.stream() | ||
.parallel() | ||
|
@@ -101,26 +164,11 @@ public void buildGraph() { | |
|
||
LOG.debug("Linking stop '{}' {}", stop, ts0); | ||
|
||
for (RouteRequest transferProfile : transferRequests) { | ||
for (NearbyStop sd : nearbyStopFinder.findNearbyStops( | ||
ts0, | ||
transferProfile, | ||
transferProfile.journey().transfer(), | ||
false | ||
)) { | ||
// Skip the origin stop, loop transfers are not needed. | ||
if (sd.stop == stop) { | ||
continue; | ||
} | ||
if (sd.stop.transfersNotAllowed()) { | ||
continue; | ||
} | ||
distinctTransfers.put( | ||
new TransferKey(stop, sd.stop, sd.edges), | ||
new PathTransfer(stop, sd.stop, sd.distance, sd.edges) | ||
); | ||
} | ||
if (OTPFeature.FlexRouting.isOn()) { | ||
for (RouteRequest transferProfile : filteredTransferRequests) { | ||
findNearbyStops(nearbyStopFinder, ts0, transferProfile, stop, distinctTransfers); | ||
} | ||
if (OTPFeature.FlexRouting.isOn()) { | ||
for (RouteRequest transferProfile : filteredTransferRequests) { | ||
// 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( | ||
|
@@ -144,6 +192,22 @@ public void buildGraph() { | |
} | ||
} | ||
|
||
// This calculates transfers between stops that can use trips with cars. | ||
for (RouteRequest transferProfile : carsAllowedStopTransferRequests) { | ||
StreetMode mode = transferProfile.journey().transfer().mode(); | ||
if ( | ||
carsAllowedStops.contains(ts0) && carsAllowedStopNearbyStopFinders.containsKey(mode) | ||
) { | ||
findNearbyStops( | ||
carsAllowedStopNearbyStopFinders.get(mode), | ||
ts0, | ||
transferProfile, | ||
stop, | ||
distinctTransfers | ||
); | ||
} | ||
} | ||
|
||
LOG.debug( | ||
"Linked stop {} with {} transfers to stops with different patterns.", | ||
stop, | ||
|
@@ -179,7 +243,10 @@ public void buildGraph() { | |
* whether the graph has a street network and if ConsiderPatternsForDirectTransfers feature is | ||
* enabled. | ||
*/ | ||
private NearbyStopFinder createNearbyStopFinder() { | ||
private NearbyStopFinder createNearbyStopFinder( | ||
Duration radiusByDuration, | ||
Set<Vertex> findOnlyVertices | ||
) { | ||
var transitService = new DefaultTransitService(timetableRepository); | ||
NearbyStopFinder finder; | ||
if (!graph.hasStreets) { | ||
|
@@ -189,7 +256,7 @@ private NearbyStopFinder createNearbyStopFinder() { | |
finder = new StraightLineNearbyStopFinder(transitService, radiusByDuration); | ||
} else { | ||
LOG.info("Creating direct transfer edges between stops using the street network from OSM..."); | ||
finder = new StreetNearbyStopFinder(radiusByDuration, 0, null); | ||
finder = new StreetNearbyStopFinder(radiusByDuration, 0, null, Set.of(), findOnlyVertices); | ||
} | ||
|
||
if (OTPFeature.ConsiderPatternsForDirectTransfers.isOn()) { | ||
|
@@ -199,5 +266,32 @@ private NearbyStopFinder createNearbyStopFinder() { | |
} | ||
} | ||
|
||
private void findNearbyStops( | ||
NearbyStopFinder nearbyStopFinder, | ||
TransitStopVertex ts0, | ||
RouteRequest transferProfile, | ||
RegularStop stop, | ||
Map<TransferKey, PathTransfer> distinctTransfers | ||
) { | ||
for (NearbyStop sd : nearbyStopFinder.findNearbyStops( | ||
ts0, | ||
transferProfile, | ||
transferProfile.journey().transfer(), | ||
false | ||
)) { | ||
// Skip the origin stop, loop transfers are not needed. | ||
if (sd.stop == stop) { | ||
continue; | ||
} | ||
if (sd.stop.transfersNotAllowed()) { | ||
continue; | ||
} | ||
distinctTransfers.put( | ||
new TransferKey(stop, sd.stop, sd.edges), | ||
new PathTransfer(stop, sd.stop, sd.distance, sd.edges) | ||
); | ||
} | ||
} | ||
|
||
private record TransferKey(StopLocation source, StopLocation target, List<Edge> edges) {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this builder method wasn't introduced in this pr but this name is really confusing. Idk what we call Access+Transfer+Egress+Direct. It's difficult to come up with a name that doesn't confuse you into thinking that all modes are enabled. Just making this singular would maybe make it less confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to set each mode separately for more clarity