From 3a448f5da21536d4a58a4e9b06721ec13343ac8c Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 2 Mar 2021 12:13:21 -0500 Subject: [PATCH 01/32] Allow negative transfer time for debugging --- src/main/java/com/conveyal/r5/transit/path/StopSequence.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/transit/path/StopSequence.java b/src/main/java/com/conveyal/r5/transit/path/StopSequence.java index 9ad83105a..63753ef40 100644 --- a/src/main/java/com/conveyal/r5/transit/path/StopSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/StopSequence.java @@ -70,7 +70,7 @@ public int transferTime(PathResult.Iteration iteration) { } else { int transferTimeSeconds = iteration.totalTime - access.time - egress.time - iteration.waitTimes.sum() - rideTimesSeconds.sum(); - checkState(transferTimeSeconds >= 0); + // checkState(transferTimeSeconds >= 0); return transferTimeSeconds; } } From cd5058e00d402d60e0c2bfce889a5f314f2358fe Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 4 Mar 2021 15:46:11 -0500 Subject: [PATCH 02/32] Revert "Allow negative transfer time for debugging" This reverts commit 3a448f5da21536d4a58a4e9b06721ec13343ac8c. --- src/main/java/com/conveyal/r5/transit/path/StopSequence.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/transit/path/StopSequence.java b/src/main/java/com/conveyal/r5/transit/path/StopSequence.java index 63753ef40..9ad83105a 100644 --- a/src/main/java/com/conveyal/r5/transit/path/StopSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/StopSequence.java @@ -70,7 +70,7 @@ public int transferTime(PathResult.Iteration iteration) { } else { int transferTimeSeconds = iteration.totalTime - access.time - egress.time - iteration.waitTimes.sum() - rideTimesSeconds.sum(); - // checkState(transferTimeSeconds >= 0); + checkState(transferTimeSeconds >= 0); return transferTimeSeconds; } } From caa5db1a9ad19a91273034477795354ec4ed605b Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 4 Mar 2021 15:48:08 -0500 Subject: [PATCH 03/32] feat(paths): add assertion to single-point path requests Regional analyses that included path results implicitly checked that the components of travel time did not exceed the total travel time. This change adds such a check to single-point analyses that include path results. --- src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index 783eb5c21..fac5a5ece 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -155,6 +155,8 @@ public static class PathIterations { this.egress = pathTemplate.stopSequence.egress == null ? null : pathTemplate.stopSequence.egress.toString(); this.transitLegs = pathTemplate.transitLegs(transitLayer); this.iterations = iterations.stream().map(HumanReadableIteration::new).collect(Collectors.toList()); + iterations.forEach(pathTemplate.stopSequence::transferTime); // Sense check that travel time components + // do not exceed total travel time TODO report transfer times in single-point responses? } } From 607bc16ea1a29fbf2247a521d8196d6ed15e73de Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 4 Mar 2021 15:53:32 -0500 Subject: [PATCH 04/32] refactor(raptor): iterate over active scheduled trips instead of iterating over all trips in a pattern, and skipping over inactive and frequency-based trips in two different places --- .../conveyal/r5/profile/FastRaptorWorker.java | 17 ++++++----------- .../com/conveyal/r5/transit/TripPattern.java | 11 +++++++++++ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index eeee044bc..2cc2339a2 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -483,7 +483,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { patternIndex = patternsToExplore.nextSetBit(patternIndex + 1) ) { TripPattern pattern = transit.tripPatterns.get(patternIndex); - int onTrip = -1; + int onTrip = -1; // Used as index into list of active scheduled trips running on this pattern int waitTime = 0; int boardTime = 0; int boardStop = -1; @@ -512,14 +512,13 @@ private void doScheduledSearchForRound (RaptorState outputState) { pattern.pickups[stopPositionInPattern] != PickDropType.NONE ) { int earliestBoardTime = inputState.bestTimes[stop] + MINIMUM_BOARD_WAIT_SEC; + List candidateSchedules = pattern.activeScheduledTrips(servicesActive); if (onTrip == -1) { int candidateTripIndex = -1; - for (TripSchedule candidateSchedule : pattern.tripSchedules) { + for (TripSchedule candidateSchedule : candidateSchedules) { + // Iterate through schedules of active scheduled trips on this pattern, in ascending order + // of departure time from first stop of the pattern candidateTripIndex++; - if (!servicesActive.get(candidateSchedule.serviceCode) || candidateSchedule.headwaySeconds != null) { - // frequency trip or not running - continue; - } if (earliestBoardTime < candidateSchedule.departures[stopPositionInPattern]) { // board this trip (the earliest trip that can be boarded on this pattern at this stop) onTrip = candidateTripIndex; @@ -541,12 +540,8 @@ private void doScheduledSearchForRound (RaptorState outputState) { // The tripSchedules in a given pattern are sorted by time of departure from the first // stop. So they are sorted by time of departure at this stop, if the possibility // of overtaking is ignored. - TripSchedule earlierTripSchedule = pattern.tripSchedules.get(earlierTripIdx); + TripSchedule earlierTripSchedule = candidateSchedules.get(earlierTripIdx); - if (earlierTripSchedule.headwaySeconds != null || !servicesActive.get(earlierTripSchedule.serviceCode)) { - // This is a frequency trip or it is not running on the day of the search. - continue; - } // The assertion below is a sanity check, but not a complete check that all the // tripSchedules are sorted, because later tripSchedules are not considered. checkState(earlierTripSchedule.departures[0] <= schedule.departures[0], diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 88324db66..ccf78e8e2 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -226,4 +226,15 @@ public List getHopGeometries(TransitLayer transitLayer) { return geometries; } + /** + * Returns list of trips in this pattern that have a service code in the supplied set of service codes (e.g. for a + * particular date) and fully specified timetables (i.e. not frequency-based). + */ + public List activeScheduledTrips (BitSet servicesActive) { + return this.tripSchedules.stream().filter( + schedule -> servicesActive.get(schedule.serviceCode) && // Trip running on service day + schedule.headwaySeconds == null // Not a frequency trip + ).collect(Collectors.toList()); + } + } From 893114e11f11fc727dbdae2ed114e420cbf65c9d Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 4 Mar 2021 16:28:06 -0500 Subject: [PATCH 05/32] feat(overtaking): handle overtaking scheduled trips upstream of boarding stop The router should have passengers board the first departure from a given stop. Previously, boarding logic was based on the sorting order of trips within a pattern (by time of departure from the first stop) and the assumption that trips would not overtake each other upstream of the boarding stop. This commit relaxes that assumption for scheduled trips. --- .../conveyal/r5/profile/FastRaptorWorker.java | 32 +++++++++++++------ .../com/conveyal/r5/transit/TripPattern.java | 12 +++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index 2cc2339a2..bc41b4913 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -485,7 +485,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { TripPattern pattern = transit.tripPatterns.get(patternIndex); int onTrip = -1; // Used as index into list of active scheduled trips running on this pattern int waitTime = 0; - int boardTime = 0; + int boardTime = Integer.MAX_VALUE; int boardStop = -1; TripSchedule schedule = null; @@ -519,14 +519,21 @@ private void doScheduledSearchForRound (RaptorState outputState) { // Iterate through schedules of active scheduled trips on this pattern, in ascending order // of departure time from first stop of the pattern candidateTripIndex++; - if (earliestBoardTime < candidateSchedule.departures[stopPositionInPattern]) { - // board this trip (the earliest trip that can be boarded on this pattern at this stop) + + if (earliestBoardTime < candidateSchedule.departures[stopPositionInPattern] && + candidateSchedule.departures[stopPositionInPattern] < boardTime + ) { onTrip = candidateTripIndex; schedule = candidateSchedule; boardTime = candidateSchedule.departures[stopPositionInPattern]; waitTime = boardTime - inputState.bestTimes[stop]; boardStop = stop; - break; + // Check that all remaining trips depart this stop after the trip we boarded. + if (pattern.departuresInOrder(candidateSchedules.subList(candidateTripIndex, + candidateSchedules.size()), stopPositionInPattern)) { + // We are confident of being on the earliest feasible departure. + break; + } } } } else { @@ -534,12 +541,15 @@ private void doScheduledSearchForRound (RaptorState outputState) { // depart from this stop before this trip does, it might be preferable to board at this stop // instead. if (earliestBoardTime < schedule.departures[stopPositionInPattern]) { - // First, it might be possible to board an earlier trip at this stop. + // First, it might be possible to board an earlier trip at this stop. Iterate backwards + // through trips that depart the first stop of the pattern (and presumably this stop) + // before the trip we're on. int earlierTripIdx = onTrip; while (--earlierTripIdx >= 0) { // The tripSchedules in a given pattern are sorted by time of departure from the first - // stop. So they are sorted by time of departure at this stop, if the possibility - // of overtaking is ignored. + // stop of the pattern. For now, we assume no overtaking so the tripSchedules are + // also sorted by time of departure at this stop. The predicate for the break + // statement at the end of this loop handles overtaking. TripSchedule earlierTripSchedule = candidateSchedules.get(earlierTripIdx); // The assertion below is a sanity check, but not a complete check that all the @@ -556,8 +566,12 @@ private void doScheduledSearchForRound (RaptorState outputState) { boardStop = stop; } else { // The trip under consideration arrives at this stop earlier than one could feasibly - // board. Stop searching, because trips are sorted by departure time within a pattern. - break; + // board. Check that all remaining trips depart this stop at least as early. + if (pattern.departuresInOrder(candidateSchedules.subList(0, earlierTripIdx + 1), + stopPositionInPattern)) { + // We are confident of being on the earliest feasible departure. + break; + } } } // Second, if we care about paths or travel time components, check whether boarding at diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index ccf78e8e2..80e0ce214 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -132,6 +132,8 @@ public void setOrVerifyDirection (int directionId) { /** * Linear search. * @return null if no departure is possible. + * FIXME this is unused. And is active true by definition (this.servicesActive is a BitSet with serviceCode set for + * every one of this.tripSchedules)? */ TripSchedule findNextDeparture (int time, int stopOffset) { TripSchedule bestSchedule = null; @@ -237,4 +239,14 @@ public List activeScheduledTrips (BitSet servicesActive) { ).collect(Collectors.toList()); } + public boolean departuresInOrder(List tripsToCheck, int stopOffset) { + for (int i = 0; i < tripsToCheck.size() - 1; i++) { + if (tripsToCheck.get(i).departures[stopOffset] > tripsToCheck.get(i + 1).departures[stopOffset]) { + LOG.warn("Overtaking: route {}, pattern {}, upstream of stop {}", routeId, originalId, stopOffset); + return false; + } + } + return true; + } + } From c2926bd84f624538ad7be57d2681087ac35005d2 Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 4 Mar 2021 17:01:48 -0500 Subject: [PATCH 06/32] docs(overtaking): add javadoc --- src/main/java/com/conveyal/r5/transit/TripPattern.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 80e0ce214..89b43ce0f 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -239,6 +239,13 @@ public List activeScheduledTrips (BitSet servicesActive) { ).collect(Collectors.toList()); } + /** + * Checks whether departure times at a specified stop are in ascending order. We generally expect this to be true + * for trips in a pattern, because they are sorted by departure time from the first stop. But this is not + * guaranteed to be true, because a trip on a pattern could be overtaken by another. + * @param tripsToCheck caller is responsible for ensuring all are active on a given date + * @return true if the departure times at this stop are in ascending order, false otherwise + */ public boolean departuresInOrder(List tripsToCheck, int stopOffset) { for (int i = 0; i < tripsToCheck.size() - 1; i++) { if (tripsToCheck.get(i).departures[stopOffset] > tripsToCheck.get(i + 1).departures[stopOffset]) { From 17589155fd7ebc876edf85d18e9f75636188bbe8 Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 4 Mar 2021 17:33:49 -0500 Subject: [PATCH 07/32] docs(overtaking): switch log statement to debug level The warning gets noisy, even for a feed with only a couple overtaking trips --- src/main/java/com/conveyal/r5/transit/TripPattern.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 89b43ce0f..581b9fdf8 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -249,7 +249,7 @@ public List activeScheduledTrips (BitSet servicesActive) { public boolean departuresInOrder(List tripsToCheck, int stopOffset) { for (int i = 0; i < tripsToCheck.size() - 1; i++) { if (tripsToCheck.get(i).departures[stopOffset] > tripsToCheck.get(i + 1).departures[stopOffset]) { - LOG.warn("Overtaking: route {}, pattern {}, upstream of stop {}", routeId, originalId, stopOffset); + LOG.debug("Overtaking: route {}, pattern {}, upstream of stop {}", routeId, originalId, stopOffset); return false; } } From 0978929a337a192288c6a05366073f9203a6c78f Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 4 Mar 2021 22:17:43 -0500 Subject: [PATCH 08/32] fix(overtaking): replace loop with flag --- .../conveyal/r5/profile/FastRaptorWorker.java | 37 ++++++++----------- .../com/conveyal/r5/transit/TripPattern.java | 20 +--------- 2 files changed, 17 insertions(+), 40 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index bc41b4913..eb69510ca 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -529,46 +529,39 @@ private void doScheduledSearchForRound (RaptorState outputState) { waitTime = boardTime - inputState.bestTimes[stop]; boardStop = stop; // Check that all remaining trips depart this stop after the trip we boarded. - if (pattern.departuresInOrder(candidateSchedules.subList(candidateTripIndex, - candidateSchedules.size()), stopPositionInPattern)) { + if (pattern.noOvertakingConfirmed) { // We are confident of being on the earliest feasible departure. break; } } } } else { - // A specific trip on this pattern could be boarded at an upstream stop. If we are ready to - // depart from this stop before this trip does, it might be preferable to board at this stop - // instead. + // A specific trip on this pattern was boarded at an upstream stop in this scan. If we are + // ready to depart from this stop before this trip does, it might be preferable to board at + // this stop instead. if (earliestBoardTime < schedule.departures[stopPositionInPattern]) { - // First, it might be possible to board an earlier trip at this stop. Iterate backwards - // through trips that depart the first stop of the pattern (and presumably this stop) - // before the trip we're on. - int earlierTripIdx = onTrip; - while (--earlierTripIdx >= 0) { + // First, it might be possible to board an earlier trip at this stop. + int candidateTripIndex = pattern.noOvertakingConfirmed ? onTrip : candidateSchedules.size() - 1; + while (--candidateTripIndex >= 0) { // The tripSchedules in a given pattern are sorted by time of departure from the first // stop of the pattern. For now, we assume no overtaking so the tripSchedules are // also sorted by time of departure at this stop. The predicate for the break // statement at the end of this loop handles overtaking. - TripSchedule earlierTripSchedule = candidateSchedules.get(earlierTripIdx); + TripSchedule candidateSchedule = candidateSchedules.get(candidateTripIndex); - // The assertion below is a sanity check, but not a complete check that all the - // tripSchedules are sorted, because later tripSchedules are not considered. - checkState(earlierTripSchedule.departures[0] <= schedule.departures[0], - "Trip schedules not sorted by departure time at first stop of pattern"); - - if (earliestBoardTime < earlierTripSchedule.departures[stopPositionInPattern]) { + if (earliestBoardTime < candidateSchedule.departures[stopPositionInPattern] && + candidateSchedule.departures[stopPositionInPattern] < schedule.departures[stopPositionInPattern] + ) { // The trip under consideration can be boarded at this stop - onTrip = earlierTripIdx; - schedule = earlierTripSchedule; - boardTime = earlierTripSchedule.departures[stopPositionInPattern]; + onTrip = candidateTripIndex; + schedule = candidateSchedule; + boardTime = candidateSchedule.departures[stopPositionInPattern]; waitTime = boardTime - inputState.bestTimes[stop]; boardStop = stop; } else { // The trip under consideration arrives at this stop earlier than one could feasibly // board. Check that all remaining trips depart this stop at least as early. - if (pattern.departuresInOrder(candidateSchedules.subList(0, earlierTripIdx + 1), - stopPositionInPattern)) { + if (pattern.noOvertakingConfirmed) { // We are confident of being on the earliest feasible departure. break; } diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 581b9fdf8..26fd45755 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -63,6 +63,8 @@ public class TripPattern implements Serializable, Cloneable { /** does this trip pattern have any scheduled trips */ public boolean hasSchedules; + public boolean noOvertakingConfirmed = false; + // This set includes the numeric codes for all services on which at least one trip in this pattern is active. public BitSet servicesActive = new BitSet(); @@ -238,22 +240,4 @@ public List activeScheduledTrips (BitSet servicesActive) { schedule.headwaySeconds == null // Not a frequency trip ).collect(Collectors.toList()); } - - /** - * Checks whether departure times at a specified stop are in ascending order. We generally expect this to be true - * for trips in a pattern, because they are sorted by departure time from the first stop. But this is not - * guaranteed to be true, because a trip on a pattern could be overtaken by another. - * @param tripsToCheck caller is responsible for ensuring all are active on a given date - * @return true if the departure times at this stop are in ascending order, false otherwise - */ - public boolean departuresInOrder(List tripsToCheck, int stopOffset) { - for (int i = 0; i < tripsToCheck.size() - 1; i++) { - if (tripsToCheck.get(i).departures[stopOffset] > tripsToCheck.get(i + 1).departures[stopOffset]) { - LOG.debug("Overtaking: route {}, pattern {}, upstream of stop {}", routeId, originalId, stopOffset); - return false; - } - } - return true; - } - } From e401f14721c40f8b813423628e529624aa157207 Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 7 Mar 2021 12:01:40 -0500 Subject: [PATCH 09/32] perf(prefiltering): filter trips and cache on transit layer --- .../conveyal/r5/profile/FastRaptorWorker.java | 72 ++++++++----------- .../conveyal/r5/transit/FilteredPattern.java | 65 +++++++++++++++++ .../com/conveyal/r5/transit/TransitLayer.java | 59 +++++++++++++++ .../com/conveyal/r5/transit/TripPattern.java | 6 +- 4 files changed, 157 insertions(+), 45 deletions(-) create mode 100644 src/main/java/com/conveyal/r5/transit/FilteredPattern.java diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index eb69510ca..b952323eb 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -1,11 +1,9 @@ package com.conveyal.r5.profile; import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; -import com.conveyal.r5.api.util.TransitModes; +import com.conveyal.r5.transit.FilteredPattern; import com.conveyal.r5.transit.PickDropType; -import com.conveyal.r5.transit.RouteInfo; import com.conveyal.r5.transit.TransitLayer; -import com.conveyal.r5.transit.TripPattern; import com.conveyal.r5.transit.TripSchedule; import com.conveyal.r5.transit.path.Path; import gnu.trove.iterator.TIntIterator; @@ -111,12 +109,6 @@ public class FastRaptorWorker { /** The routing parameters. */ private final AnalysisWorkerTask request; - /** The indexes of the trip patterns running on a given day with frequency-based trips of selected modes. */ - private final BitSet runningFrequencyPatterns = new BitSet(); - - /** The indexes of the trip patterns running on a given day with scheduled trips of selected modes. */ - private final BitSet runningScheduledPatterns = new BitSet(); - /** Generates and stores departure time offsets for every frequency-based set of trips. */ private final FrequencyRandomOffsets offsets; @@ -171,7 +163,7 @@ public FastRaptorWorker (TransitLayer transitLayer, AnalysisWorkerTask request, */ public int[][] route () { raptorTimer.fullSearch.start(); - prefilterPatterns(); + filterPatternsAndTripsIfNeeded(); // Initialize result storage. Results are one arrival time at each stop, for every raptor iteration. final int nStops = transit.getStopCount(); final int nIterations = iterationsPerMinute * nMinutes; @@ -239,25 +231,21 @@ private void dumpAllTimesToFile(int[][] arrivalTimesAtStopsPerIteration, int max } /** - * Before routing, filter the set of patterns down to only the ones that are actually running on the search date. - * We can also filter down to only those modes enabled in the search request, because all trips in a pattern are - * defined to be on same route, and GTFS allows only one mode per route. + * Before routing, filter the set of patterns and trips to only the ones relevant for the search request (date + * and transit modes). We set the filtered patterns and trips on the transitLayer, in effect caching them for + * repeated searches on the same date with the same modes. */ - private void prefilterPatterns () { - for (int patternIndex = 0; patternIndex < transit.tripPatterns.size(); patternIndex++) { - TripPattern pattern = transit.tripPatterns.get(patternIndex); - RouteInfo routeInfo = transit.routes.get(pattern.routeIndex); - TransitModes mode = TransitLayer.getTransitModes(routeInfo.route_type); - if (pattern.servicesActive.intersects(servicesActive) && request.transitModes.contains(mode)) { - // At least one trip on this pattern is relevant, based on the profile request's date and modes. - if (pattern.hasFrequencies) { - runningFrequencyPatterns.set(patternIndex); - } - // Schedule case is not an "else" clause because we support patterns with both frequency and schedule. - if (pattern.hasSchedules) { - runningScheduledPatterns.set(patternIndex); - } - } + private void filterPatternsAndTripsIfNeeded() { + boolean performFiltering; + if (transit.filteredTripPatterns == null) { + // Trip filtering has not yet been performed. + performFiltering = true; + } else { + // Trip filtering was already performed, but the filtering criteria may have changed. + performFiltering = !transit.filteredTripPatterns.matchesRequest(request.transitModes, servicesActive); + } + if (performFiltering) { + transit.filterPatternsAndTrips(request.transitModes, servicesActive); } } @@ -477,12 +465,14 @@ private static Path[] pathToEachStop(RaptorState state) { */ private void doScheduledSearchForRound (RaptorState outputState) { final RaptorState inputState = outputState.previous; - BitSet patternsToExplore = patternsToExploreInNextRound(inputState, runningScheduledPatterns, true); + BitSet patternsToExplore = patternsToExploreInNextRound(inputState, + transit.filteredTripPatterns.runningScheduledPatterns, + true); for (int patternIndex = patternsToExplore.nextSetBit(0); patternIndex >= 0; patternIndex = patternsToExplore.nextSetBit(patternIndex + 1) ) { - TripPattern pattern = transit.tripPatterns.get(patternIndex); + FilteredPattern pattern = transit.filteredTripPatterns.patterns.get(patternIndex); int onTrip = -1; // Used as index into list of active scheduled trips running on this pattern int waitTime = 0; int boardTime = Integer.MAX_VALUE; @@ -512,7 +502,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { pattern.pickups[stopPositionInPattern] != PickDropType.NONE ) { int earliestBoardTime = inputState.bestTimes[stop] + MINIMUM_BOARD_WAIT_SEC; - List candidateSchedules = pattern.activeScheduledTrips(servicesActive); + List candidateSchedules = pattern.runningScheduledTrips; if (onTrip == -1) { int candidateTripIndex = -1; for (TripSchedule candidateSchedule : candidateSchedules) { @@ -529,7 +519,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { waitTime = boardTime - inputState.bestTimes[stop]; boardStop = stop; // Check that all remaining trips depart this stop after the trip we boarded. - if (pattern.noOvertakingConfirmed) { + if (pattern.noScheduledOvertaking) { // We are confident of being on the earliest feasible departure. break; } @@ -541,7 +531,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { // this stop instead. if (earliestBoardTime < schedule.departures[stopPositionInPattern]) { // First, it might be possible to board an earlier trip at this stop. - int candidateTripIndex = pattern.noOvertakingConfirmed ? onTrip : candidateSchedules.size() - 1; + int candidateTripIndex = pattern.noScheduledOvertaking ? onTrip : candidateSchedules.size() - 1; while (--candidateTripIndex >= 0) { // The tripSchedules in a given pattern are sorted by time of departure from the first // stop of the pattern. For now, we assume no overtaking so the tripSchedules are @@ -561,7 +551,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { } else { // The trip under consideration arrives at this stop earlier than one could feasibly // board. Check that all remaining trips depart this stop at least as early. - if (pattern.noOvertakingConfirmed) { + if (pattern.noScheduledOvertaking) { // We are confident of being on the earliest feasible departure. break; } @@ -618,21 +608,17 @@ private void doFrequencySearchForRound (RaptorState outputState, FrequencyBoardi // are applying randomized schedules that are not present in the accumulated range-raptor upper bound state. // Those randomized frequency routes may cascade improvements from updates made at previous departure minutes. final boolean withinMinute = (frequencyBoardingMode == UPPER_BOUND); - BitSet patternsToExplore = patternsToExploreInNextRound(inputState, runningFrequencyPatterns, withinMinute); + BitSet patternsToExplore = patternsToExploreInNextRound(inputState, + transit.filteredTripPatterns.runningFrequencyPatterns, + withinMinute); for (int patternIndex = patternsToExplore.nextSetBit(0); patternIndex >= 0; patternIndex = patternsToExplore.nextSetBit(patternIndex + 1) ) { - TripPattern pattern = transit.tripPatterns.get(patternIndex); - + FilteredPattern pattern = transit.filteredTripPatterns.patterns.get(patternIndex); int tripScheduleIndex = -1; // First loop iteration will immediately increment to 0. - for (TripSchedule schedule : pattern.tripSchedules) { + for (TripSchedule schedule : pattern.runningFrequencyTrips) { tripScheduleIndex++; - - // If this trip's service is inactive (it's not running) or it's a scheduled (non-freq) trip, skip it. - if (!servicesActive.get(schedule.serviceCode) || schedule.headwaySeconds == null) { - continue; - } // Loop through all the entries for this trip (time windows with service at a given frequency). for (int frequencyEntryIdx = 0; frequencyEntryIdx < schedule.headwaySeconds.length; diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java new file mode 100644 index 000000000..b92263a0d --- /dev/null +++ b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java @@ -0,0 +1,65 @@ +package com.conveyal.r5.transit; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.BitSet; +import java.util.List; + + +public class FilteredPattern extends TripPattern { + + private static Logger LOG = LoggerFactory.getLogger(FilteredPattern.class); + + /** Schedule-based (i.e. not frequency-based) trips running in a particular set of GTFS services */ + public List runningScheduledTrips = new ArrayList<>(); + + /** Frequency-based trips active in a particular set of GTFS services */ + public List runningFrequencyTrips = new ArrayList<>(); + + /** If no active schedule-based trip of this pattern overtakes another. */ + public boolean noScheduledOvertaking; + + /** + * Copy a TripPattern, ignoring fields not used in Raptor routing. The source pattern's trip schedules are + * filtered to exclude trips not active in the supplied set of services, then divided into separate + * scheduled and frequency trip lists. Finally, check the runningScheduledTrips for overtaking. + */ + public FilteredPattern(TripPattern source, BitSet servicesActive) { + this.originalId = source.originalId; + this.routeId = source.routeId; + this.stops = source.stops; + this.pickups = source.pickups; + this.dropoffs = source.dropoffs; + for (TripSchedule schedule : source.tripSchedules) { + if (servicesActive.get(schedule.serviceCode)) { + if (schedule.headwaySeconds == null) { + runningScheduledTrips.add(schedule); + } else { + runningFrequencyTrips.add(schedule); + } + } + } + // These could be set more strictly when looping over tripSchedules; just use the source pattern's values for + // now. + this.hasFrequencies = source.hasFrequencies; + this.hasSchedules = source.hasSchedules; + + // Check for overtaking + boolean noScheduledOvertaking = true; + loopOverStops: + for (int stopOffset = 0; stopOffset < source.stops.length; stopOffset++) { + for (int i = 0; i < runningScheduledTrips.size() - 1; i++) { + if (runningScheduledTrips.get(i).departures[stopOffset] > + runningScheduledTrips.get(i + 1).departures[stopOffset] + ) { + LOG.warn("Overtaking: route {}, pattern {}, upstream of stop {}", routeId, originalId, stopOffset); + noScheduledOvertaking = false; + break loopOverStops; + } + } + } + this.noScheduledOvertaking = noScheduledOvertaking; + } +} diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index dc53e7921..d91e63ccd 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -46,6 +46,7 @@ import java.util.BitSet; import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -104,6 +105,64 @@ public class TransitLayer implements Serializable, Cloneable { public List tripPatterns = new ArrayList<>(); + /** Stores the relevant patterns and trips based on the transit modes and date in an analysis request */ + public transient FilteredPatterns filteredTripPatterns; + + /** Associate filtered patterns with the criteria (modes and service) used to filter them */ + public static class FilteredPatterns { + /** + * Transit modes in the analysis request. We filter down to only those modes enabled in the search request, + * because all trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. + */ + private final EnumSet modes; + + /** GTFS services, e.g. active on a specific date */ + private final BitSet services; + + /** + * List with the same length and indexes as full tripPatterns. Patterns that do not meed the mode/services + * filtering criteria are recorded as null. + */ + public List patterns = new ArrayList<>(); + /** + * The indexes of the trip patterns running on a given day with frequency-based trips of selected modes. */ + public BitSet runningFrequencyPatterns = new BitSet(); + + /** The indexes of the trip patterns running on a given day with scheduled trips of selected modes. */ + public BitSet runningScheduledPatterns = new BitSet(); + + FilteredPatterns(EnumSet modes, BitSet services) { + this.modes = modes; + this.services = services; + } + + public boolean matchesRequest(EnumSet modes, BitSet services) { + return this.modes.equals(modes) && this.services.equals(services); + } + } + + public void filterPatternsAndTrips(EnumSet modes, BitSet services) { + this.filteredTripPatterns = new FilteredPatterns(modes, services); + for (int patternIndex = 0; patternIndex < this.tripPatterns.size(); patternIndex++) { + TripPattern pattern = this.tripPatterns.get(patternIndex); + RouteInfo routeInfo = this.routes.get(pattern.routeIndex); + TransitModes mode = TransitLayer.getTransitModes(routeInfo.route_type); + if (pattern.servicesActive.intersects(services) && modes.contains(mode)) { + this.filteredTripPatterns.patterns.add(new FilteredPattern(pattern, services)); + // At least one trip on this pattern is relevant, based on the profile request's date and modes. + if (pattern.hasFrequencies) { + this.filteredTripPatterns.runningFrequencyPatterns.set(patternIndex); + } + // Schedule case is not an "else" clause because we support patterns with both frequency and schedule. + if (pattern.hasSchedules) { + this.filteredTripPatterns.runningScheduledPatterns.set(patternIndex); + } + } else { + this.filteredTripPatterns.patterns.add(null); + } + } + } + // Maybe we need a StopStore that has (streetVertexForStop, transfers, flags, etc.) public TIntList streetVertexForStop = new TIntArrayList(); diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 26fd45755..8fc0980db 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -63,8 +63,6 @@ public class TripPattern implements Serializable, Cloneable { /** does this trip pattern have any scheduled trips */ public boolean hasSchedules; - public boolean noOvertakingConfirmed = false; - // This set includes the numeric codes for all services on which at least one trip in this pattern is active. public BitSet servicesActive = new BitSet(); @@ -109,6 +107,10 @@ public TripPattern(String routeId, Iterable stopTimes, TObjectIntMap Date: Sun, 7 Mar 2021 19:50:07 -0500 Subject: [PATCH 10/32] refactor(prefiltering): include time in overtaking log message --- src/main/java/com/conveyal/r5/transit/FilteredPattern.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java index b92263a0d..d827b3239 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java @@ -54,7 +54,8 @@ public FilteredPattern(TripPattern source, BitSet servicesActive) { if (runningScheduledTrips.get(i).departures[stopOffset] > runningScheduledTrips.get(i + 1).departures[stopOffset] ) { - LOG.warn("Overtaking: route {}, pattern {}, upstream of stop {}", routeId, originalId, stopOffset); + LOG.warn("Overtaking: route {} pattern {} stop #{} time {}", + routeId, originalId, stopOffset, runningScheduledTrips.get(i + 1).departures[stopOffset]); noScheduledOvertaking = false; break loopOverStops; } From dcc96f7911ba704182bb99d37defb73e6658b761 Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 7 Mar 2021 19:50:18 -0500 Subject: [PATCH 11/32] refactor(prefiltering): rm unused method --- .../java/com/conveyal/r5/transit/TripPattern.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 8fc0980db..672fa1ded 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -231,15 +231,4 @@ public List getHopGeometries(TransitLayer transitLayer) { } return geometries; } - - /** - * Returns list of trips in this pattern that have a service code in the supplied set of service codes (e.g. for a - * particular date) and fully specified timetables (i.e. not frequency-based). - */ - public List activeScheduledTrips (BitSet servicesActive) { - return this.tripSchedules.stream().filter( - schedule -> servicesActive.get(schedule.serviceCode) && // Trip running on service day - schedule.headwaySeconds == null // Not a frequency trip - ).collect(Collectors.toList()); - } } From 2b8c891e4a2755ad51f0ac40e6d991d0d851aa7f Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 14 Mar 2021 20:32:31 -0400 Subject: [PATCH 12/32] fix(overtaking): don't ignore last trip schedule in pattern --- src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index b952323eb..e1ad4ac2c 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -531,7 +531,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { // this stop instead. if (earliestBoardTime < schedule.departures[stopPositionInPattern]) { // First, it might be possible to board an earlier trip at this stop. - int candidateTripIndex = pattern.noScheduledOvertaking ? onTrip : candidateSchedules.size() - 1; + int candidateTripIndex = pattern.noScheduledOvertaking ? onTrip : candidateSchedules.size(); while (--candidateTripIndex >= 0) { // The tripSchedules in a given pattern are sorted by time of departure from the first // stop of the pattern. For now, we assume no overtaking so the tripSchedules are From d90a33f1b5122bbc9f5625a13bc7c20fa6fb89fb Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 14 Mar 2021 20:32:49 -0400 Subject: [PATCH 13/32] docs(overtaking): update comments --- .../conveyal/r5/profile/FastRaptorWorker.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index e1ad4ac2c..bea05834f 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -277,7 +277,10 @@ private void initializeScheduleState (int departureTime) { /** * Set the departure time in the scheduled search to the given departure time, and prepare for the scheduled * search at the next-earlier minute. This is reusing results from one departure time as an upper bound on - * arrival times for an earlier departure time (i.e. range raptor). + * arrival times for an earlier departure time (i.e. range raptor). Note that this reuse can give riders + * "look-ahead" abilities about trips that will be overtaken, depending on the departure time window; they will + * not board the first feasible departure from a stop if a later one (that has been ridden in a later departure + * minute) will arrive at their destination stop earlier. */ private void advanceScheduledSearchToPreviousMinute (int nextMinuteDepartureTime) { for (RaptorState state : this.scheduleState) { @@ -518,9 +521,10 @@ private void doScheduledSearchForRound (RaptorState outputState) { boardTime = candidateSchedule.departures[stopPositionInPattern]; waitTime = boardTime - inputState.bestTimes[stop]; boardStop = stop; - // Check that all remaining trips depart this stop after the trip we boarded. if (pattern.noScheduledOvertaking) { - // We are confident of being on the earliest feasible departure. + // All remaining candidateSchedules depart this stop after the one we are + // considering, because we are iterating in ascending order of departure time + // from the first stop of the pattern and there is no overtaking. break; } } @@ -531,11 +535,16 @@ private void doScheduledSearchForRound (RaptorState outputState) { // this stop instead. if (earliestBoardTime < schedule.departures[stopPositionInPattern]) { // First, it might be possible to board an earlier trip at this stop. + // If trips are well-sorted on this pattern (no overtaking), the only trips that need to be + // checked are the ones that depart the first stop of the pattern before the trip we are + // on does, and we can break out of the loop once the departures are too early to board. + // If there is overtaking, all candidate trip schedules need to be checked. int candidateTripIndex = pattern.noScheduledOvertaking ? onTrip : candidateSchedules.size(); + // Note decrement below (otherwise, we'd use candidateSchedules.size() - 1 above) while (--candidateTripIndex >= 0) { // The tripSchedules in a given pattern are sorted by time of departure from the first - // stop of the pattern. For now, we assume no overtaking so the tripSchedules are - // also sorted by time of departure at this stop. The predicate for the break + // stop of the pattern. In this first step, we assume no overtaking so the tripSchedules + // are also sorted by time of departure at this stop. The predicate for the break // statement at the end of this loop handles overtaking. TripSchedule candidateSchedule = candidateSchedules.get(candidateTripIndex); @@ -550,7 +559,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { boardStop = stop; } else { // The trip under consideration arrives at this stop earlier than one could feasibly - // board. Check that all remaining trips depart this stop at least as early. + // board. if (pattern.noScheduledOvertaking) { // We are confident of being on the earliest feasible departure. break; From 7f2f6456b1970fee0215f33315309c31d7bfd40e Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 14 Mar 2021 20:36:20 -0400 Subject: [PATCH 14/32] tests(grid-layout): factor out pointIndex calculation as suggested in existing TODO --- .../r5/analyst/network/GridLayout.java | 8 ++++ .../analyst/network/SimpsonDesertTests.java | 38 +++++-------------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java b/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java index c870d9dfc..6adf897a0 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java @@ -4,6 +4,7 @@ import com.conveyal.osmlib.OSM; import com.conveyal.r5.analyst.Grid; import com.conveyal.r5.analyst.scenario.Scenario; +import com.conveyal.r5.analyst.WebMercatorGridPointSet; import com.conveyal.r5.common.SphericalDistanceLibrary; import com.conveyal.r5.point_to_point.builder.TNBuilderConfig; import com.conveyal.r5.profile.StreetMode; @@ -194,6 +195,13 @@ public Grid makeRightHalfOpportunityDataset (double density) { return grid; } + public int pointIndex(AnalysisWorkerTask task, int x, int y) { + Coordinate destLatLon = this.getIntersectionLatLon(x, y); + // Here is a bit of awkwardness where WebMercatorGridPointSet and Grid both extend PointSet, but don't share + // their grid referencing code, so one would have to be converted to the other to get the point index. + return new WebMercatorGridPointSet(task.getWebMercatorExtents()).getPointIndexContaining(destLatLon); + } + public String nextIntegerId() { return Integer.toString(nextIntegerId++); } diff --git a/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java b/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java index 7f70e5e32..567c9dd57 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java +++ b/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java @@ -2,7 +2,6 @@ import com.conveyal.r5.OneOriginResult; import com.conveyal.r5.analyst.TravelTimeComputer; -import com.conveyal.r5.analyst.WebMercatorGridPointSet; import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; import com.conveyal.r5.analyst.cluster.TimeGridWriter; import com.conveyal.r5.analyst.cluster.TravelTimeResult; @@ -51,16 +50,8 @@ public void testGridScheduled () throws Exception { // Write travel times to Geotiff for debugging visualization in desktop GIS: // toGeotiff(oneOriginResult, task); - // TODO move this into reproducible method, perhaps on GridLayout - // Now to verify the results. We have only our 5 percentiles here, not the full list of travel times. - // They are also arranged on a grid. This grid does not match the full extents of the network, rather it - // matches the extents set in the task, which must exactly match those of the opportunity grid. - Coordinate destLatLon = gridLayout.getIntersectionLatLon(40, 40); - // Here is a bit of awkwardness where WebMercatorGridPointSet and Grid both extend PointSet, but don't share - // their grid referencing code, so one would have to be converted to the other to get the point index. - int pointIndex = new WebMercatorGridPointSet(task.getWebMercatorExtents()).getPointIndexContaining(destLatLon); - - int[] travelTimePercentiles = oneOriginResult.travelTimes.getTarget(pointIndex); + int destination = gridLayout.pointIndex(task, 40, 40); + int[] travelTimePercentiles = oneOriginResult.travelTimes.getTarget(destination); // Transit takes 30 seconds per block. Mean wait time is 10 minutes. Any trip takes one transfer. // 20+20 blocks at 30 seconds each = 20 minutes. Two waits at 0-20 minutes each, mean is 20 minutes. @@ -94,11 +85,8 @@ public void testGridFrequency () throws Exception { TravelTimeComputer computer = new TravelTimeComputer(task, network); OneOriginResult oneOriginResult = computer.computeTravelTimes(); - - // This should be factored out into a method eventually - Coordinate destLatLon = gridLayout.getIntersectionLatLon(40, 40); - int pointIndex = new WebMercatorGridPointSet(task.getWebMercatorExtents()).getPointIndexContaining(destLatLon); - int[] travelTimePercentiles = oneOriginResult.travelTimes.getTarget(pointIndex); + int destination = gridLayout.pointIndex(task, 40, 40); + int[] travelTimePercentiles = oneOriginResult.travelTimes.getTarget(destination); // Frequency travel time reasoning is similar to scheduled test method. // But transfer time is variable from 0...20 minutes. @@ -133,12 +121,8 @@ public void testGridFrequencyAlternatives () throws Exception { TravelTimeComputer computer = new TravelTimeComputer(task, network); OneOriginResult oneOriginResult = computer.computeTravelTimes(); - - // This should be factored out into a method eventually - Coordinate destLatLon = gridLayout.getIntersectionLatLon(40, 40); - int pointIndex = new WebMercatorGridPointSet(task.getWebMercatorExtents()).getPointIndexContaining(destLatLon); - int[] travelTimePercentiles = oneOriginResult.travelTimes.getTarget(pointIndex); - + int destination = gridLayout.pointIndex(task, 40, 40); + int[] travelTimePercentiles = oneOriginResult.travelTimes.getTarget(destination); // FIXME convolving new Distribution(2, 10) with itself and delaying 20 minutes is not the same // as convolving new Distribution(2, 10).delay(10) with itself, but it should be. @@ -149,7 +133,7 @@ public void testGridFrequencyAlternatives () throws Exception { Distribution twoAlternatives = Distribution.or(twoRideAsAndWalk, twoRideBsAndWalk).delay(3); // Compare expected and actual - Distribution observed = Distribution.fromTravelTimeResult(oneOriginResult.travelTimes, pointIndex); + Distribution observed = Distribution.fromTravelTimeResult(oneOriginResult.travelTimes, destination); twoAlternatives.assertSimilar(observed); DistributionTester.assertExpectedDistribution(twoAlternatives, travelTimePercentiles); @@ -175,12 +159,8 @@ public void testExperiments () throws Exception { .monteCarloDraws(4000) .build(); - TravelTimeComputer computer = new TravelTimeComputer(task, network); - OneOriginResult oneOriginResult = computer.computeTravelTimes(); - - // This should be factored out into a method eventually - Coordinate destLatLon = gridLayout.getIntersectionLatLon(80, 80); - int pointIndex = new WebMercatorGridPointSet(task.getWebMercatorExtents()).getPointIndexContaining(destLatLon); + OneOriginResult oneOriginResult = new TravelTimeComputer(task, network).computeTravelTimes(); + int pointIndex = gridLayout.pointIndex(task, 80, 80); int[] travelTimePercentiles = oneOriginResult.travelTimes.getTarget(pointIndex); // Each 60-block ride should take 30 minutes (across and up). From 5f44781fc4c03a63acc608c129b63ee479984b6b Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 14 Mar 2021 20:37:50 -0400 Subject: [PATCH 15/32] tests(overtaking): add unit test for proper handling of overtaking --- .../r5/analyst/cluster/PathResult.java | 2 +- .../analyst/network/DistributionTester.java | 9 +- .../r5/analyst/network/GridGtfsGenerator.java | 101 ++++++++++-------- .../r5/analyst/network/GridLayout.java | 22 +++- .../r5/analyst/network/GridRoute.java | 42 ++++++++ .../network/GridSinglePointTaskBuilder.java | 22 +++- .../analyst/network/SimpsonDesertTests.java | 75 ++++++++++++- 7 files changed, 219 insertions(+), 54 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index fac5a5ece..5e86a537b 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -163,7 +163,7 @@ public static class PathIterations { /** * Returns human-readable details of path iterations, for JSON representation (e.g. in the UI console). */ - List getPathIterationsForDestination() { + public List getPathIterationsForDestination() { checkState(iterationsForPathTemplates.length == 1, "Paths were stored for multiple " + "destinations, but only one is being requested"); List detailsForDestination = new ArrayList<>(); diff --git a/src/test/java/com/conveyal/r5/analyst/network/DistributionTester.java b/src/test/java/com/conveyal/r5/analyst/network/DistributionTester.java index 4629fa729..8d3d04e67 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/DistributionTester.java +++ b/src/test/java/com/conveyal/r5/analyst/network/DistributionTester.java @@ -32,4 +32,11 @@ public static void assertExpectedDistribution (Distribution expectedDistribution } } -} \ No newline at end of file + public static void assertEqualValues (int[] expected, int[] values) { + assertEquals(expected.length, values.length); + for (int i = 0; i < expected.length; i++) { + assertEquals(expected[i], values[i]); + } + } + +} diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java b/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java index f45ffe5e9..3619b7241 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java @@ -34,13 +34,13 @@ public class GridGtfsGenerator { private final boolean mergeStops; - public GridGtfsGenerator (GridLayout gridLayout) { + public GridGtfsGenerator(GridLayout gridLayout) { this.gridLayout = gridLayout; feed = new GTFSFeed(); // Temp file db, can we do this in memory instead? mergeStops = true; } - public GTFSFeed generate () { + public GTFSFeed generate() { for (GridRoute route : gridLayout.routes) { addRoute(route); } @@ -48,7 +48,7 @@ public GTFSFeed generate () { return feed; } - private void addCommonTables () { + private void addCommonTables() { Agency agency = new Agency(); agency.agency_id = AGENCY_ID; agency.agency_name = AGENCY_ID; @@ -63,7 +63,7 @@ private void addCommonTables () { feed.services.put(service.service_id, service); } - private void addRoute (GridRoute gridRoute) { + private void addRoute(GridRoute gridRoute) { Route route = new Route(); route.agency_id = AGENCY_ID; route.route_id = gridRoute.id; @@ -77,13 +77,13 @@ private void addRoute (GridRoute gridRoute) { } } - private void addRoute (GridRoute route, boolean back) { + private void addRoute(GridRoute route, boolean back) { addStopsForRoute(route, back); addTripsForRoute(route, back); } // If mergeStops is true, certain stops will be created multiple times but IDs will collide on insertion. - public void addStopsForRoute (GridRoute route, boolean back) { + public void addStopsForRoute(GridRoute route, boolean back) { for (int s = 0; s < route.nStops; s++) { String stopId = route.stopId(s, mergeStops); Stop stop = new Stop(); @@ -96,48 +96,61 @@ public void addStopsForRoute (GridRoute route, boolean back) { } } - public void addTripsForRoute (GridRoute route, boolean back) { - int tripIndex = 0; - int start = route.startHour * 60 * 60; - int end = route.endHour * 60 * 60; - int headway = route.headwayMinutes * 60; - int dwell = gridLayout.transitDwellSeconds; - int interstop = route.stopSpacingBlocks * gridLayout.transitBlockTraversalTimeSeconds; - // Maybe we should use exact_times = 1 instead of generating individual trips. - for (int intialDeparture = start; intialDeparture < end; intialDeparture += headway, tripIndex++) { - Trip trip = new Trip(); - trip.direction_id = back ? 1 : 0; - trip.trip_id = String.format("%s:%d:%d", route.id, tripIndex, trip.direction_id); - trip.route_id = route.id; - trip.service_id = SERVICE_ID; - feed.trips.put(trip.trip_id, trip); - int departureTime = intialDeparture; - int arrivalTime = departureTime - dwell; - for (int stopSequence = 0; stopSequence < route.nStops; stopSequence++) { - int stopInRoute = back ? route.nStops - 1 - stopSequence: stopSequence; - StopTime stopTime = new StopTime(); - stopTime.stop_id = route.stopId(stopInRoute, mergeStops); - stopTime.stop_sequence = stopSequence; - stopTime.arrival_time = arrivalTime; - stopTime.departure_time = departureTime; - stopTime.trip_id = trip.trip_id; - feed.stop_times.put(new Fun.Tuple2<>(stopTime.trip_id, stopTime.stop_sequence), stopTime); - arrivalTime += interstop; - departureTime += interstop; + public void addTripsForRoute(GridRoute route, boolean back) { + if (route.headwayMinutes > 0) { + int tripIndex = 0; + int start = route.startHour * 60 * 60; + int end = route.endHour * 60 * 60; + int headway = route.headwayMinutes * 60; + + // Maybe we should use exact_times = 1 instead of generating individual trips. + for (int startTime = start; startTime < end; startTime += headway, tripIndex++) { + String tripId = addTrip(route, back, startTime, tripIndex); + if (route.pureFrequency) { + Frequency frequency = new Frequency(); + frequency.start_time = start; + frequency.end_time = end; + frequency.headway_secs = headway; + frequency.exact_times = 0; + feed.frequencies.add(new Fun.Tuple2<>(tripId, frequency)); + // Do not make any additional trips, frequency entry represents them. + break; + } } - if (route.pureFrequency) { - Frequency frequency = new Frequency(); - frequency.start_time = start; - frequency.end_time = end; - frequency.headway_secs = headway; - frequency.exact_times = 0; - feed.frequencies.add(new Fun.Tuple2<>(trip.trip_id, frequency)); - // Do not make any additional trips, frequency entry represents them. - break; + } else { + for (int i = 0; i < route.startTimes.length; i++ ) { + addTrip(route, back, route.startTimes[i], i); } } + } - + private String addTrip(GridRoute route, boolean back, int startTime, int tripIndex) { + Trip trip = new Trip(); + trip.direction_id = back ? 1 : 0; + String tripId = String.format("%s:%d:%d", route.id, tripIndex, trip.direction_id); + trip.trip_id = tripId; + trip.route_id = route.id; + trip.service_id = SERVICE_ID; + feed.trips.put(trip.trip_id,trip); + int dwell = gridLayout.transitDwellSeconds; + int departureTime = startTime; + int arrivalTime = departureTime - dwell; + for (int stopSequence = 0; stopSequence < route.nStops; stopSequence++) { + int stopInRoute = back ? route.nStops - 1 - stopSequence : stopSequence; + StopTime stopTime = new StopTime(); + stopTime.stop_id = route.stopId(stopInRoute, mergeStops); + stopTime.stop_sequence = stopSequence; + stopTime.arrival_time = arrivalTime; + stopTime.departure_time = departureTime; + stopTime.trip_id = tripId; + feed.stop_times.put(new Fun.Tuple2<>(tripId, stopTime.stop_sequence), stopTime); + if (stopSequence < route.nStops - 1) { + int hopTime = (int) route.hopTime(new GridRoute.TripHop(tripIndex, stopSequence)); + arrivalTime += hopTime; + departureTime += hopTime; + } + } + return tripId; } } diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java b/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java index 6adf897a0..c7adbc883 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java @@ -2,9 +2,13 @@ import com.conveyal.gtfs.GTFSFeed; import com.conveyal.osmlib.OSM; +import com.conveyal.r5.OneOriginResult; import com.conveyal.r5.analyst.Grid; -import com.conveyal.r5.analyst.scenario.Scenario; +import com.conveyal.r5.analyst.TravelTimeComputer; import com.conveyal.r5.analyst.WebMercatorGridPointSet; +import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; +import com.conveyal.r5.analyst.cluster.PathResult; +import com.conveyal.r5.analyst.cluster.TimeGridWriter; import com.conveyal.r5.common.SphericalDistanceLibrary; import com.conveyal.r5.point_to_point.builder.TNBuilderConfig; import com.conveyal.r5.profile.StreetMode; @@ -13,11 +17,10 @@ import org.locationtech.jts.geom.CoordinateXY; import org.locationtech.jts.geom.Envelope; +import java.io.FileOutputStream; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.UUID; import java.util.stream.Stream; @@ -25,7 +28,7 @@ /** * This is used in testing, to represent and create gridded transport systems with very regular spacing of roads and - * transit stops, yieling highly predictable travel times that can be tested against actual output from the router. + * transit stops, yielding highly predictable travel times that can be tested against actual output from the router. * * A square grid of points is established to the north and east of a specified origin point, and roads are constructed * running horizontally and vertically through the points. Each grid point is then a road intersection. These grid @@ -146,6 +149,13 @@ public void addHorizontalRoute (int row, int headwayMinutes) { this.routes.add(GridRoute.newHorizontalRoute(this, row, headwayMinutes)); } + /** Add an east-west route at the given row of the grid, running at the default speed. Explicit schedules must be + set separately via startTimes * */ + public void addHorizontalRoute (int row) { + this.routes.add(GridRoute.newHorizontalRoute(this, row, -1)); + } + + /** Add a north-south route at the given column of the grid, running at the default speed and the given headway. */ public void addVerticalRoute (int col, int headwayMinutes) { this.routes.add(GridRoute.newVerticalRoute(this, col, headwayMinutes)); @@ -165,6 +175,10 @@ public GridSinglePointTaskBuilder newTaskBuilder() { return new GridSinglePointTaskBuilder(this); } + public GridSinglePointTaskBuilder copyTask(AnalysisWorkerTask task) { + return new GridSinglePointTaskBuilder(this, task); + } + /** Get the minimum envelope containing all the points in this grid. */ public Envelope gridEnvelope () { Coordinate farCorner = getIntersectionLatLon(widthAndHeightInBlocks, widthAndHeightInBlocks); diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridRoute.java b/src/test/java/com/conveyal/r5/analyst/network/GridRoute.java index 6560533c3..d269a7ed7 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridRoute.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridRoute.java @@ -1,5 +1,7 @@ package com.conveyal.r5.analyst.network; +import java.util.Map; +import java.util.Objects; import java.util.stream.Stream; /** @@ -18,6 +20,12 @@ public class GridRoute { public boolean bidirectional; public int startHour; public int endHour; + /** Explicit departure times from first stop; if set, startHour and endHour will be ignored*/ + public int[] startTimes; + /** + * Override default hop times. Map of (trip, stopAtStartOfHop) to factor by which default hop is multiplied + */ + public Map hopTimeScaling; public int headwayMinutes; public boolean pureFrequency; @@ -65,6 +73,16 @@ public String stopId (int stop, boolean mergeStops) { } } + public double hopTime (TripHop tripHop) { + double scale; + if (hopTimeScaling != null && hopTimeScaling.get(tripHop) != null) { + scale = hopTimeScaling.get(tripHop); + } else { + scale = 1; + } + return scale * stopSpacingBlocks * gridLayout.transitBlockTraversalTimeSeconds; + } + private static GridRoute newBareRoute (GridLayout gridLayout, int headwayMinutes) { GridRoute route = new GridRoute(); route.id = gridLayout.nextIntegerId(); // Avoid collisions when same route is added multiple times @@ -101,4 +119,28 @@ public GridRoute pureFrequency () { return this; } + public static class TripHop{ + int trip; + int hop; + + public TripHop(int trip, int hop){ + this.trip = trip; + this.hop = hop; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + TripHop tripHop = (TripHop) o; + return trip == tripHop.trip && + hop == tripHop.hop; + } + + @Override + public int hashCode() { + return Objects.hash(trip, hop); + } + } + } diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java b/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java index 5f4c7b068..97dcba40b 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java @@ -3,7 +3,6 @@ import com.conveyal.r5.analyst.FreeFormPointSet; import com.conveyal.r5.analyst.Grid; import com.conveyal.r5.analyst.PointSet; -import com.conveyal.r5.analyst.WebMercatorExtents; import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; import com.conveyal.r5.analyst.cluster.TravelTimeSurfaceTask; import com.conveyal.r5.analyst.decay.StepDecayFunction; @@ -11,12 +10,10 @@ import com.conveyal.r5.api.util.TransitModes; import org.locationtech.jts.geom.Coordinate; -import java.time.LocalDate; import java.time.LocalTime; import java.util.EnumSet; import java.util.stream.IntStream; -import static com.conveyal.r5.analyst.WebMercatorGridPointSet.DEFAULT_ZOOM; import static com.conveyal.r5.analyst.network.GridGtfsGenerator.GTFS_DATE; /** @@ -59,6 +56,11 @@ public GridSinglePointTaskBuilder (GridLayout gridLayout) { task.recordTravelTimeHistograms = true; } + public GridSinglePointTaskBuilder (GridLayout layout, AnalysisWorkerTask task) { + this.gridLayout = layout; + this.task = task.clone(); + } + public GridSinglePointTaskBuilder setOrigin (int gridX, int gridY) { Coordinate origin = gridLayout.getIntersectionLatLon(gridX, gridY); task.fromLat = origin.y; @@ -70,6 +72,9 @@ public GridSinglePointTaskBuilder setDestination (int gridX, int gridY) { Coordinate destination = gridLayout.getIntersectionLatLon(gridX, gridY); task.destinationPointSets = new PointSet[] { new FreeFormPointSet(destination) }; task.destinationPointSetKeys = new String[] { "ID" }; + task.toLat = destination.y; + task.toLon = destination.x; + task.includePathResults = true; return this; } @@ -79,6 +84,17 @@ public GridSinglePointTaskBuilder morningPeak () { return this; } + public GridSinglePointTaskBuilder departureTimeWindow(int startHour, int startMinute, int durationMinutes) { + task.fromTime = LocalTime.of(startHour, startMinute).toSecondOfDay(); + task.toTime = LocalTime.of(startHour, startMinute + durationMinutes).toSecondOfDay(); + return this; + } + + public GridSinglePointTaskBuilder maxRides(int rides) { + task.maxRides = rides; + return this; + } + public GridSinglePointTaskBuilder uniformOpportunityDensity (double density) { Grid grid = gridLayout.makeUniformOpportunityDataset(density); task.destinationPointSets = new PointSet[] { grid }; diff --git a/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java b/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java index 567c9dd57..f6ba2c8d4 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java +++ b/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java @@ -3,14 +3,18 @@ import com.conveyal.r5.OneOriginResult; import com.conveyal.r5.analyst.TravelTimeComputer; import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; +import com.conveyal.r5.analyst.cluster.PathResult; import com.conveyal.r5.analyst.cluster.TimeGridWriter; -import com.conveyal.r5.analyst.cluster.TravelTimeResult; import com.conveyal.r5.transit.TransportNetwork; import org.junit.jupiter.api.Test; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.CoordinateXY; import java.io.FileOutputStream; +import java.time.LocalTime; +import java.util.HashMap; +import java.util.List; +import java.util.Map; /** * This is a collection of tests using roads and transit lines laid out in a large perfect grid in the desert. @@ -139,6 +143,75 @@ public void testGridFrequencyAlternatives () throws Exception { DistributionTester.assertExpectedDistribution(twoAlternatives, travelTimePercentiles); } + /** + * Test that the router correctly handles overtaking trips on the same route. Consider Trip A and Trip B, on a + * horizontal route where each hop time is 30 seconds, except when Trip A slows to 10 minutes/hop for hops 20 and + * 21. If Trip A leaves the first stop at 7:10 and Trip B leaves the first stop at 7:20, Trip A runs 10 minutes + * ahead of Trip B until stop 20, is passed around stop 21, and runs 9 minutes behind Trip B for the remaining + * stops. This example is tested with 3 cases for a rider traveling to stop 42 (well downstream of the overtaking + * point): + * + * 1. Standard rider, departing stop 30 between 7:00 and 7:05, who always rides Trip B (the earliest feasible + * departure from that stop) + * 2. Naive rider, departing stop 10 between 7:00 and 7:05, who always rides Trip A (the earliest feasible + * departure from that stop) + * 3. Savvy rider, departing stop 10 between 7:13 and 7:18, who always rides Trip B (avoiding boarding the + * slower Trip A thanks to the "look-ahead" abilities when ENABLE_OPTIMIZATION_RANGE_RAPTOR is true) + */ + @Test + public void testOvertakingCases () throws Exception { + GridLayout gridLayout = new GridLayout(SIMPSON_DESERT_CORNER, 100); + gridLayout.addHorizontalRoute(50); + gridLayout.routes.get(0).startTimes = new int[] { + LocalTime.of(7, 10).toSecondOfDay(), // Trip A + LocalTime.of(7, 20).toSecondOfDay() // Trip B + }; + Map hopTimeScaling = new HashMap<>(); + // Trip A slows down from stop 20 to 22, allowing Trip B to overtake it. + hopTimeScaling.put(new GridRoute.TripHop(0, 20), 20.0); + hopTimeScaling.put(new GridRoute.TripHop(0, 21), 20.0); + gridLayout.routes.get(0).hopTimeScaling = hopTimeScaling; + TransportNetwork network = gridLayout.generateNetwork(); + + // 1. Standard rider: upstream overtaking means Trip B departs origin first and is fastest to destination. + AnalysisWorkerTask standardRider = gridLayout.newTaskBuilder() + .departureTimeWindow(7, 0, 5) + .maxRides(1) + .setOrigin(30, 50) + .setDestination(42, 50) + .uniformOpportunityDensity(10) + .build(); + + OneOriginResult standardResult = new TravelTimeComputer(standardRider, network).computeTravelTimes(); + List standardPaths = standardResult.paths.getPathIterationsForDestination(); + int[] standardTimes = standardPaths.get(0).iterations.stream().mapToInt(i -> (int) i.totalTime).toArray(); + // Trip B departs stop 30 at 7:35. So 30-35 minute wait, plus ~5 minute ride and ~5 minute egress leg + DistributionTester.assertEqualValues(new int[]{45, 44, 43, 42, 41}, standardTimes); + + // 2. Naive rider: downstream overtaking means Trip A departs origin first but is not fastest to destination. + AnalysisWorkerTask naiveRider = gridLayout.copyTask(standardRider) + .setOrigin(10, 50) + .build(); + + OneOriginResult naiveResult = new TravelTimeComputer(naiveRider, network).computeTravelTimes(); + List naivePaths = naiveResult.paths.getPathIterationsForDestination(); + int[] naiveTimes = naivePaths.get(0).iterations.stream().mapToInt(i -> (int) i.totalTime).toArray(); + // Trip A departs stop 10 at 7:15. So 10-15 minute wait, plus ~35 minute ride and ~5 minute egress leg + DistributionTester.assertEqualValues(new int[]{54, 53, 52, 51, 50}, naiveTimes); + + // 3. Savvy rider (look-ahead abilities from starting the trip 13 minutes later): waits to board Trip B, even + // when boarding Trip A is possible + AnalysisWorkerTask savvyRider = gridLayout.copyTask(naiveRider) + .departureTimeWindow(7, 13, 5) + .build(); + + OneOriginResult savvyResult = new TravelTimeComputer(savvyRider, network).computeTravelTimes(); + List savvyPaths = savvyResult.paths.getPathIterationsForDestination(); + int[] savvyTimes = savvyPaths.get(0).iterations.stream().mapToInt(i -> (int) i.totalTime).toArray(); + // Trip B departs stop 10 at 7:25. So 8-12 minute wait, plus ~16 minute ride and ~5 minute egress leg + DistributionTester.assertEqualValues(new int[]{32, 31, 30, 29, 28}, savvyTimes); + } + /** * Experiments */ From 479726143d0869c05d503d3ff5ff360282b2b7bb Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Tue, 13 Apr 2021 11:56:03 +0800 Subject: [PATCH 16/32] thread-aware method for caching filtered patterns also augmented some Javadoc --- .../conveyal/r5/profile/FastRaptorWorker.java | 71 ++++++---------- .../conveyal/r5/transit/FilteredPatterns.java | 74 ++++++++++++++++ .../com/conveyal/r5/transit/TransitLayer.java | 84 ++++++------------- 3 files changed, 128 insertions(+), 101 deletions(-) create mode 100644 src/main/java/com/conveyal/r5/transit/FilteredPatterns.java diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index bea05834f..b4fe8b912 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -2,6 +2,7 @@ import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; import com.conveyal.r5.transit.FilteredPattern; +import com.conveyal.r5.transit.FilteredPatterns; import com.conveyal.r5.transit.PickDropType; import com.conveyal.r5.transit.TransitLayer; import com.conveyal.r5.transit.TripSchedule; @@ -25,11 +26,11 @@ import static com.google.common.base.Preconditions.checkState; /** - * FastRaptorWorker is faster than the old RaptorWorker and made to be more maintainable. - * It is simpler, as it only focuses on the transit network; see the Propagater class for the methods that extend - * the travel times from the final transit stop of a trip out to the individual targets. - * - * The algorithm used herein is described in + * FastRaptorWorker finds stop-to-stop paths through the transit network. + * The PerTargetPropagater extends travel times from the final transit stop of trips out to the individual targets. + * This system also accounts for pure-frequency routes by using Monte Carlo methods (generating randomized schedules). + * There is support for saving paths, so we can report how to reach a destination rather than just how long it takes. + * The algorithm used herein is described in: * * Conway, Matthew Wigginton, Andrew Byrd, and Marco van der Linden. “Evidence-Based Transit and Land Use Sketch Planning * Using Interactive Accessibility Methods on Combined Schedule and Headway-Based Networks.” Transportation Research @@ -38,22 +39,15 @@ * Delling, Daniel, Thomas Pajor, and Renato Werneck. “Round-Based Public Transit Routing,” January 1, 2012. * http://research.microsoft.com/pubs/156567/raptor_alenex.pdf. * - * There is basic support for saving paths, so we can report how to reach a destination rather than just how long it takes. - * - * This class originated as a rewrite of our RAPTOR code that would use "thin workers", allowing computation by a - * generic function-execution service like AWS Lambda. The gains in efficiency were significant enough that this is now - * the way we do all analysis work. This system also accounts for pure-frequency routes by using Monte Carlo methods - * (generating randomized schedules). * - * TODO rename to remove "fast" and revise above comments, there is only one worker now. - * Maybe just call it TransitRouter. But then there's also McRaptor. + * TODO rename to remove "fast". Maybe just call it TransitRouter, but then there's also McRaptor. */ public class FastRaptorWorker { private static final Logger LOG = LoggerFactory.getLogger(FastRaptorWorker.class); /** - * This value essentially serves as Infinity for ints - it's bigger than every other number. + * This value is used as positive infinity for ints - it's bigger than every other number. * It is the travel time to a transit stop or a target before that stop or target is ever reached. * Be careful when propagating travel times from stops to targets, adding anything to UNREACHED will cause overflow. */ @@ -68,7 +62,7 @@ public class FastRaptorWorker { public static final int SECONDS_PER_MINUTE = 60; /** - * Step for departure times. Use caution when changing this as the functions request.getTimeWindowLengthMinutes + * Step for departure times. Use caution when changing this, as the functions request.getTimeWindowLengthMinutes * and request.getMonteCarloDrawsPerMinute below which assume this value is 1 minute. */ private static final int DEPARTURE_STEP_SEC = 60; @@ -80,6 +74,7 @@ public class FastRaptorWorker { private static final int MINIMUM_BOARD_WAIT_SEC = 60; // ENABLE_OPTIMIZATION_X flags enable code paths that should affect efficiency but have no effect on output. + // They may change results where our algorithm is not perfectly optimal, for example with respect to overtaking. public static final boolean ENABLE_OPTIMIZATION_RANGE_RAPTOR = true; public static final boolean ENABLE_OPTIMIZATION_FREQ_UPPER_BOUND = true; @@ -112,9 +107,12 @@ public class FastRaptorWorker { /** Generates and stores departure time offsets for every frequency-based set of trips. */ private final FrequencyRandomOffsets offsets; - /** Services active on the date of the search */ + /** Services active on the date of the search. */ private final BitSet servicesActive; + /** TripPatterns that have been prefiltered for the specific search date and modes. */ + private FilteredPatterns filteredPatterns; + /** * The state resulting from the scheduled search at a particular departure minute. * This state is reused at each departure minute without re-initializing it (this is the range-raptor optimization). @@ -134,6 +132,10 @@ public class FastRaptorWorker { /** If we're going to store paths to every destination (e.g. for static sites) then they'll be retained here. */ public List pathsPerIteration; + /** + * Only fast initialization steps are performed in the constructor. + * All slower work is done in route() so timing information can be collected. + */ public FastRaptorWorker (TransitLayer transitLayer, AnalysisWorkerTask request, TIntIntMap accessStops) { this.transit = transitLayer; this.request = request; @@ -163,7 +165,7 @@ public FastRaptorWorker (TransitLayer transitLayer, AnalysisWorkerTask request, */ public int[][] route () { raptorTimer.fullSearch.start(); - filterPatternsAndTripsIfNeeded(); + filteredPatterns = transit.getFilteredPatterns(request.transitModes, servicesActive); // Initialize result storage. Results are one arrival time at each stop, for every raptor iteration. final int nStops = transit.getStopCount(); final int nIterations = iterationsPerMinute * nMinutes; @@ -230,25 +232,6 @@ private void dumpAllTimesToFile(int[][] arrivalTimesAtStopsPerIteration, int max } } - /** - * Before routing, filter the set of patterns and trips to only the ones relevant for the search request (date - * and transit modes). We set the filtered patterns and trips on the transitLayer, in effect caching them for - * repeated searches on the same date with the same modes. - */ - private void filterPatternsAndTripsIfNeeded() { - boolean performFiltering; - if (transit.filteredTripPatterns == null) { - // Trip filtering has not yet been performed. - performFiltering = true; - } else { - // Trip filtering was already performed, but the filtering criteria may have changed. - performFiltering = !transit.filteredTripPatterns.matchesRequest(request.transitModes, servicesActive); - } - if (performFiltering) { - transit.filterPatternsAndTrips(request.transitModes, servicesActive); - } - } - /** * Create the initial array of states for the latest departure minute in the window, one state for each round. * One state for each allowed transit ride, plus a zeroth round containing the results of the access street search. @@ -468,14 +451,14 @@ private static Path[] pathToEachStop(RaptorState state) { */ private void doScheduledSearchForRound (RaptorState outputState) { final RaptorState inputState = outputState.previous; - BitSet patternsToExplore = patternsToExploreInNextRound(inputState, - transit.filteredTripPatterns.runningScheduledPatterns, - true); + BitSet patternsToExplore = patternsToExploreInNextRound( + inputState, filteredPatterns.runningScheduledPatterns, true + ); for (int patternIndex = patternsToExplore.nextSetBit(0); patternIndex >= 0; patternIndex = patternsToExplore.nextSetBit(patternIndex + 1) ) { - FilteredPattern pattern = transit.filteredTripPatterns.patterns.get(patternIndex); + FilteredPattern pattern = filteredPatterns.patterns.get(patternIndex); int onTrip = -1; // Used as index into list of active scheduled trips running on this pattern int waitTime = 0; int boardTime = Integer.MAX_VALUE; @@ -617,14 +600,14 @@ private void doFrequencySearchForRound (RaptorState outputState, FrequencyBoardi // are applying randomized schedules that are not present in the accumulated range-raptor upper bound state. // Those randomized frequency routes may cascade improvements from updates made at previous departure minutes. final boolean withinMinute = (frequencyBoardingMode == UPPER_BOUND); - BitSet patternsToExplore = patternsToExploreInNextRound(inputState, - transit.filteredTripPatterns.runningFrequencyPatterns, - withinMinute); + BitSet patternsToExplore = patternsToExploreInNextRound( + inputState, filteredPatterns.runningFrequencyPatterns, withinMinute + ); for (int patternIndex = patternsToExplore.nextSetBit(0); patternIndex >= 0; patternIndex = patternsToExplore.nextSetBit(patternIndex + 1) ) { - FilteredPattern pattern = transit.filteredTripPatterns.patterns.get(patternIndex); + FilteredPattern pattern = filteredPatterns.patterns.get(patternIndex); int tripScheduleIndex = -1; // First loop iteration will immediately increment to 0. for (TripSchedule schedule : pattern.runningFrequencyTrips) { tripScheduleIndex++; diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java new file mode 100644 index 000000000..ba3caecb8 --- /dev/null +++ b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java @@ -0,0 +1,74 @@ +package com.conveyal.r5.transit; + +import com.conveyal.r5.api.util.TransitModes; + +import java.util.ArrayList; +import java.util.BitSet; +import java.util.EnumSet; +import java.util.List; + +import static com.conveyal.r5.transit.TransitLayer.getTransitModes; + +/** + * Associate filtered patterns within a particular TransportNetwork (scenario) with the criteria (transit modes and + * active services) used to filter them. Many patterns contain a mixture of trips from different days, and those trips + * appear to overtake one another if we do not filter them down. Filtering allows us to more accurately flag which + * patterns have no overtaking, because departure time searches can be optimized on trips with no overtaking. + */ +public class FilteredPatterns { + + /** + * Transit modes in the analysis request. We filter down to only those modes enabled in the search request, + * because all trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. + */ + private final EnumSet modes; + + /** GTFS services, e.g. active on a specific date */ + private final BitSet services; + + /** + * List with the same length and indexes as the unfiltered tripPatterns. + * Patterns that do not meet the mode/services filtering criteria are recorded as null. + */ + public final List patterns; + + /** The indexes of the trip patterns running on a given day with frequency-based trips of selected modes. */ + public BitSet runningFrequencyPatterns = new BitSet(); + + /** The indexes of the trip patterns running on a given day with scheduled trips of selected modes. */ + public BitSet runningScheduledPatterns = new BitSet(); + + public boolean matchesRequest (EnumSet modes, BitSet services) { + return this.modes.equals(modes) && this.services.equals(services); + } + + /** + * Construct a FilteredPatterns from the given transitLayer, filtering for the specified modes and active services. + */ + public FilteredPatterns (TransitLayer transitLayer, EnumSet modes, BitSet services) { + this.modes = modes; + this.services = services; + List sourcePatterns = transitLayer.tripPatterns; + patterns = new ArrayList<>(sourcePatterns.size()); + for (int patternIndex = 0; patternIndex < sourcePatterns.size(); patternIndex++) { + TripPattern pattern = sourcePatterns.get(patternIndex); + RouteInfo routeInfo = transitLayer.routes.get(pattern.routeIndex); + TransitModes mode = getTransitModes(routeInfo.route_type); + if (pattern.servicesActive.intersects(services) && modes.contains(mode)) { + patterns.add(new FilteredPattern(pattern, services)); + // At least one trip on this pattern is relevant, based on the profile request's date and modes. + if (pattern.hasFrequencies) { + runningFrequencyPatterns.set(patternIndex); + } + // Schedule case is not an "else" clause because we support patterns with both frequency and schedule. + if (pattern.hasSchedules) { + runningScheduledPatterns.set(patternIndex); + } + } else { + patterns.add(null); + } + } + } + + +} diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index d91e63ccd..ecb9dac4a 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -105,63 +105,12 @@ public class TransitLayer implements Serializable, Cloneable { public List tripPatterns = new ArrayList<>(); - /** Stores the relevant patterns and trips based on the transit modes and date in an analysis request */ - public transient FilteredPatterns filteredTripPatterns; - - /** Associate filtered patterns with the criteria (modes and service) used to filter them */ - public static class FilteredPatterns { - /** - * Transit modes in the analysis request. We filter down to only those modes enabled in the search request, - * because all trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. - */ - private final EnumSet modes; - - /** GTFS services, e.g. active on a specific date */ - private final BitSet services; - - /** - * List with the same length and indexes as full tripPatterns. Patterns that do not meed the mode/services - * filtering criteria are recorded as null. - */ - public List patterns = new ArrayList<>(); - /** - * The indexes of the trip patterns running on a given day with frequency-based trips of selected modes. */ - public BitSet runningFrequencyPatterns = new BitSet(); - - /** The indexes of the trip patterns running on a given day with scheduled trips of selected modes. */ - public BitSet runningScheduledPatterns = new BitSet(); - - FilteredPatterns(EnumSet modes, BitSet services) { - this.modes = modes; - this.services = services; - } - - public boolean matchesRequest(EnumSet modes, BitSet services) { - return this.modes.equals(modes) && this.services.equals(services); - } - } - - public void filterPatternsAndTrips(EnumSet modes, BitSet services) { - this.filteredTripPatterns = new FilteredPatterns(modes, services); - for (int patternIndex = 0; patternIndex < this.tripPatterns.size(); patternIndex++) { - TripPattern pattern = this.tripPatterns.get(patternIndex); - RouteInfo routeInfo = this.routes.get(pattern.routeIndex); - TransitModes mode = TransitLayer.getTransitModes(routeInfo.route_type); - if (pattern.servicesActive.intersects(services) && modes.contains(mode)) { - this.filteredTripPatterns.patterns.add(new FilteredPattern(pattern, services)); - // At least one trip on this pattern is relevant, based on the profile request's date and modes. - if (pattern.hasFrequencies) { - this.filteredTripPatterns.runningFrequencyPatterns.set(patternIndex); - } - // Schedule case is not an "else" clause because we support patterns with both frequency and schedule. - if (pattern.hasSchedules) { - this.filteredTripPatterns.runningScheduledPatterns.set(patternIndex); - } - } else { - this.filteredTripPatterns.patterns.add(null); - } - } - } + /** + * Stores the relevant patterns and trips based on the transit modes and date in an analysis request. + * Caches the most recently generated. + * This should only be accessed through the synchronized accessor method. + */ + private transient FilteredPatterns filteredTripPatterns; // Maybe we need a StopStore that has (streetVertexForStop, transfers, flags, etc.) public TIntList streetVertexForStop = new TIntArrayList(); @@ -899,4 +848,25 @@ public String stopString(int stopIndex, boolean includeName) { return stop; } + /** + * Before routing, filter the set of patterns and trips to only the ones relevant for the search request (date + * and transit modes). We set the filtered patterns and trips on the transitLayer, in effect caching them for + * repeated searches on the same date with the same modes. Although it does not affect correctness, for efficiency + * the caller should retain a reference to the returned FilteredPatterns because the single cached value may change + * rapidly over time as other threads call this method. + * FIXME this will be inefficient for two dates or two sets of modes on the same scenario. + */ + public FilteredPatterns getFilteredPatterns (EnumSet modes, BitSet services) { + // Locking would not be strictly necessary here if we made a local copy of the reference. + // Java guarantees no tearing of reference reads and writes across threads. + // All threads might then perform the same filtering at the same time, but that's not necessarily a problem. + synchronized (this) { + // Check if trip filtering was not yet performed, or if the filtering criteria have changed. + if (filteredTripPatterns == null || !(filteredTripPatterns.matchesRequest(modes, services))) { + filteredTripPatterns = new FilteredPatterns(this, modes, services); + } + return filteredTripPatterns; + } + } + } From 99f414ad41710c567729cc6a93d0daede0a27d9a Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Tue, 13 Apr 2021 17:49:37 +0800 Subject: [PATCH 17/32] LoadingCache for multiple FilteredPatterns per TransitLayer Factor cache key class out of FilteredPatterns. Introduce reusable generic Tuple2 class for map keys among other things. Record and log computation time for filtering. --- .../conveyal/r5/profile/FastRaptorWorker.java | 5 ++- .../com/conveyal/r5/profile/RaptorTimer.java | 2 + .../conveyal/r5/transit/FilteredPattern.java | 20 ++++++--- .../r5/transit/FilteredPatternCache.java | 45 +++++++++++++++++++ .../conveyal/r5/transit/FilteredPatterns.java | 18 ++------ .../com/conveyal/r5/transit/TransitLayer.java | 29 +----------- .../com/conveyal/r5/transit/TripPattern.java | 1 + .../java/com/conveyal/r5/util/Tuple2.java | 31 +++++++++++++ 8 files changed, 101 insertions(+), 50 deletions(-) create mode 100644 src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java create mode 100644 src/main/java/com/conveyal/r5/util/Tuple2.java diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index b4fe8b912..73d21015e 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -5,6 +5,7 @@ import com.conveyal.r5.transit.FilteredPatterns; import com.conveyal.r5.transit.PickDropType; import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.TripPattern; import com.conveyal.r5.transit.TripSchedule; import com.conveyal.r5.transit.path.Path; import gnu.trove.iterator.TIntIterator; @@ -165,7 +166,9 @@ public FastRaptorWorker (TransitLayer transitLayer, AnalysisWorkerTask request, */ public int[][] route () { raptorTimer.fullSearch.start(); - filteredPatterns = transit.getFilteredPatterns(request.transitModes, servicesActive); + raptorTimer.patternFiltering.start(); + filteredPatterns = transit.filteredPatternCache.get(request.transitModes, servicesActive); + raptorTimer.patternFiltering.stop(); // Initialize result storage. Results are one arrival time at each stop, for every raptor iteration. final int nStops = transit.getStopCount(); final int nIterations = iterationsPerMinute * nMinutes; diff --git a/src/main/java/com/conveyal/r5/profile/RaptorTimer.java b/src/main/java/com/conveyal/r5/profile/RaptorTimer.java index e4b4504e3..1a6954d91 100644 --- a/src/main/java/com/conveyal/r5/profile/RaptorTimer.java +++ b/src/main/java/com/conveyal/r5/profile/RaptorTimer.java @@ -7,6 +7,8 @@ public class RaptorTimer { public final ExecutionTimer fullSearch = new ExecutionTimer("Full range-Raptor search"); + public final ExecutionTimer patternFiltering = new ExecutionTimer(fullSearch, "Pattern filtering"); + public final ExecutionTimer scheduledSearch = new ExecutionTimer(fullSearch, "Scheduled/bounds search"); public final ExecutionTimer scheduledSearchTransit = new ExecutionTimer(scheduledSearch, "Scheduled search"); diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java index d827b3239..2d24b40ef 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java @@ -18,7 +18,7 @@ public class FilteredPattern extends TripPattern { /** Frequency-based trips active in a particular set of GTFS services */ public List runningFrequencyTrips = new ArrayList<>(); - /** If no active schedule-based trip of this pattern overtakes another. */ + /** If no active schedule-based trip of this filtered pattern overtakes another. */ public boolean noScheduledOvertaking; /** @@ -26,7 +26,7 @@ public class FilteredPattern extends TripPattern { * filtered to exclude trips not active in the supplied set of services, then divided into separate * scheduled and frequency trip lists. Finally, check the runningScheduledTrips for overtaking. */ - public FilteredPattern(TripPattern source, BitSet servicesActive) { + public FilteredPattern (TripPattern source, BitSet servicesActive) { this.originalId = source.originalId; this.routeId = source.routeId; this.stops = source.stops; @@ -42,25 +42,31 @@ public FilteredPattern(TripPattern source, BitSet servicesActive) { } } // These could be set more strictly when looping over tripSchedules; just use the source pattern's values for - // now. + // now. TODO can't these be runningScheduledTrips.size() > 0? this.hasFrequencies = source.hasFrequencies; this.hasSchedules = source.hasSchedules; // Check for overtaking - boolean noScheduledOvertaking = true; + // TODO invert loops, should be more efficient to do inner loop over contiguous arrays, + // also allows factoring out a method to check two TripSchedules for overtaking. + noScheduledOvertaking = true; loopOverStops: for (int stopOffset = 0; stopOffset < source.stops.length; stopOffset++) { for (int i = 0; i < runningScheduledTrips.size() - 1; i++) { if (runningScheduledTrips.get(i).departures[stopOffset] > runningScheduledTrips.get(i + 1).departures[stopOffset] ) { - LOG.warn("Overtaking: route {} pattern {} stop #{} time {}", - routeId, originalId, stopOffset, runningScheduledTrips.get(i + 1).departures[stopOffset]); + LOG.warn( + "Overtaking: route {} pattern {} stop #{} time {}", + source.routeId, + source.originalId, + stopOffset, + runningScheduledTrips.get(i + 1).departures[stopOffset] + ); noScheduledOvertaking = false; break loopOverStops; } } } - this.noScheduledOvertaking = noScheduledOvertaking; } } diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java b/src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java new file mode 100644 index 000000000..3d76dd04d --- /dev/null +++ b/src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java @@ -0,0 +1,45 @@ +package com.conveyal.r5.transit; + +import com.conveyal.r5.api.util.TransitModes; +import com.conveyal.r5.util.Tuple2; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; + +import java.util.BitSet; +import java.util.EnumSet; + +/** + * Stores the relevant patterns and trips based on the transit modes and date in an analysis request. + * We can't just cache the single most recently used filtered patterns, because a worker might need to simultaneously + * handle two requests for the same scenario on different dates or with different modes. + * + * There are good reasons why this cache is specific to a single TransitLayer (representing one specific scenario). + * To create FilteredPatterns we need the source TransitLayer object. LoadingCaches must compute values based only on + * their keys. So a system-wide FilteredPatternCache would either need to recursively look up TransportNetworks in + * the TransportNetworkCache, or would need to have TransportNetwork or TransitLayer references in its keys. Neither + * of these seems desirable - the latter would impede garbage collection of evicted TransportNetworks. + */ +public class FilteredPatternCache { + + private final TransitLayer transitLayer; + private final LoadingCache cache; + + public FilteredPatternCache (TransitLayer transitLayer) { + this.transitLayer = transitLayer; + this.cache = Caffeine.newBuilder().maximumSize(2).build(key -> { + return new FilteredPatterns(transitLayer, key.a, key.b); + }); + } + + // TODO replace all keys and tuples with Java 16/17 Records + public static class Key extends Tuple2, BitSet> { + public Key (EnumSet transitModes, BitSet bitSet) { + super(transitModes, bitSet); + } + } + + public FilteredPatterns get (EnumSet transitModes, BitSet bitSet) { + return cache.get(new Key(transitModes, bitSet)); + } + +} diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java index ba3caecb8..c4e8a841d 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java @@ -1,6 +1,7 @@ package com.conveyal.r5.transit; import com.conveyal.r5.api.util.TransitModes; +import com.conveyal.r5.util.Tuple2; import java.util.ArrayList; import java.util.BitSet; @@ -14,18 +15,10 @@ * active services) used to filter them. Many patterns contain a mixture of trips from different days, and those trips * appear to overtake one another if we do not filter them down. Filtering allows us to more accurately flag which * patterns have no overtaking, because departure time searches can be optimized on trips with no overtaking. + * All trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. */ public class FilteredPatterns { - /** - * Transit modes in the analysis request. We filter down to only those modes enabled in the search request, - * because all trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. - */ - private final EnumSet modes; - - /** GTFS services, e.g. active on a specific date */ - private final BitSet services; - /** * List with the same length and indexes as the unfiltered tripPatterns. * Patterns that do not meet the mode/services filtering criteria are recorded as null. @@ -38,16 +31,11 @@ public class FilteredPatterns { /** The indexes of the trip patterns running on a given day with scheduled trips of selected modes. */ public BitSet runningScheduledPatterns = new BitSet(); - public boolean matchesRequest (EnumSet modes, BitSet services) { - return this.modes.equals(modes) && this.services.equals(services); - } - /** * Construct a FilteredPatterns from the given transitLayer, filtering for the specified modes and active services. + * It's tempting to use List.of() or Collectors.toUnmodifiableList() but these cause an additional array copy. */ public FilteredPatterns (TransitLayer transitLayer, EnumSet modes, BitSet services) { - this.modes = modes; - this.services = services; List sourcePatterns = transitLayer.tripPatterns; patterns = new ArrayList<>(sourcePatterns.size()); for (int patternIndex = 0; patternIndex < sourcePatterns.size(); patternIndex++) { diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index ecb9dac4a..0c0c7f6fa 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -105,12 +105,8 @@ public class TransitLayer implements Serializable, Cloneable { public List tripPatterns = new ArrayList<>(); - /** - * Stores the relevant patterns and trips based on the transit modes and date in an analysis request. - * Caches the most recently generated. - * This should only be accessed through the synchronized accessor method. - */ - private transient FilteredPatterns filteredTripPatterns; + /** Stores the relevant patterns and trips based on the transit modes and date in an analysis request. */ + public transient FilteredPatternCache filteredPatternCache = new FilteredPatternCache(this); // Maybe we need a StopStore that has (streetVertexForStop, transfers, flags, etc.) public TIntList streetVertexForStop = new TIntArrayList(); @@ -848,25 +844,4 @@ public String stopString(int stopIndex, boolean includeName) { return stop; } - /** - * Before routing, filter the set of patterns and trips to only the ones relevant for the search request (date - * and transit modes). We set the filtered patterns and trips on the transitLayer, in effect caching them for - * repeated searches on the same date with the same modes. Although it does not affect correctness, for efficiency - * the caller should retain a reference to the returned FilteredPatterns because the single cached value may change - * rapidly over time as other threads call this method. - * FIXME this will be inefficient for two dates or two sets of modes on the same scenario. - */ - public FilteredPatterns getFilteredPatterns (EnumSet modes, BitSet services) { - // Locking would not be strictly necessary here if we made a local copy of the reference. - // Java guarantees no tearing of reference reads and writes across threads. - // All threads might then perform the same filtering at the same time, but that's not necessarily a problem. - synchronized (this) { - // Check if trip filtering was not yet performed, or if the filtering criteria have changed. - if (filteredTripPatterns == null || !(filteredTripPatterns.matchesRequest(modes, services))) { - filteredTripPatterns = new FilteredPatterns(this, modes, services); - } - return filteredTripPatterns; - } - } - } diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 672fa1ded..3358d87f1 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -33,6 +33,7 @@ public class TripPattern implements Serializable, Cloneable { * This is the ID of this trip pattern _in the original transport network_. This is important because if it were the * ID in this transport network the ID would depend on the order of application of scenarios, and because this ID is * used to map results back to the original network. + * TODO clarify this comment. What is the "original" transport network? This field doesn't seem to be used anywhere. */ public int originalId; diff --git a/src/main/java/com/conveyal/r5/util/Tuple2.java b/src/main/java/com/conveyal/r5/util/Tuple2.java new file mode 100644 index 000000000..595e0f356 --- /dev/null +++ b/src/main/java/com/conveyal/r5/util/Tuple2.java @@ -0,0 +1,31 @@ +package com.conveyal.r5.util; + +import java.util.Objects; + +/** + * Generic logic for a 2-tuple of different types. + * Reduces high-maintenance boilerplate clutter when making map key types. + * TODO replace with Records in Java 16 or 17 + */ +public class Tuple2 { + public final A a; + public final B b; + + public Tuple2 (A a, B b) { + this.a = a; + this.b = b; + } + + @Override + public boolean equals (Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Tuple2 tuple2 = (Tuple2) o; + return Objects.equals(a, tuple2.a) && Objects.equals(b, tuple2.b); + } + + @Override + public int hashCode () { + return Objects.hash(a, b); + } +} From 9a880f977408f6a32c55fe7b1bcea3d61bc80442 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Tue, 13 Apr 2021 17:54:57 +0800 Subject: [PATCH 18/32] Compose FilteredPattern with TripPattern by index rather than inheriting from TripPattern and partially copying fields --- .../conveyal/r5/profile/FastRaptorWorker.java | 16 +++++++++------- .../com/conveyal/r5/transit/FilteredPattern.java | 11 +---------- .../conveyal/r5/transit/FilteredPatterns.java | 1 - 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index 73d21015e..e585a0fd4 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -461,7 +461,8 @@ private void doScheduledSearchForRound (RaptorState outputState) { patternIndex >= 0; patternIndex = patternsToExplore.nextSetBit(patternIndex + 1) ) { - FilteredPattern pattern = filteredPatterns.patterns.get(patternIndex); + FilteredPattern filteredPattern = filteredPatterns.patterns.get(patternIndex); + TripPattern pattern = transit.tripPatterns.get(patternIndex); int onTrip = -1; // Used as index into list of active scheduled trips running on this pattern int waitTime = 0; int boardTime = Integer.MAX_VALUE; @@ -491,7 +492,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { pattern.pickups[stopPositionInPattern] != PickDropType.NONE ) { int earliestBoardTime = inputState.bestTimes[stop] + MINIMUM_BOARD_WAIT_SEC; - List candidateSchedules = pattern.runningScheduledTrips; + List candidateSchedules = filteredPattern.runningScheduledTrips; if (onTrip == -1) { int candidateTripIndex = -1; for (TripSchedule candidateSchedule : candidateSchedules) { @@ -507,7 +508,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { boardTime = candidateSchedule.departures[stopPositionInPattern]; waitTime = boardTime - inputState.bestTimes[stop]; boardStop = stop; - if (pattern.noScheduledOvertaking) { + if (filteredPattern.noScheduledOvertaking) { // All remaining candidateSchedules depart this stop after the one we are // considering, because we are iterating in ascending order of departure time // from the first stop of the pattern and there is no overtaking. @@ -525,7 +526,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { // checked are the ones that depart the first stop of the pattern before the trip we are // on does, and we can break out of the loop once the departures are too early to board. // If there is overtaking, all candidate trip schedules need to be checked. - int candidateTripIndex = pattern.noScheduledOvertaking ? onTrip : candidateSchedules.size(); + int candidateTripIndex = filteredPattern.noScheduledOvertaking ? onTrip : candidateSchedules.size(); // Note decrement below (otherwise, we'd use candidateSchedules.size() - 1 above) while (--candidateTripIndex >= 0) { // The tripSchedules in a given pattern are sorted by time of departure from the first @@ -546,7 +547,7 @@ private void doScheduledSearchForRound (RaptorState outputState) { } else { // The trip under consideration arrives at this stop earlier than one could feasibly // board. - if (pattern.noScheduledOvertaking) { + if (filteredPattern.noScheduledOvertaking) { // We are confident of being on the earliest feasible departure. break; } @@ -610,9 +611,10 @@ private void doFrequencySearchForRound (RaptorState outputState, FrequencyBoardi patternIndex >= 0; patternIndex = patternsToExplore.nextSetBit(patternIndex + 1) ) { - FilteredPattern pattern = filteredPatterns.patterns.get(patternIndex); + FilteredPattern filteredPattern = filteredPatterns.patterns.get(patternIndex); + TripPattern pattern = transit.tripPatterns.get(patternIndex); int tripScheduleIndex = -1; // First loop iteration will immediately increment to 0. - for (TripSchedule schedule : pattern.runningFrequencyTrips) { + for (TripSchedule schedule : filteredPattern.runningFrequencyTrips) { tripScheduleIndex++; // Loop through all the entries for this trip (time windows with service at a given frequency). for (int frequencyEntryIdx = 0; diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java index 2d24b40ef..9179639d3 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java @@ -8,7 +8,7 @@ import java.util.List; -public class FilteredPattern extends TripPattern { +public class FilteredPattern { private static Logger LOG = LoggerFactory.getLogger(FilteredPattern.class); @@ -27,11 +27,6 @@ public class FilteredPattern extends TripPattern { * scheduled and frequency trip lists. Finally, check the runningScheduledTrips for overtaking. */ public FilteredPattern (TripPattern source, BitSet servicesActive) { - this.originalId = source.originalId; - this.routeId = source.routeId; - this.stops = source.stops; - this.pickups = source.pickups; - this.dropoffs = source.dropoffs; for (TripSchedule schedule : source.tripSchedules) { if (servicesActive.get(schedule.serviceCode)) { if (schedule.headwaySeconds == null) { @@ -41,10 +36,6 @@ public FilteredPattern (TripPattern source, BitSet servicesActive) { } } } - // These could be set more strictly when looping over tripSchedules; just use the source pattern's values for - // now. TODO can't these be runningScheduledTrips.size() > 0? - this.hasFrequencies = source.hasFrequencies; - this.hasSchedules = source.hasSchedules; // Check for overtaking // TODO invert loops, should be more efficient to do inner loop over contiguous arrays, diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java index c4e8a841d..a78743091 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java @@ -58,5 +58,4 @@ public FilteredPatterns (TransitLayer transitLayer, EnumSet modes, } } - } From 2c4bc06c0542f4be84e5bec1b8ceb468a6b73f2c Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Tue, 13 Apr 2021 18:03:05 +0800 Subject: [PATCH 19/32] transpose loops over trips and stops It should be more efficient to do the inner loop over contiguous arrays, also allows factoring out method to check TripSchedules for overtaking. --- .../conveyal/r5/transit/FilteredPattern.java | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java index 9179639d3..67da780dc 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java @@ -37,27 +37,22 @@ public FilteredPattern (TripPattern source, BitSet servicesActive) { } } - // Check for overtaking - // TODO invert loops, should be more efficient to do inner loop over contiguous arrays, - // also allows factoring out a method to check two TripSchedules for overtaking. + // Check whether any running trip on this pattern overtakes another noScheduledOvertaking = true; - loopOverStops: - for (int stopOffset = 0; stopOffset < source.stops.length; stopOffset++) { - for (int i = 0; i < runningScheduledTrips.size() - 1; i++) { - if (runningScheduledTrips.get(i).departures[stopOffset] > - runningScheduledTrips.get(i + 1).departures[stopOffset] - ) { - LOG.warn( - "Overtaking: route {} pattern {} stop #{} time {}", - source.routeId, - source.originalId, - stopOffset, - runningScheduledTrips.get(i + 1).departures[stopOffset] - ); - noScheduledOvertaking = false; - break loopOverStops; - } + for (int i = 0; i < runningScheduledTrips.size() - 1; i++) { + if (overtakes(runningScheduledTrips.get(i), runningScheduledTrips.get(i + 1))) { + noScheduledOvertaking = false; + LOG.warn("Overtaking: route {} pattern {}", source.routeId, source.originalId); + break; } } } + + private static boolean overtakes (TripSchedule a, TripSchedule b) { + for (int s = 0; s < a.departures.length; s++) { + if (a.departures[s] > b.departures[s]) return true; + } + return false; + } + } From e53007e0c9a030f32a0d234c0f44c6430402389f Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 14 Apr 2021 18:56:58 +0800 Subject: [PATCH 20/32] clarify raptor departure search Update code comments, factor out candidate departure time, nest ifs to simplify interpretation of else clause (trip arrives too early). --- .../conveyal/r5/profile/FastRaptorWorker.java | 63 +++++++++---------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index e585a0fd4..f19ff39eb 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -468,12 +468,11 @@ private void doScheduledSearchForRound (RaptorState outputState) { int boardTime = Integer.MAX_VALUE; int boardStop = -1; TripSchedule schedule = null; - + // Iterate over all stops in the current TripPattern for (int stopPositionInPattern = 0; stopPositionInPattern < pattern.stops.length; stopPositionInPattern++) { int stop = pattern.stops[stopPositionInPattern]; - - // attempt to alight if we're on board and if drop off is allowed, done above the board search so - // that we don't check for alighting when boarding + // Alight at the current stop in the pattern if drop-off is allowed. + // This is done before the board search so that we don't alight from the same stop where we boarded. if (onTrip > -1 && pattern.dropoffs[stopPositionInPattern] != PickDropType.NONE) { int alightTime = schedule.arrivals[stopPositionInPattern]; int inVehicleTime = alightTime - boardTime; @@ -485,23 +484,22 @@ private void doScheduledSearchForRound (RaptorState outputState) { outputState.setTimeAtStop(stop, alightTime, patternIndex, boardStop, waitTime, inVehicleTime, false); } - - // Don't attempt to board if this stop was not reached in the last round or if pick up is not allowed. - // Scheduled searches only care about updates within this departure minute, enabling range-raptor. + // If the current stop was reached in the previous round and allows pick-up, board or re-board a trip. + // Second parameter is true to only look at changes within this departure minute, enabling range-raptor. if (inputState.stopWasUpdated(stop, true) && pattern.pickups[stopPositionInPattern] != PickDropType.NONE ) { int earliestBoardTime = inputState.bestTimes[stop] + MINIMUM_BOARD_WAIT_SEC; List candidateSchedules = filteredPattern.runningScheduledTrips; if (onTrip == -1) { + // We have not yet boarded any trip. Iterate through active scheduled trips on this pattern, in + // ascending order of departure time from first stop of the pattern. Look for the earliest + // usable departure, iteratively improving upon the best departure seen so far in the list. int candidateTripIndex = -1; for (TripSchedule candidateSchedule : candidateSchedules) { - // Iterate through schedules of active scheduled trips on this pattern, in ascending order - // of departure time from first stop of the pattern candidateTripIndex++; - if (earliestBoardTime < candidateSchedule.departures[stopPositionInPattern] && - candidateSchedule.departures[stopPositionInPattern] < boardTime + candidateSchedule.departures[stopPositionInPattern] < boardTime ) { onTrip = candidateTripIndex; schedule = candidateSchedule; @@ -509,44 +507,43 @@ private void doScheduledSearchForRound (RaptorState outputState) { waitTime = boardTime - inputState.bestTimes[stop]; boardStop = stop; if (filteredPattern.noScheduledOvertaking) { - // All remaining candidateSchedules depart this stop after the one we are - // considering, because we are iterating in ascending order of departure time - // from the first stop of the pattern and there is no overtaking. + // All remaining candidateSchedules are known to depart this stop after the one we + // are considering. No need to search further, we already have the earliest one. break; } } } } else { - // A specific trip on this pattern was boarded at an upstream stop in this scan. If we are - // ready to depart from this stop before this trip does, it might be preferable to board at - // this stop instead. + // During this scan, a specific trip on this pattern was already boarded at an upstream stop. + // If we are ready to depart from this stop before that trip does, it might be preferable to + // board an earlier trip at this stop than the one we're on. if (earliestBoardTime < schedule.departures[stopPositionInPattern]) { // First, it might be possible to board an earlier trip at this stop. // If trips are well-sorted on this pattern (no overtaking), the only trips that need to be // checked are the ones that depart the first stop of the pattern before the trip we are // on does, and we can break out of the loop once the departures are too early to board. // If there is overtaking, all candidate trip schedules need to be checked. - int candidateTripIndex = filteredPattern.noScheduledOvertaking ? onTrip : candidateSchedules.size(); + int candidateTripIndex = filteredPattern.noScheduledOvertaking + ? onTrip : candidateSchedules.size(); // Note decrement below (otherwise, we'd use candidateSchedules.size() - 1 above) while (--candidateTripIndex >= 0) { // The tripSchedules in a given pattern are sorted by time of departure from the first - // stop of the pattern. In this first step, we assume no overtaking so the tripSchedules - // are also sorted by time of departure at this stop. The predicate for the break - // statement at the end of this loop handles overtaking. + // stop of the pattern. If there is no overtaking in the tripSchedules, the break + // statement at the end of this loop can exit without scanning the whole list. TripSchedule candidateSchedule = candidateSchedules.get(candidateTripIndex); - - if (earliestBoardTime < candidateSchedule.departures[stopPositionInPattern] && - candidateSchedule.departures[stopPositionInPattern] < schedule.departures[stopPositionInPattern] - ) { - // The trip under consideration can be boarded at this stop - onTrip = candidateTripIndex; - schedule = candidateSchedule; - boardTime = candidateSchedule.departures[stopPositionInPattern]; - waitTime = boardTime - inputState.bestTimes[stop]; - boardStop = stop; + final int candidateDeparture = candidateSchedule.departures[stopPositionInPattern]; + if (earliestBoardTime < candidateDeparture) { + // The candidate trip can be boarded at this stop (we reach it early enough). + if (candidateDeparture < schedule.departures[stopPositionInPattern]) { + // The candidate trip departs this stop earlier than the one we're already on. + onTrip = candidateTripIndex; + schedule = candidateSchedule; + boardTime = candidateDeparture; + waitTime = boardTime - inputState.bestTimes[stop]; + boardStop = stop; + } } else { - // The trip under consideration arrives at this stop earlier than one could feasibly - // board. + // The trip under consideration arrives at this stop too early to board it. if (filteredPattern.noScheduledOvertaking) { // We are confident of being on the earliest feasible departure. break; From 332856dfcb5f642eea3c41abb2b324192f3e69e9 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 14 Apr 2021 23:53:42 +0800 Subject: [PATCH 21/32] factor out two boarding / reboarding search methods One is used for the special case of reboarding with no overtaking. The other is used for first boarding and reboarding with overtaking. This avoids the potentially confusing reuse of variables identifying the currently boarded trip to tentatively hold replacement trips within reboarding searches. Symbolic constants are used everywhere for "no trip" or "no time". --- .../conveyal/r5/profile/FastRaptorWorker.java | 187 +++++++++--------- 1 file changed, 99 insertions(+), 88 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index f19ff39eb..f4447d005 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -24,6 +24,7 @@ import static com.conveyal.r5.profile.FastRaptorWorker.FrequencyBoardingMode.HALF_HEADWAY; import static com.conveyal.r5.profile.FastRaptorWorker.FrequencyBoardingMode.MONTE_CARLO; import static com.conveyal.r5.profile.FastRaptorWorker.FrequencyBoardingMode.UPPER_BOUND; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; /** @@ -447,6 +448,61 @@ private static Path[] pathToEachStop(RaptorState state) { return paths; } + + /** + * Starting from a trip we're already riding, step backward through the trips in the supplied filteredPattern to see + * if there is a usable one that departs earlier from the current stop position in the pattern, and if so return its + * index within the filtered pattern. This method assumes there is no overtaking in the FilteredPattern's schedules. + */ + private int checkEarlierScheduledDeparture ( + int departAfter, FilteredPattern filteredPattern, int stopInPattern, int currentTrip + ) { + TripSchedule schedule = filteredPattern.runningScheduledTrips.get(currentTrip); + final int scheduleDeparture = schedule.departures[stopInPattern]; + int bestTrip = currentTrip; + checkArgument(filteredPattern.noScheduledOvertaking); + int candidateTrip = currentTrip; + while (--candidateTrip >= 0) { + // The tripSchedules in a the supplied pattern are known to be sorted by departure time at all stops. + TripSchedule candidateSchedule = filteredPattern.runningScheduledTrips.get(candidateTrip); + final int candidateDeparture = candidateSchedule.departures[stopInPattern]; + if (candidateDeparture > departAfter) { + bestTrip = candidateTrip; + } else { + // We are confident of being on the earliest feasible departure. + break; + } + } + return bestTrip; + } + + + /** + * Perform a linear search through the trips in the supplied filteredPattern, finding the one that departs + * earliest from the given stop position in the pattern, and returning its index within the filtered pattern. + */ + private int findEarliestScheduledDeparture ( + int departAfter, FilteredPattern filteredPattern, int stopInPattern + ) { + List trips = filteredPattern.runningScheduledTrips; + boolean noOvertaking = filteredPattern.noScheduledOvertaking; + int bestTrip = -1; + int bestDeparture = Integer.MAX_VALUE; + for (int t = 0; t < trips.size(); t++) { + TripSchedule ts = trips.get(t); + final int departure = ts.departures[stopInPattern]; + if (departure > departAfter && departure < bestDeparture) { + bestTrip = t; + bestDeparture = departure; + if (noOvertaking) break; + } + } + return bestTrip; + } + + // Chosen to be completely invalid as an array index or time in order to fail fast. + private static final int NONE = -1; + /** * A sub-step in the process of performing a RAPTOR search at one specific departure time (at one specific minute). * This method handles only the routes that have exact schedules. There is another method that handles only the @@ -455,7 +511,7 @@ private static Path[] pathToEachStop(RaptorState state) { private void doScheduledSearchForRound (RaptorState outputState) { final RaptorState inputState = outputState.previous; BitSet patternsToExplore = patternsToExploreInNextRound( - inputState, filteredPatterns.runningScheduledPatterns, true + inputState, filteredPatterns.runningScheduledPatterns, true ); for (int patternIndex = patternsToExplore.nextSetBit(0); patternIndex >= 0; @@ -463,104 +519,59 @@ private void doScheduledSearchForRound (RaptorState outputState) { ) { FilteredPattern filteredPattern = filteredPatterns.patterns.get(patternIndex); TripPattern pattern = transit.tripPatterns.get(patternIndex); - int onTrip = -1; // Used as index into list of active scheduled trips running on this pattern + // As we scan down the stops of the pattern, we may board a trip, and possibly re-board a different trip. + // Keep track of the index of the currently boarded trip within the list of filtered TripSchedules. + int onTrip = NONE; int waitTime = 0; - int boardTime = Integer.MAX_VALUE; - int boardStop = -1; + int boardTime = NONE; + int boardStop = NONE; TripSchedule schedule = null; - // Iterate over all stops in the current TripPattern - for (int stopPositionInPattern = 0; stopPositionInPattern < pattern.stops.length; stopPositionInPattern++) { - int stop = pattern.stops[stopPositionInPattern]; - // Alight at the current stop in the pattern if drop-off is allowed. - // This is done before the board search so that we don't alight from the same stop where we boarded. - if (onTrip > -1 && pattern.dropoffs[stopPositionInPattern] != PickDropType.NONE) { - int alightTime = schedule.arrivals[stopPositionInPattern]; + // Iterate over all stops in the current TripPattern ("scan" down the pattern) + for (int stopInPattern = 0; stopInPattern < pattern.stops.length; stopInPattern++) { + int stop = pattern.stops[stopInPattern]; + // Alight at the current stop in the pattern if drop-off is allowed and we're already on a trip. + // This block is above the boarding search so that we don't alight from the same stop where we boarded. + if (onTrip != NONE && pattern.dropoffs[stopInPattern] != PickDropType.NONE) { + int alightTime = schedule.arrivals[stopInPattern]; int inVehicleTime = alightTime - boardTime; - - // Use checkState instead? - if (waitTime + inVehicleTime + inputState.bestTimes[boardStop] > alightTime) { - LOG.error("Components of travel time are larger than travel time!"); - } - + checkState (alightTime == inputState.bestTimes[boardStop] + waitTime + inVehicleTime, + "Components of travel time are larger than travel time!"); outputState.setTimeAtStop(stop, alightTime, patternIndex, boardStop, waitTime, inVehicleTime, false); } // If the current stop was reached in the previous round and allows pick-up, board or re-board a trip. // Second parameter is true to only look at changes within this departure minute, enabling range-raptor. if (inputState.stopWasUpdated(stop, true) && - pattern.pickups[stopPositionInPattern] != PickDropType.NONE + pattern.pickups[stopInPattern] != PickDropType.NONE ) { int earliestBoardTime = inputState.bestTimes[stop] + MINIMUM_BOARD_WAIT_SEC; - List candidateSchedules = filteredPattern.runningScheduledTrips; - if (onTrip == -1) { - // We have not yet boarded any trip. Iterate through active scheduled trips on this pattern, in - // ascending order of departure time from first stop of the pattern. Look for the earliest - // usable departure, iteratively improving upon the best departure seen so far in the list. - int candidateTripIndex = -1; - for (TripSchedule candidateSchedule : candidateSchedules) { - candidateTripIndex++; - if (earliestBoardTime < candidateSchedule.departures[stopPositionInPattern] && - candidateSchedule.departures[stopPositionInPattern] < boardTime - ) { - onTrip = candidateTripIndex; - schedule = candidateSchedule; - boardTime = candidateSchedule.departures[stopPositionInPattern]; - waitTime = boardTime - inputState.bestTimes[stop]; - boardStop = stop; - if (filteredPattern.noScheduledOvertaking) { - // All remaining candidateSchedules are known to depart this stop after the one we - // are considering. No need to search further, we already have the earliest one. - break; - } - } - } + // Boarding/reboarding search is conditional on previous-round arrival at this stop earlier than the + // current trip in the current round. Otherwise the search is unnecessary and yields later trips. + if (schedule != null && (earliestBoardTime >= schedule.departures[stopInPattern])) { + continue; + } + int newTrip; + if (onTrip != NONE && filteredPattern.noScheduledOvertaking) { + // Optimized reboarding search: Already on a trip, trips known to be sorted by departure time. + newTrip = checkEarlierScheduledDeparture(earliestBoardTime, filteredPattern, stopInPattern, onTrip); } else { - // During this scan, a specific trip on this pattern was already boarded at an upstream stop. - // If we are ready to depart from this stop before that trip does, it might be preferable to - // board an earlier trip at this stop than the one we're on. - if (earliestBoardTime < schedule.departures[stopPositionInPattern]) { - // First, it might be possible to board an earlier trip at this stop. - // If trips are well-sorted on this pattern (no overtaking), the only trips that need to be - // checked are the ones that depart the first stop of the pattern before the trip we are - // on does, and we can break out of the loop once the departures are too early to board. - // If there is overtaking, all candidate trip schedules need to be checked. - int candidateTripIndex = filteredPattern.noScheduledOvertaking - ? onTrip : candidateSchedules.size(); - // Note decrement below (otherwise, we'd use candidateSchedules.size() - 1 above) - while (--candidateTripIndex >= 0) { - // The tripSchedules in a given pattern are sorted by time of departure from the first - // stop of the pattern. If there is no overtaking in the tripSchedules, the break - // statement at the end of this loop can exit without scanning the whole list. - TripSchedule candidateSchedule = candidateSchedules.get(candidateTripIndex); - final int candidateDeparture = candidateSchedule.departures[stopPositionInPattern]; - if (earliestBoardTime < candidateDeparture) { - // The candidate trip can be boarded at this stop (we reach it early enough). - if (candidateDeparture < schedule.departures[stopPositionInPattern]) { - // The candidate trip departs this stop earlier than the one we're already on. - onTrip = candidateTripIndex; - schedule = candidateSchedule; - boardTime = candidateDeparture; - waitTime = boardTime - inputState.bestTimes[stop]; - boardStop = stop; - } - } else { - // The trip under consideration arrives at this stop too early to board it. - if (filteredPattern.noScheduledOvertaking) { - // We are confident of being on the earliest feasible departure. - break; - } - } - } - // Second, if we care about paths or travel time components, check whether boarding at - // this stop instead of the upstream one would allow a shorter access/transfer leg. - // Doing so will not affect total travel time (as long as this is in a conditional - // ensuring we won't miss the trip we're on), but it will affect the breakdown of walk vs. - // wait time. - if (retainPaths && inputState.shorterAccessOrTransferLeg(stop, boardStop)) { - boardTime = schedule.departures[stopPositionInPattern]; - waitTime = boardTime - inputState.bestTimes[stop]; - boardStop = stop; - } - } + // General purpose departure search: not already on a trip or trips are not known to be sorted. + newTrip = findEarliestScheduledDeparture(earliestBoardTime, filteredPattern, stopInPattern); + } + // If we care about paths or travel time components, check whether boarding at this stop instead of + // the upstream one would allow a shorter access/transfer leg. Doing so will not affect total travel + // time (as long as this is in a conditional ensuring we won't miss the trip we're on), but it will + // affect the breakdown of walk vs. wait time. + final boolean reboardForPaths = retainPaths + && (onTrip != NONE) + && inputState.shorterAccessOrTransferLeg(stop, boardStop); + + if ((newTrip != onTrip) || reboardForPaths) { + checkState(newTrip != NONE); // Should never change from being on a trip to on no trip. + onTrip = newTrip; + schedule = filteredPattern.runningScheduledTrips.get(newTrip); + boardTime = schedule.departures[stopInPattern]; + waitTime = boardTime - inputState.bestTimes[stop]; + boardStop = stop; } } } From 04511c452bd727f40920a0a7c73a16f174fb4351 Mon Sep 17 00:00:00 2001 From: ansons Date: Wed, 14 Apr 2021 17:19:18 -0400 Subject: [PATCH 22/32] refactor(style): add spaces back to method declarations --- .../r5/analyst/network/GridGtfsGenerator.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java b/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java index 3619b7241..b77582982 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridGtfsGenerator.java @@ -34,13 +34,13 @@ public class GridGtfsGenerator { private final boolean mergeStops; - public GridGtfsGenerator(GridLayout gridLayout) { + public GridGtfsGenerator (GridLayout gridLayout) { this.gridLayout = gridLayout; feed = new GTFSFeed(); // Temp file db, can we do this in memory instead? mergeStops = true; } - public GTFSFeed generate() { + public GTFSFeed generate () { for (GridRoute route : gridLayout.routes) { addRoute(route); } @@ -48,7 +48,7 @@ public GTFSFeed generate() { return feed; } - private void addCommonTables() { + private void addCommonTables () { Agency agency = new Agency(); agency.agency_id = AGENCY_ID; agency.agency_name = AGENCY_ID; @@ -63,7 +63,7 @@ private void addCommonTables() { feed.services.put(service.service_id, service); } - private void addRoute(GridRoute gridRoute) { + private void addRoute (GridRoute gridRoute) { Route route = new Route(); route.agency_id = AGENCY_ID; route.route_id = gridRoute.id; @@ -77,13 +77,13 @@ private void addRoute(GridRoute gridRoute) { } } - private void addRoute(GridRoute route, boolean back) { + private void addRoute (GridRoute route, boolean back) { addStopsForRoute(route, back); addTripsForRoute(route, back); } // If mergeStops is true, certain stops will be created multiple times but IDs will collide on insertion. - public void addStopsForRoute(GridRoute route, boolean back) { + public void addStopsForRoute (GridRoute route, boolean back) { for (int s = 0; s < route.nStops; s++) { String stopId = route.stopId(s, mergeStops); Stop stop = new Stop(); @@ -96,7 +96,7 @@ public void addStopsForRoute(GridRoute route, boolean back) { } } - public void addTripsForRoute(GridRoute route, boolean back) { + public void addTripsForRoute (GridRoute route, boolean back) { if (route.headwayMinutes > 0) { int tripIndex = 0; int start = route.startHour * 60 * 60; @@ -118,20 +118,20 @@ public void addTripsForRoute(GridRoute route, boolean back) { } } } else { - for (int i = 0; i < route.startTimes.length; i++ ) { + for (int i = 0; i < route.startTimes.length; i++) { addTrip(route, back, route.startTimes[i], i); } } } - private String addTrip(GridRoute route, boolean back, int startTime, int tripIndex) { + private String addTrip (GridRoute route, boolean back, int startTime, int tripIndex) { Trip trip = new Trip(); trip.direction_id = back ? 1 : 0; String tripId = String.format("%s:%d:%d", route.id, tripIndex, trip.direction_id); trip.trip_id = tripId; trip.route_id = route.id; trip.service_id = SERVICE_ID; - feed.trips.put(trip.trip_id,trip); + feed.trips.put(trip.trip_id, trip); int dwell = gridLayout.transitDwellSeconds; int departureTime = startTime; int arrivalTime = departureTime - dwell; From 0872a1184ac1bc2cb997694b52a0eb074b4d5357 Mon Sep 17 00:00:00 2001 From: ansons Date: Wed, 14 Apr 2021 17:19:36 -0400 Subject: [PATCH 23/32] docs(paths): clarify comment --- .../java/com/conveyal/r5/analyst/cluster/PathResult.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index 5e86a537b..edf8ad057 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -155,8 +155,11 @@ public static class PathIterations { this.egress = pathTemplate.stopSequence.egress == null ? null : pathTemplate.stopSequence.egress.toString(); this.transitLegs = pathTemplate.transitLegs(transitLayer); this.iterations = iterations.stream().map(HumanReadableIteration::new).collect(Collectors.toList()); - iterations.forEach(pathTemplate.stopSequence::transferTime); // Sense check that travel time components - // do not exceed total travel time TODO report transfer times in single-point responses? + iterations.forEach(pathTemplate.stopSequence::transferTime); // The transferTime method includes an + // assertion that the transfer time is non-negative, i.e. that the access + egress + wait + ride times of + // a specific iteration do not exceed the total travel time. Perform that sense check here, even though + // the transfer time is not reported to the front-end for the human-readable single-point responses. + // TODO add transferTime to HumanReadableIteration? } } From d0a499ecaacf720fd0ce76eabd2b0b9f9a18afd0 Mon Sep 17 00:00:00 2001 From: ansons Date: Wed, 14 Apr 2021 17:26:10 -0400 Subject: [PATCH 24/32] refactor(patterns): remove unneeded no-arg constructor Not needed after 9a880f9 --- src/main/java/com/conveyal/r5/transit/TripPattern.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 3358d87f1..faa4e0e82 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -108,10 +108,6 @@ public TripPattern(String routeId, Iterable stopTimes, TObjectIntMap Date: Wed, 14 Apr 2021 20:31:35 -0400 Subject: [PATCH 25/32] docs(patterns): minor updates to comments --- src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java | 6 +++++- src/main/java/com/conveyal/r5/transit/FilteredPattern.java | 5 ++++- src/main/java/com/conveyal/r5/transit/FilteredPatterns.java | 6 +++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java index f4447d005..04e6487aa 100644 --- a/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java +++ b/src/main/java/com/conveyal/r5/profile/FastRaptorWorker.java @@ -76,7 +76,8 @@ public class FastRaptorWorker { private static final int MINIMUM_BOARD_WAIT_SEC = 60; // ENABLE_OPTIMIZATION_X flags enable code paths that should affect efficiency but have no effect on output. - // They may change results where our algorithm is not perfectly optimal, for example with respect to overtaking. + // They may change results where our algorithm is not perfectly optimal, for example with respect to overtaking + // (see discussion at #708). public static final boolean ENABLE_OPTIMIZATION_RANGE_RAPTOR = true; public static final boolean ENABLE_OPTIMIZATION_FREQ_UPPER_BOUND = true; @@ -484,6 +485,7 @@ private int checkEarlierScheduledDeparture ( private int findEarliestScheduledDeparture ( int departAfter, FilteredPattern filteredPattern, int stopInPattern ) { + // Trips are sorted in ascending order by time of departure from first stop List trips = filteredPattern.runningScheduledTrips; boolean noOvertaking = filteredPattern.noScheduledOvertaking; int bestTrip = -1; @@ -494,6 +496,8 @@ private int findEarliestScheduledDeparture ( if (departure > departAfter && departure < bestDeparture) { bestTrip = t; bestDeparture = departure; + // No overtaking plus sorting by time of departure from first stop guarantees sorting by time of + // departure at this stop; so we know this is the earliest departure and can break early. if (noOvertaking) break; } } diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java index 67da780dc..095033386 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java @@ -12,7 +12,10 @@ public class FilteredPattern { private static Logger LOG = LoggerFactory.getLogger(FilteredPattern.class); - /** Schedule-based (i.e. not frequency-based) trips running in a particular set of GTFS services */ + /** + * Schedule-based (i.e. not frequency-based) trips running in a particular set of GTFS services, sorted in + * ascending order by time of departure from first stop + */ public List runningScheduledTrips = new ArrayList<>(); /** Frequency-based trips active in a particular set of GTFS services */ diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java index a78743091..6c5f65cdc 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java @@ -13,9 +13,9 @@ /** * Associate filtered patterns within a particular TransportNetwork (scenario) with the criteria (transit modes and * active services) used to filter them. Many patterns contain a mixture of trips from different days, and those trips - * appear to overtake one another if we do not filter them down. Filtering allows us to more accurately flag which - * patterns have no overtaking, because departure time searches can be optimized on trips with no overtaking. - * All trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. + * appear to overtake one another if we do not filter them down. Filtering allows us to flag more effectively which + * patterns have no overtaking, which is useful because departure time searches can be then optimized for patterns with + * no overtaking. All trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. */ public class FilteredPatterns { From 22b80803852019daf96dec2f4447112d5e946ae8 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 16 Apr 2021 16:47:45 +0800 Subject: [PATCH 26/32] comments, rename fields, make method private --- .../conveyal/r5/transit/FilteredPattern.java | 14 +++++++++----- .../r5/transit/FilteredPatternCache.java | 17 +++++++++++------ .../conveyal/r5/transit/FilteredPatterns.java | 16 +++++++++------- .../com/conveyal/r5/transit/TripPattern.java | 15 ++++++--------- src/main/resources/logback.xml | 3 ++- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java index 095033386..5ef398791 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPattern.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPattern.java @@ -7,7 +7,13 @@ import java.util.BitSet; import java.util.List; - +/** + * FilteredPatterns correspond to a single specific TripPattern, indicating all the trips running on a particular day. + * TripPatterns contain all the trips on a route that follow the same stop sequence. This often includes trips on + * different days of the week or special schedules where vehicles travel faster or slower. By filtering down to only + * those trips running on a particular day (a particular set of service codes), we usually get a smaller set of trips + * with no overtaking, which enables certain optimizations and is more efficient for routing. + */ public class FilteredPattern { private static Logger LOG = LoggerFactory.getLogger(FilteredPattern.class); @@ -25,9 +31,8 @@ public class FilteredPattern { public boolean noScheduledOvertaking; /** - * Copy a TripPattern, ignoring fields not used in Raptor routing. The source pattern's trip schedules are - * filtered to exclude trips not active in the supplied set of services, then divided into separate - * scheduled and frequency trip lists. Finally, check the runningScheduledTrips for overtaking. + * Filter the trips in a source TripPattern, excluding trips not active in the supplied set of services, and + * dividing them into separate scheduled and frequency trip lists. Check the runningScheduledTrips for overtaking. */ public FilteredPattern (TripPattern source, BitSet servicesActive) { for (TripSchedule schedule : source.tripSchedules) { @@ -39,7 +44,6 @@ public FilteredPattern (TripPattern source, BitSet servicesActive) { } } } - // Check whether any running trip on this pattern overtakes another noScheduledOvertaking = true; for (int i = 0; i < runningScheduledTrips.size() - 1; i++) { diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java b/src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java index 3d76dd04d..712d1d6c3 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java @@ -9,7 +9,7 @@ import java.util.EnumSet; /** - * Stores the relevant patterns and trips based on the transit modes and date in an analysis request. + * Stores the patterns and trips relevant for routing based on the transit modes and date in an analysis request. * We can't just cache the single most recently used filtered patterns, because a worker might need to simultaneously * handle two requests for the same scenario on different dates or with different modes. * @@ -21,7 +21,12 @@ */ public class FilteredPatternCache { + /** + * All FilteredPatterns stored in this cache will be derived from this single TransitLayer representing a single + * scenario, but for different unique combinations of (transitModes, services). + */ private final TransitLayer transitLayer; + private final LoadingCache cache; public FilteredPatternCache (TransitLayer transitLayer) { @@ -32,14 +37,14 @@ public FilteredPatternCache (TransitLayer transitLayer) { } // TODO replace all keys and tuples with Java 16/17 Records - public static class Key extends Tuple2, BitSet> { - public Key (EnumSet transitModes, BitSet bitSet) { - super(transitModes, bitSet); + private static class Key extends Tuple2, BitSet> { + public Key (EnumSet transitModes, BitSet servicesActive) { + super(transitModes, servicesActive); } } - public FilteredPatterns get (EnumSet transitModes, BitSet bitSet) { - return cache.get(new Key(transitModes, bitSet)); + public FilteredPatterns get (EnumSet transitModes, BitSet servicesActive) { + return cache.get(new Key(transitModes, servicesActive)); } } diff --git a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java index 6c5f65cdc..59173a0e3 100644 --- a/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java +++ b/src/main/java/com/conveyal/r5/transit/FilteredPatterns.java @@ -11,16 +11,18 @@ import static com.conveyal.r5.transit.TransitLayer.getTransitModes; /** - * Associate filtered patterns within a particular TransportNetwork (scenario) with the criteria (transit modes and - * active services) used to filter them. Many patterns contain a mixture of trips from different days, and those trips - * appear to overtake one another if we do not filter them down. Filtering allows us to flag more effectively which - * patterns have no overtaking, which is useful because departure time searches can be then optimized for patterns with - * no overtaking. All trips in a pattern are defined to be on same route, and GTFS allows only one mode per route. + * Holds all the FilteredPatterns instances for a particular TransitLayer (scenario) given a particular set of + * filtering criteria (transit modes and active services). There is one FilteredPattern instance for each TripPattern + * that is present in the filtered TransitLayer. Many TripPatterns contain a mixture of trips from different days, + * and those trips appear to overtake one another if we do not filter them down. Filtering allows us to flag more + * effectively which patterns have no overtaking, which is useful because departure time searches can be then optimized + * for patterns with no overtaking. All trips in a TripPattern are defined to be on same route, and GTFS allows only one + * mode per route. */ public class FilteredPatterns { /** - * List with the same length and indexes as the unfiltered tripPatterns. + * List with the same length and indexes as the unfiltered TripPatterns in the input TransitLayer. * Patterns that do not meet the mode/services filtering criteria are recorded as null. */ public final List patterns; @@ -32,7 +34,7 @@ public class FilteredPatterns { public BitSet runningScheduledPatterns = new BitSet(); /** - * Construct a FilteredPatterns from the given transitLayer, filtering for the specified modes and active services. + * Construct FilteredPatterns from the given TransitLayer, filtering for the specified modes and active services. * It's tempting to use List.of() or Collectors.toUnmodifiableList() but these cause an additional array copy. */ public FilteredPatterns (TransitLayer transitLayer, EnumSet modes, BitSet services) { diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index faa4e0e82..7c7e08224 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -22,8 +22,8 @@ import java.util.stream.StreamSupport; /** + * All the Trips on the same Route that have the same sequence of stops, with the same pickup/dropoff options. * This is like a Transmodel JourneyPattern. - * All the trips on the same Route that have the same sequence of stops, with the same pickup/dropoff options. */ public class TripPattern implements Serializable, Cloneable { @@ -33,7 +33,7 @@ public class TripPattern implements Serializable, Cloneable { * This is the ID of this trip pattern _in the original transport network_. This is important because if it were the * ID in this transport network the ID would depend on the order of application of scenarios, and because this ID is * used to map results back to the original network. - * TODO clarify this comment. What is the "original" transport network? This field doesn't seem to be used anywhere. + * TODO This concept of an "original" transport network may be obsolete, this field doesn't seem to be used anywhere. */ public int originalId; @@ -45,8 +45,7 @@ public class TripPattern implements Serializable, Cloneable { public PickDropType[] dropoffs; public BitSet wheelchairAccessible; // One bit per stop - /** TripSchedules for all trips following this pattern, sorted in ascending order by time of departure from first - * stop */ + /** TripSchedules for all trips in this pattern, sorted in ascending order by time of departure from first stop. */ public List tripSchedules = new ArrayList<>(); /** GTFS shape for this pattern. Should be left null in non-customer-facing applications */ @@ -68,8 +67,8 @@ public class TripPattern implements Serializable, Cloneable { public BitSet servicesActive = new BitSet(); /** - * index of this route in TransitLayer data. -1 if detailed route information has not been loaded - * TODO clarify what "this route" means. The route of this tripPattern? + * The index of this TripPatterns's route in the TransitLayer, or -1 if not yet loaded. + * Do we really want/need this redundant representation of routeId? */ public int routeIndex = -1; @@ -180,9 +179,7 @@ public String toStringDetailed (TransitLayer transitLayer) { return sb.toString(); } - /** - * @return true when none of the supplied tripIds are on this pattern. - */ + /** @return true when none of the supplied tripIds are on this pattern. */ public boolean containsNoTrips(Set tripIds) { return this.tripSchedules.stream().noneMatch(ts -> tripIds.contains(ts.tripId)); } diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml index 9a893278c..d1fb15d9d 100644 --- a/src/main/resources/logback.xml +++ b/src/main/resources/logback.xml @@ -1,4 +1,4 @@ - +