Merge pull request #708 from conveyal/transfer-time

Trip and pattern filtering

.github/workflows/cypress-integration.yml

@@ -16,7 +16,7 @@ jobs:
       - uses: actions/checkout@v2
         with:
           repository: conveyal/analysis-ui
-          ref: 62dfe5b8d8e76a095b8f923b6458b75d3e10c2c8
+          ref: dee1f13ba891e35b859438c97e5f66a0a7347d38
           path: ui
       - uses: actions/checkout@v2
         with:

src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java

@@ -155,13 +155,18 @@ 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); // 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?
         }
     }
 
     /**
      * Returns human-readable details of path iterations, for JSON representation (e.g. in the UI console).
      */
-    List<PathIterations> getPathIterationsForDestination() {
+    public List<PathIterations> getPathIterationsForDestination() {
         checkState(iterationsForPathTemplates.length == 1, "Paths were stored for multiple " +
                 "destinations, but only one is being requested");
         List<PathIterations> detailsForDestination = new ArrayList<>();

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");

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;
+
+/**
+ * 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);
+
+    /**
+     * 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<TripSchedule> runningScheduledTrips = new ArrayList<>();
+
+    /** Frequency-based trips active in a particular set of GTFS services */
+    public List<TripSchedule> runningFrequencyTrips = new ArrayList<>();
+
+    /** If no active schedule-based trip of this filtered pattern overtakes another. */
+    public boolean noScheduledOvertaking;
+
+    /**
+     * 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) {
+            if (servicesActive.get(schedule.serviceCode)) {
+                if (schedule.headwaySeconds == null) {
+                    runningScheduledTrips.add(schedule);
+                } else {
+                    runningFrequencyTrips.add(schedule);
+                }
+            }
+        }
+        // Check whether any running trip on this pattern overtakes another
+        noScheduledOvertaking = true;
+        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;
+    }
+
+}

src/main/java/com/conveyal/r5/transit/FilteredPatternCache.java

@@ -0,0 +1,50 @@
+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 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.
+ *
+ * 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 {
+
+    /**
+     * 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<Key, FilteredPatterns> 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
+    private static class Key extends Tuple2<EnumSet<TransitModes>, BitSet> {
+        public Key (EnumSet<TransitModes> transitModes, BitSet servicesActive) {
+            super(transitModes, servicesActive);
+        }
+    }
+
+    public FilteredPatterns get (EnumSet<TransitModes> transitModes, BitSet servicesActive) {
+        return cache.get(new Key(transitModes, servicesActive));
+    }
+
+}

src/main/java/com/conveyal/r5/transit/FilteredPatterns.java

@@ -0,0 +1,63 @@
+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;
+import java.util.EnumSet;
+import java.util.List;
+
+import static com.conveyal.r5.transit.TransitLayer.getTransitModes;
+
+/**
+ * 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 in the input TransitLayer.
+     * Patterns that do not meet the mode/services filtering criteria are recorded as null.
+     */
+    public final List<FilteredPattern> 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();
+
+    /**
+     * 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<TransitModes> modes, BitSet services) {
+        List<TripPattern> 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);
+            }
+        }
+    }
+
+}

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,9 @@ public class TransitLayer implements Serializable, Cloneable {
 
     public List<TripPattern> tripPatterns = new ArrayList<>();
 
+    /** 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();
 
@@ -748,6 +752,7 @@ public TransitLayer scenarioCopy(TransportNetwork newScenarioNetwork, boolean wi
             // the scenario that modified it. If the scenario will not affect the contents of the layer, its
             // scenarioId remains unchanged as is done in StreetLayer.
             copy.scenarioId = newScenarioNetwork.scenarioId;
+            copy.filteredPatternCache = new FilteredPatternCache(copy);
         }
         return copy;
     }

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,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 This concept of an "original" transport network may be obsolete, this field doesn't seem to be used anywhere.
      */
     public int originalId;
 
@@ -44,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<TripSchedule> tripSchedules = new ArrayList<>();
 
     /** GTFS shape for this pattern. Should be left null in non-customer-facing applications */
@@ -67,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;
 
@@ -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;
@@ -177,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<String> tripIds) {
         return this.tripSchedules.stream().noneMatch(ts -> tripIds.contains(ts.tripId));
     }
@@ -225,5 +225,4 @@ public List<LineString> getHopGeometries(TransitLayer transitLayer) {
         }
         return geometries;
     }
-
 }

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<A, B> {
+    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);
+    }
+}

src/main/resources/logback.xml

@@ -1,4 +1,4 @@
-<configuration debug="true">
+<configuration>
 
     <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
         <!-- encoders are  by default assigned the type
@@ -15,5 +15,6 @@
     <logger name="com.conveyal.osmlib" level="INFO" />
     <logger name="com.conveyal.gtfs" level="INFO" />
     <logger name="com.conveyal.r5.profile.ExecutionTimer" level="INFO"/>
+    <logger name="com.conveyal.r5.profile.FastRaptorWorker" level="INFO" />
 
 </configuration>

src/test/java/com/conveyal/r5/analyst/network/DistributionTester.java

@@ -32,4 +32,4 @@ public static void assertExpectedDistribution (Distribution expectedDistribution
         }
     }
 
-}
+}