diff --git a/src/main/java/com/conveyal/gtfs/GTFSCache.java b/src/main/java/com/conveyal/gtfs/GTFSCache.java index 6a5b27cbd..35ed22cd4 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSCache.java +++ b/src/main/java/com/conveyal/gtfs/GTFSCache.java @@ -48,10 +48,10 @@ public class GTFSCache implements Component { // The following two caches hold spatial indexes of GTFS geometries for generating Mapbox vector tiles, one spatial // index per feed keyed on BundleScopedFeedId. They could potentially be combined such that cache values are a // compound type holding two indexes, or cache values are a single index containing a mix of different geometry - // types that are filtered on iteration. They could also be integreated into the GTFSFeed values of the main - // GTFSCache#cache. However GTFSFeed is already a very long class, and we may want to tune eviction parameters + // types that are filtered on iteration. They could also be integrated into the GTFSFeed values of the main + // GTFSCache#cache. However, GTFSFeed is already a very long class, and we may want to tune eviction parameters // separately for GTFSFeed and these indexes. While GTFSFeeds are expected to incur constant memory use, the - // spatial indexes are potentially unlimited in size and we may want to evict them faster or limit their quantity. + // spatial indexes are potentially unlimited in size, so we may want to evict them faster or limit their quantity. // We have decided to keep them as separate caches until we're certain of the chosen eviction tuning parameters. /** A cache of spatial indexes of TripPattern shapes, keyed on the BundleScopedFeedId. */ @@ -127,6 +127,8 @@ public FileStorageKey getFileKey (String id, String extension) { // The feedId of the GTFSFeed objects may not be unique - we can have multiple versions of the same feed // covering different time periods, uploaded by different users. Therefore we record another ID here that is // known to be unique across the whole application - the ID used to fetch the feed. + // NOTE as of 2023, this is no longer true. All uploaded feeds have assigned unique UUIDs so as far as I know + // they can't collide, we don't need this uniqueId field, and we may not even need bundle-scoped feed IDs. feed.uniqueId = id; return feed; } diff --git a/src/main/java/com/conveyal/gtfs/GTFSFeed.java b/src/main/java/com/conveyal/gtfs/GTFSFeed.java index 83236ad01..e001b42ce 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSFeed.java +++ b/src/main/java/com/conveyal/gtfs/GTFSFeed.java @@ -85,16 +85,18 @@ public class GTFSFeed implements Cloneable, Closeable { /** The MapDB database handling persistence of Maps to a pair of disk files behind the scenes. */ private DB db; - /** An ID (sometimes declared by the feed itself) which may remain the same across successive feed versions. */ + /** + * An ID (sometimes declared by the feed itself) which may remain the same across successive feed versions. + * In R5 as of 2023 this is always overwritten with a unique UUID to avoid problems with successive feed versions + * or edited/modified versions of the same feeds. + */ public String feedId; /** - * This field was merged in from the wrapper FeedSource. It is a unique identifier for this particular GTFS file. - * Successive versions of the data for the same operators, or even different copies of the same operator's data - * uploaded by different people, should have different uniqueIds. - * In practice this is mostly copied into WrappedGTFSEntity instances used in the Analysis GraphQL API. + * In R5 as of 2023, this field will contain the bundle-scoped feed ID used to fetch the feed object from the + * GTFSCache (but is not present on disk or before saving - only after it's been reloaded from a file by the cache). */ - public transient String uniqueId; // set this to feedId until it is overwritten, to match FeedSource behavior + public transient String uniqueId; // All tables below should be MapDB maps so the entire GTFSFeed is persistent and uses constant memory. diff --git a/src/main/java/com/conveyal/r5/analyst/Grid.java b/src/main/java/com/conveyal/r5/analyst/Grid.java index ad9faf1ed..957d4235d 100644 --- a/src/main/java/com/conveyal/r5/analyst/Grid.java +++ b/src/main/java/com/conveyal/r5/analyst/Grid.java @@ -53,6 +53,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -170,7 +171,8 @@ public List getPixelWeights (Geometry geometry, boolean relativeToP double area = geometry.getArea(); if (area < 1e-12) { - throw new IllegalArgumentException("Feature geometry is too small"); + LOG.warn("Discarding feature. Its area is too small to serve as a denominator ({} square degrees).", area); + return Collections.EMPTY_LIST; } if (area > MAX_FEATURE_AREA_SQ_DEG) { diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java index 91563d55f..2add61702 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java @@ -498,14 +498,6 @@ protected void handleOneRegionalTask (RegionalTask task) throws Throwable { oneOriginResult = new OneOriginResult(null, new AccessibilityResult(task), null, null); } - // Post-process the OneOriginResult to filter paths down to only those passing through the selected links. - // The set of routes and stop pairs concerned are precalculated and retained on per regional analysis. - // The first thing to do is specify the point of interest on the request. selectedLink: {lat, lon, radiusMeters} - // Without precomputing anything ... just do the geometric calculations every time. And memoize the results. - transportNetwork.transitLayer.tripPatterns.getFirst().shape; - transportNetwork.transitLayer.tripPatterns.getFirst().getHopGeometries(); - - // Accumulate accessibility results, which will be returned to the backend in batches. // For most regional analyses, this is an accessibility indicator value for one of many origins, // but for static sites the indicator value is not known, it is computed in the UI. We still want to return 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 7d523c4e4..bdc9c25aa 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -2,6 +2,7 @@ import com.conveyal.r5.analyst.StreetTimesAndModes; import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.TripPattern; import com.conveyal.r5.transit.path.Path; import com.conveyal.r5.transit.path.PatternSequence; import com.conveyal.r5.transit.path.RouteSequence; @@ -10,7 +11,11 @@ import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; import org.apache.commons.lang3.ArrayUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.awt.*; +import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collection; import java.util.Comparator; @@ -32,6 +37,8 @@ public class PathResult { + private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + /** * The maximum number of destinations for which we'll generate detailed path information in a single request. * Detailed path information was added on to the original design, which returned a simple grid of travel times. @@ -41,12 +48,14 @@ public class PathResult { public static int maxDestinations = 5000; private final int nDestinations; + /** * Array with one entry per destination. Each entry is a map from a "path template" to the associated iteration * details. For now, the path template is a route-based path ignoring per-iteration details such as wait time. * With additional changes, patterns could be collapsed further to route combinations or modes. */ public final Multimap[] iterationsForPathTemplates; + private final TransitLayer transitLayer; public static String[] DATA_COLUMNS = new String[]{ @@ -83,6 +92,42 @@ public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) { * pattern-based keys */ public void setTarget(int targetIndex, Multimap patterns) { + + // When selected link analysis is enabled, filter down the PatternSequences to include only those passing + // through the selected links. + // TODO Maybe selectedLink should be on TransitLayer, and somehow capture the number of filtered iterations. + if (transitLayer.parentNetwork.selectedLink != null) { + final SelectedLink selectedLink = transitLayer.parentNetwork.selectedLink; + Multimap filteredPatterns = HashMultimap.create(); + for (PatternSequence patternSequence : patterns.keySet()) { + // Why do we have some null patterns lists? Walk-only routes with no transit legs? + if (patternSequence.patterns == null) { + continue; + } + boolean retain = false; + // Iterate over the three parallel arrays containing TripPattern, board stop, and alight stop indexes. + for (int ride = 0; ride < patternSequence.patterns.size(); ride++) { + int pattern = patternSequence.patterns.get(ride); + int board = patternSequence.stopSequence.boardStops.get(ride); + int alight = patternSequence.stopSequence.alightStops.get(ride); + if (selectedLink.includes(pattern, board, alight)) { + retain = true; + // String routeId = transitLayer.tripPatterns.get(pattern).routeId; + // String boardStopName = transitLayer.stopNames.get(board); + // String alightStopName = transitLayer.stopNames.get(alight); + // LOG.info("Retaining {} from {} to {}", routeId, boardStopName, alightStopName); + break; + } + } + if (retain) { + Collection iterations = patterns.get(patternSequence); + filteredPatterns.putAll(patternSequence, iterations); + } + } + patterns = filteredPatterns; + } + + // The rest of this runs independent of whether a SelectedLink filtered down the patterns-iterations map. Multimap routes = HashMultimap.create(); patterns.forEach(((patternSeq, iteration) -> routes.put(new RouteSequence(patternSeq, transitLayer), iteration))); iterationsForPathTemplates[targetIndex] = routes; diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/SelectedLink.java b/src/main/java/com/conveyal/r5/analyst/cluster/SelectedLink.java index 4f636e01a..5738b9e47 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/SelectedLink.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/SelectedLink.java @@ -3,13 +3,27 @@ import com.conveyal.r5.transit.TransitLayer; import com.conveyal.r5.transit.TransportNetworkCache; import com.conveyal.r5.transit.TripPattern; +import com.conveyal.r5.util.TIntIntHashMultimap; +import com.conveyal.r5.util.TIntIntMultimap; +import gnu.trove.TIntCollection; +import gnu.trove.set.TIntSet; import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.geom.Polygon; -import java.util.List; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +import static com.conveyal.r5.common.GeometryUtils.envelopeForCircle; +import static com.conveyal.r5.common.GeometryUtils.polygonForEnvelope; /** * For Selected Link Analysis. + * This object caches a collection of every segment of every pattern that passes through a certain polygon. + * It also provides methods for quickly checking whether any leg of a public transit trip overlaps these selected segments. + * + * Implementation considerations follow: * * Simplifications: * Assumes all trips on the same pattern have the same geometry. @@ -27,49 +41,107 @@ * Do the workers have access to the GTFS files or not? * WorkerComponents has a TransportNetworkCache which is injected into the AnalysisWorker constructor. This is the only * path to access a GtfsCache which is private, so we need a method on TransportNetworkCache. + * The full path to the GtfsCache is: AnalysisWorker.networkPreloader.transportNetworkCache.gtfsCache. + * + * The best way to easily get the intended behavior is probably to create a new modification type. + * This provides a mechanism for attaching things to a network, at a point where we may still have access to the gtfs. + * And ensure that the network with extra information is properly cached for similar requests. + * We can't attach it to the raw TransportNetwork, because then the first request for that network would always need + * to be one with the selected-link behavior specified. We need to be able to apply it to a network that was loaded + * without the selected link. Or we could treat the network as mutable and write to it, which is not clean but would + * get the job done. + * + * Well, TransportNetworkCache#getNetworkForScenario is where we always apply scenarios in the worker, and that class + * has direct access to the GtfsCache. + * + * The SelectedLink instance will need to be stored/referenced: + * - Somewhere that is reachable from inside PathResult.setTarget or PathResult.summarizeIterations + * - Somewhere that is correctly scoped to where the selected-link filtering is specified (request/task or scenario) + * - Somewhere that is writable in the places where we have access to the gtfsCache + * - Somewhere that is PERSISTENT across requests - this is inherently the case for TransportNetwork but for Task we'd + * need to introduce another cache. The problem being that the base TransportNetwork's scope is too wide (could be + * used in requests with or without the SelectedLink), so it needs to be a modification on a specific scenario. + * + * PathResult is constructed with a Task and a TransitLayer. It retains only the TransitLayer but could retain both. + * In AnalysisWorker.handleAndSerializeOneSinglePointTask we still have the task context, but deeper on the stack in + * networkPreloader and then transportNetworkCache (which has gtfsCache), we have the scenario but not the task. But + * then once you go deeper into applying the scenario modifications, the gtfsCache is no longer visible. + * Anyway this doesn't feel like a modification. It feels like a parameter to the CSV path output in the task. + * The AnalysisWorker could have a Map from SelectionBox to SelectedLink (fuzzy matching keys... ugh... linear scan + * maybe). + * Also we need to tie items in the TransportNetwork to the GTFS... it feels like this should be on the TransportNetwork + * of a scenario. + * What if we scan over the incoming modifications and inject the GtfsCache onto a transient field of any + * SelectedLinkModification present? + * Basically: getting this into the Scenario makes it auto-retained, gives it a stable identity so we don't need to + * fuzzy-match it in the task to cache. That could also be done by uploading a geometry file with an ID, but ugh. + * In the future it would make sense to treat all lat/lon effectively integers (fixed-point) since it simplifies this + * kind of keying and matching. + * + * Alternatively to all this we could switch on the storage of GTFS route shapes on the network file. Then the + * modification could be applied normally without injecting a GtfsCache. + * + * Additional problem: + * The gtfsCache feed IDs (gtfs file names) are bundle-scoped but the ones in the TripPatterns are not. + * TransportNetworks and TransitLayers apparently do not retain their bundle ID. In any case they can have multiple + * feeds originally uploaded with different bundles. + * TransitLayer.feedChecksums keys are the same feed IDs prefixing TripPattern.routeId, which are the gtfsFeed.feedId, + * which is not bundle-scoped so can't be used to get a feed from gtfsCache. + * + * A network is always based on one bundle with the same ID, but the bundle config can also reference GTFS with a + * different bundle scope (originally uploaded for another bundle). So knowing the network ID is not sufficient. + * + * Based on GtfsController.bundleScopedFeedIdFromRequest, the bundleScopedFeedId is feedId_feedGroupId. So they're no + * longer based on the bundle/network ID, but the feed group. + * It seems like we wouldn't need these scopes at all since all feeds now have unique IDs. + * + * When we make the TransportNetwork from these bundles, it's always on a worker, based on the bundle's + * TransportNetworkConfig JSON file. This is in TransportNetworkCache.buildNetworkFromConfig(). + * At first it looks like the bundleScopedId is completely lost after we go through the loading process. + * But GtfsCache.get(String id) stores that key id in feed.uniqueId. That field is never read (or written) anywhere else. + * This means they're available during network creation to be retained in the TransportNetwork... but aren't retained. + * I think the only place we can get these bundle scoped feed IDs is from the TransportNetworkConfig JSON file. + * Perhaps that should be serialized into the TransportNetwork itself (check risk of serializing used Modifications). + * But in the meantime TNCache has a method to load that configuration. */ public class SelectedLink { - public SelectedLink (TransportNetworkCache transportNetworkCache, SelectionBox box) { - for (TripPattern pattern : transit.tripPatterns) { - for (LineString hopGeoms : pattern.getHopGeometries(transit)) { - - } - } - } + /** + * Contains all TripPattern inter-stop hops that pass through the selected link area for fast hash-based lookup. + * Keys are the index of a TripPattern in the TransitLayer, and values are the stop indexes in the TransitLayer. + * They are coded this way to match how they're coded in PatternSequence and minimize conversions in tight loops. + * A hop from stop A to stop B on pattern X is recorded as the mapping X -> A. Note: This is ambiguous if the stop + * appears more than once in the pattern, but PatternSequence does not seem to allow otherwise. + */ + private final TIntIntMultimap hopsInTripPattern; - public SelectedLink (SelectionBox box, TransitLayer transit) { - for (TripPattern pattern : transit.tripPatterns) { - for (LineString hopGeoms : pattern.getHopGeometries(transit)) { + // FIXME clean up or remove these notes. + // Post-process the OneOriginResult to filter paths down to only those passing through the selected links. + // The set of routes and stop pairs concerned are precalculated and retained on per regional analysis. + // The first thing to do is specify the point of interest on the request. selectedLink: {lat, lon, radiusMeters} + // Without precomputing anything ... just do the geometric calculations every time. And memoize the results. - } - } + public SelectedLink(TIntIntMultimap hopsInTripPattern) { + this.hopsInTripPattern = hopsInTripPattern; } /** - * An alternate way of specifying a bounding box where there is a central point of interest and a margin of error - * around it. Some points at the corners of the bounding box are farther away than the radius (which is the radius - * of a circle inscribed in the bounding box). + * For a given transit ride from a boardStop to an alightStop on a TripPattern, return whether that ride + * passes through this SelectedLink area. */ - public static class SelectionBox { - double lon; - double lat; - double radiusMeters; - public Envelope toEnvelope () { - Envelope env = new Envelope(); - env.expandToInclude(lon, lat); - env.expandBy(radiusMeters); // FIXME convert to lon and lat degrees + public boolean includes (int tripPattern, int boardStop, int alightStop) { + TIntCollection hops = hopsInTripPattern.get(tripPattern); + if (hops.isEmpty()) { + return false; } - } - - /** - * Uniquely identifies a segment between two subsequent stops on a TripPattern. - * This allows us to record in advance which segments pass through the link selection box. - */ - public static class TripPatternSegment { - TripPattern tripPattern; - int tripPatternIndex; // The integer ID of this tripPattern as a Raptor "route" in R5 routing. - int fromStopIndex; // Not the GTFS stop sequence number, the internal R5 index within the pattern. + for (int hop : hops.toArray()) { + // Hops are identified with the stop index at their beginning so alightStop is exclusive. + // (Alighting at a stop does not ride over the hop identified with that stop index.) + if (hop >= boardStop && hop < alightStop) { + return true; + } + } + return false; } } diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java b/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java index 012e3204a..be7a1e6e5 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java @@ -32,7 +32,7 @@ public class TransportNetworkConfig { /** ID of the OSM file, for use with OSMCache */ public String osmId; - /** IDs of the GTFS files, for use with GTFSCache */ + /** IDs of the GTFS files, for use with GTFSCache. These are "bundle-scoped" in the form feedId_feedGroupId. */ public List gtfsIds; /** The fare calculator for analysis, if any. TODO this is not yet wired up to TransportNetwork.setFareCalculator. */ diff --git a/src/main/java/com/conveyal/r5/analyst/scenario/ModificationTypeResolver.java b/src/main/java/com/conveyal/r5/analyst/scenario/ModificationTypeResolver.java index c4b95696e..599ca92bb 100644 --- a/src/main/java/com/conveyal/r5/analyst/scenario/ModificationTypeResolver.java +++ b/src/main/java/com/conveyal/r5/analyst/scenario/ModificationTypeResolver.java @@ -49,6 +49,7 @@ public class ModificationTypeResolver extends TypeIdResolverBase { .put("raster-cost", RasterCost.class) .put("shapefile-lts", ShapefileLts.class) .put("set-fare-calculator", SetFareCalculator.class) + .put("select-link", SelectLink.class) .build(); @Override diff --git a/src/main/java/com/conveyal/r5/analyst/scenario/SelectLink.java b/src/main/java/com/conveyal/r5/analyst/scenario/SelectLink.java new file mode 100644 index 000000000..26b553e32 --- /dev/null +++ b/src/main/java/com/conveyal/r5/analyst/scenario/SelectLink.java @@ -0,0 +1,146 @@ +package com.conveyal.r5.analyst.scenario; + +import com.conveyal.gtfs.GTFSCache; +import com.conveyal.gtfs.GTFSFeed; +import com.conveyal.r5.analyst.cluster.SelectedLink; +import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.TransportNetwork; +import com.conveyal.r5.transit.TripPattern; +import com.conveyal.r5.util.TIntIntHashMultimap; +import com.conveyal.r5.util.TIntIntMultimap; +import gnu.trove.set.TIntSet; +import gnu.trove.set.hash.TIntHashSet; +import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.geom.Polygon; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static com.conveyal.r5.common.GeometryUtils.envelopeForCircle; +import static com.conveyal.r5.common.GeometryUtils.polygonForEnvelope; +import static com.google.common.base.Strings.isNullOrEmpty; + +/** + * Designates all transit and/or roads passing through a given rectangle for inclusion in CSV output. + * This allows cutting down and consolidating results for some network assignment and congestion problems. + * @see SelectedLink + */ +public class SelectLink extends Modification { + + public double lon; + + public double lat; + + public double radiusMeters; + + private Polygon boxPolygon; + + private Map feedForUnscopedId; + + @Override + public boolean resolve(TransportNetwork network) { + // Convert the incoming description of the selected link area to a Geometry for computing intersections. + boxPolygon = polygonForEnvelope(envelopeForCircle(lon, lat, radiusMeters)); + // The TransportNetwork does not directly retain information on which feeds were used to create it, but each + // TripPattern retains a feed-scoped routeId in this format: String.format("%s:%s", gtfs.feedId, route.route_id) + for (TripPattern tripPattern : network.transitLayer.tripPatterns) { + String feedId = feedIdForTripPattern(tripPattern); + if (isNullOrEmpty(feedId)) { + errors.add("Could not find feed ID prefix in route ID " + tripPattern.routeId); + continue; + } + // The GTFS feed IDs are bundle-scoped, while the ones in the TripPatterns are not. + GTFSFeed feed = feedForUnscopedId.get(feedId); + if (feed == null) { + errors.add("Could not find feed for ID " + feedId); + continue; + } + } + return errors.size() > 0; + } + + + @Override + public boolean apply(TransportNetwork network) { + // Map hopsInTripPattern = new HashMap<>(); + TIntIntMultimap hopsInTripPattern = new TIntIntHashMultimap(); // This is acually a list-valued multimap. + // Path recording uses pattern index numbers rather than pattern objects, so iterate over those. + for (int patternIndex = 0; patternIndex < network.transitLayer.tripPatterns.size(); patternIndex++) { + TripPattern tripPattern = network.transitLayer.tripPatterns.get(patternIndex); + // Make a sacrificial protective copy to avoid interfering with other requests using this network. + // We're going to add shape data to this TripPattern then throw it away immediately afterward. + // Be careful not to use this clone as a key in any Maps, use the original from TransitLayer. + tripPattern = tripPattern.clone(); + String feedId = feedIdForTripPattern(tripPattern); + GTFSFeed feed = feedForUnscopedId.get(feedId); + TransitLayer.addShapeToTripPattern(feed, tripPattern); + // TransitLayer is supplied to get stop locations for straight lines (in case shapes are not present). + List hopGeometries = tripPattern.getHopGeometries(network.transitLayer); + int hopNumber = 0; + for (LineString hopGeometry : hopGeometries) { + if (boxPolygon.intersects(hopGeometry)) { + int stop = tripPattern.stops[hopNumber]; + hopsInTripPattern.put(patternIndex, stop); + } + hopNumber += 1; + } + } + // After finding all links, release GtfsFeeds which don't really belong here, to avoid memory leaks. + feedForUnscopedId = null; + network.selectedLink = new SelectedLink(hopsInTripPattern); + return errors.size() > 0; + } + + // By returning false for both affects* methods, we make a very shallow copy of the TransitNetwork for efficiency. + + @Override + public boolean affectsStreetLayer() { + return false; + } + + @Override + public boolean affectsTransitLayer() { + return false; + } + + @Override + public int getSortOrder() { + // This modification needs to be applied after any modifications affecting the transit network. + throw new UnsupportedOperationException("Method not yet implemented."); + } + + // The SAVE_SHAPES section of TransitLayer is quite complicated. + // We should not replicate all that logic again in another place. + // I had a different impression from looking somewhere else. The GtfsController accesses the GeometryCache in + // GtfsCache.patternShapes, but this just saves entire exemplar trips from end to end, not inter-stop segments. + // I was probably thrown off by TripPattern.getHopGeometries which uses LocationIndexedLine, but depends on some + // fairly complicated stop-to-shape snapping logic in the SAVE_SHAPES section of TransitLayer to pre-build fields. + // We could either re-run this code after the fact to inject the shapes into existing network files, + // or we could enable it with a network build time switch. + + /** + * Currently we do not include the GTFS shapes in the TransportNetwork, so in order to determine which routes pass + * through a given geographic area, we need access to the original GTFS data. When resolving and applying + * Modifications, the only thing supplied is the TransportNetwork itself. This is a hack, but we need to inject + * the GtfsFeed somehow. The reference can be nulled out after the modification is applied to avoid memory leaks. + * A TransportNetwork may be made from a bundle with multiple feeds, so we can't attach just one GTFSFeed. + * The TransportNetwork does not directly retain information on which feeds were used to create it, but each + * TripPattern retains a feed-scoped routeId in this format: String.format("%s:%s", gtfs.feedId, route.route_id) + * + * Potential problem: is the gtfs.feedId the bundle-scoped feed ID expected by gtfs-cache? + */ + public void injectGtfs(Map feedForUnscopedId) { + this.feedForUnscopedId = feedForUnscopedId; + } + + private String feedIdForTripPattern (TripPattern tripPattern) { + String[] parts = tripPattern.routeId.split(":"); + if (parts.length == 0) { + return null; + } else { + return parts[0]; + } + } + +} diff --git a/src/main/java/com/conveyal/r5/analyst/scenario/package-info.java b/src/main/java/com/conveyal/r5/analyst/scenario/package-info.java index 3b6b1c9f9..00a68c747 100644 --- a/src/main/java/com/conveyal/r5/analyst/scenario/package-info.java +++ b/src/main/java/com/conveyal/r5/analyst/scenario/package-info.java @@ -2,5 +2,10 @@ * This package contains classes for modeling transport scenarios as an ordered series of modifications to be applied * to an underlying baseline graph. It is used for impact analysis: the interactive creation and comparison of the * accessibility effects of modifications to a transport network. + * + * It is important to note that each of these classes has a corresponding model for use in the UI and database. + * Each type of modification has an R5 version (which is more stable over time) and a UI/DB version which can be + * changed more freely. Conversion to the R5 types in this package is performed by an implementation of + * com.conveyal.analysis.models.Modification.toR5(). */ package com.conveyal.r5.analyst.scenario; \ No newline at end of file diff --git a/src/main/java/com/conveyal/r5/common/GeometryUtils.java b/src/main/java/com/conveyal/r5/common/GeometryUtils.java index c41eef536..42cfd2665 100644 --- a/src/main/java/com/conveyal/r5/common/GeometryUtils.java +++ b/src/main/java/com/conveyal/r5/common/GeometryUtils.java @@ -8,6 +8,7 @@ import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LineSegment; +import org.locationtech.jts.geom.Polygon; import static com.conveyal.r5.streets.VertexStore.fixedDegreesToFloating; import static com.conveyal.r5.streets.VertexStore.floatingDegreesToFixed; @@ -16,6 +17,9 @@ /** * Reimplementation of OTP GeometryUtils, using copied code where there are not licensing concerns. * Also contains reusable methods for validating WGS84 envelopes and latitude and longitude values. + * + * FIXME we have two geometry util classes com.conveyal.r5.common.GeometryUtils and com.conveyal.gtfs.util.GeometryUtil + * Each has its own GeometryFactory. */ public class GeometryUtils { public static final GeometryFactory geometryFactory = new GeometryFactory(); @@ -172,4 +176,33 @@ thingBeingChecked, roughWgsEnvelopeArea(envelope), MAX_BOUNDING_BOX_AREA_SQ_KM } } + /** + * Given floating point WGS84 latitude and longitude and a radius in meters, create an envelope inscribing that + * circle. + */ + public static Envelope envelopeForCircle (double lon, double lat, double radiusMeters) { + checkLat(lat); + checkLon(lon); + checkArgument(radiusMeters < 1000, "Radius must be less than 1km."); + Envelope envelope = new Envelope(); + envelope.expandToInclude(lon, lat); + double latExpansion = SphericalDistanceLibrary.metersToDegreesLatitude(radiusMeters); + double lonExpansion = SphericalDistanceLibrary.metersToDegreesLongitude(radiusMeters, lat); + envelope.expandBy(lonExpansion, latExpansion); + return envelope; + } + + /** + * Some geometry operations such as intersections and inclusion can only operate on Geometries, not Envelopes. + */ + public static Polygon polygonForEnvelope (Envelope env) { + return geometryFactory.createPolygon(new Coordinate[] { + new Coordinate(env.getMinX(), env.getMinY()), + new Coordinate(env.getMinX(), env.getMaxY()), + new Coordinate(env.getMaxX(), env.getMaxY()), + new Coordinate(env.getMaxX(), env.getMinY()), + new Coordinate(env.getMinX(), env.getMinY()) + }); + } + } diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 34eab0ea5..fa708e7bb 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -63,6 +63,10 @@ public class TransitLayer implements Serializable, Cloneable { /** Maximum distance to record in distance tables, in meters. */ public static final int WALK_DISTANCE_LIMIT_METERS = 2000; + /** + * If this is true, the detailed shapes from GTFS will be retained in the TransitLayer. + * If false, straight-line shapes will be used between stops. + */ public static final boolean SAVE_SHAPES = false; /** @@ -230,7 +234,7 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx LOG.info("Creating trip patterns and schedules."); - // These are temporary maps used only for grouping purposes. + // These are temporary maps used only for grouping purposes within this one GTFS feed, not all feeds in a bundle. Map tripPatternForPatternId = new HashMap<>(); Multimap tripsForBlock = HashMultimap.create(); @@ -242,7 +246,6 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx Trip trip = gtfs.trips.get(tripId); Route route = gtfs.routes.get(trip.route_id); // Construct the stop pattern and schedule for this trip. - String scopedRouteId = String.join(":", gtfs.feedId, trip.route_id); TIntList arrivals = new TIntArrayList(TYPICAL_NUMBER_OF_STOPS_PER_TRIP); TIntList departures = new TIntArrayList(TYPICAL_NUMBER_OF_STOPS_PER_TRIP); TIntList stopSequences = new TIntArrayList(TYPICAL_NUMBER_OF_STOPS_PER_TRIP); @@ -270,9 +273,10 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx continue TRIPS; } - if (previousDeparture == st.arrival_time) { //Teleportation: arrive at downstream stop immediately after departing upstream - //often the result of a stop_times input with time values rounded to the nearest minute. - //TODO check if the distance of the hop is reasonably traveled in less than 60 seconds, which may vary by mode. + if (previousDeparture == st.arrival_time) { + // Teleportation: arrive at downstream stop immediately after departing upstream + // often the result of a stop_times input with time values rounded to the nearest minute. + // TODO check if the distance of the hop is reasonably traveled in less than 60 seconds, which may vary by mode. nZeroDurationHops++; } @@ -288,80 +292,24 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx String patternId = gtfs.patternForTrip.get(tripId); - // Note that patternIds are UUIDs so should be unique even across different feeds. + // Fetch or make the internal R5 TripPattern for the given gtfs-lib Pattern. + // Note that gtfs-lib Pattern IDs are UUIDs, so should be unique even across different feeds. + // The first time we encounter a trip with a given Pattern ID, we make the TripPattern. + // Certain characteristics of the pattern are derived from that Trip (notably the Shape) but could in + // theory be different on each trip in the pattern. In this edge case, encounter order of trips matters. TripPattern tripPattern = tripPatternForPatternId.get(patternId); if (tripPattern == null) { tripPattern = new TripPattern(String.format("%s:%s", gtfs.feedId, route.route_id), stopTimes, indexForUnscopedStopId); - - // if we haven't seen the route yet _from this feed_ (as IDs are only feed-unique) - // create it. if (level == LoadLevel.FULL) { + // If we haven't seen the route yet _from this feed_ (as IDs are only feed-unique) create it. if (!routeIndexForRoute.containsKey(trip.route_id)) { int routeIndex = routes.size(); RouteInfo ri = new RouteInfo(route, gtfs.agency.get(route.agency_id)); routes.add(ri); routeIndexForRoute.put(trip.route_id, routeIndex); } - tripPattern.routeIndex = routeIndexForRoute.get(trip.route_id); - - if (trip.shape_id != null && SAVE_SHAPES) { - Shape shape = gtfs.getShape(trip.shape_id); - if (shape == null) LOG.warn("Shape {} for trip {} was missing", trip.shape_id, trip.trip_id); - else { - // TODO this will not work if some trips in the pattern don't have shapes - tripPattern.shape = shape.geometry; - - // project stops onto shape - boolean stopsHaveShapeDistTraveled = StreamSupport.stream(stopTimes.spliterator(), false) - .noneMatch(st -> Double.isNaN(st.shape_dist_traveled)); - boolean shapePointsHaveDistTraveled = DoubleStream.of(shape.shape_dist_traveled) - .noneMatch(Double::isNaN); - - LinearLocation[] locations; - - if (stopsHaveShapeDistTraveled && shapePointsHaveDistTraveled) { - // create linear locations from dist traveled - locations = StreamSupport.stream(stopTimes.spliterator(), false) - .map(st -> { - double dist = st.shape_dist_traveled; - - int segment = 0; - - while (segment < shape.shape_dist_traveled.length - 2 && - dist > shape.shape_dist_traveled[segment + 1] - ) segment++; - - double endSegment = shape.shape_dist_traveled[segment + 1]; - double beginSegment = shape.shape_dist_traveled[segment]; - double proportion = (dist - beginSegment) / (endSegment - beginSegment); - - return new LinearLocation(segment, proportion); - }).toArray(LinearLocation[]::new); - } else { - // naive snapping - LocationIndexedLineInLocalCoordinateSystem line = - new LocationIndexedLineInLocalCoordinateSystem(shape.geometry.getCoordinates()); - - locations = StreamSupport.stream(stopTimes.spliterator(), false) - .map(st -> { - Stop stop = gtfs.stops.get(st.stop_id); - return line.project(new Coordinate(stop.stop_lon, stop.stop_lat)); - }) - .toArray(LinearLocation[]::new); - } - - tripPattern.stopShapeSegment = new int[locations.length]; - tripPattern.stopShapeFraction = new float[locations.length]; - - for (int i = 0; i < locations.length; i++) { - tripPattern.stopShapeSegment[i] = locations[i].getSegmentIndex(); - tripPattern.stopShapeFraction[i] = (float) locations[i].getSegmentFraction(); - } - } - } } - tripPatternForPatternId.put(patternId, tripPattern); tripPattern.originalId = tripPatterns.size(); tripPatterns.add(tripPattern); @@ -388,11 +336,21 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx } LOG.info("Done creating {} trips on {} patterns.", nTripsAdded, tripPatternForPatternId.size()); + // Store shapes and associated linear locations of stops on the set of patterns created by this one GTFS feed. + // Instead, we usually perform this one pattern at a time on demand, only when needed by modifications. + if (SAVE_SHAPES) { + LOG.info("Referencing stop locations to shapes and breaking shapes into per-hop segments..."); + for (TripPattern tripPattern : tripPatternForPatternId.values()) { + addShapeToTripPattern(gtfs, tripPattern); + } + LOG.info("Done processing shapes."); + } + LOG.info("{} zero-duration hops found.", nZeroDurationHops); LOG.info("Chaining trips together according to blocks to model interlining..."); // Chain together trips served by the same vehicle that allow transfers by simply staying on board. - // Elsewhere this is done by grouping by (serviceId, blockId) but this is not supported by the spec. + // This is done elsewhere by grouping by (serviceId, blockId) but this is not supported by the spec. // Discussion started on gtfs-changes. tripsForBlock.asMap().forEach((blockId, trips) -> { TripSchedule[] schedules = trips.toArray(new TripSchedule[trips.size()]); @@ -412,9 +370,8 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx LOG.info("Finding the approximate center of the transport network..."); findCenter(gtfs.stops.values()); - //Set transportNetwork timezone - //If there are no agencies (which is strange) it is GMT - //Otherwise it is set to first valid agency timezone and warning is shown if agencies have different timezones + // Set TransportNetwork timezone. If there are no agencies (which is strange) it defaults to GMT. + // Otherwise, it is set to first valid agency timezone. Warning is shown if agencies have different timezones. if (gtfs.agency.size() == 0) { timeZone = ZoneId.of("GMT"); LOG.warn("graph contains no agencies; API request times will be interpreted as GMT."); @@ -469,6 +426,103 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx } + /** + * This code has been factored out of loadFromGtfs because it adds a lot of data to the TransportNetwork. We usually + * don't run it when creating the TransportNetwork, but instead run it as needed on single TripPatterns to save + * space. Factoring pieces out of loadFromGtfs also makes that method a bit more readable as it's very long. + * TODO test how much this actually increases feed size and consider enabling it when creating TransportNetworks. + */ + public static void addShapeToTripPattern ( + GTFSFeed gtfsFeed, + TripPattern tripPattern + ) { + // First, find an exemplar trip that is representative of the TripPattern. + boolean foundExemplarTrip = false; + Trip trip = null; + Iterable stopTimes = null; + for (TripSchedule tripSchedule : tripPattern.tripSchedules) { + trip = gtfsFeed.trips.get(tripSchedule.tripId); + if (trip == null) { + LOG.warn("Could not find trip for ID " + tripSchedule.tripId); + continue; + } + if (trip.shape_id == null) { + continue; + } + try { + stopTimes = gtfsFeed.getInterpolatedStopTimesForTrip(tripSchedule.tripId); + } catch (GTFSFeed.FirstAndLastStopsDoNotHaveTimes e) { + continue; + } + // All checks succeeded, record the information from this trip as the exemplar for the pattern. + foundExemplarTrip = true; + break; + } + // Could add a possibly slow check: get each trip, check if exemplarTrip.shapeId equals shape in each trip. + // LOG.warn(String.format("Multiple trips in the same TripPattern have different shapes (e.g. %s and %s)") + + if (!foundExemplarTrip) { + LOG.warn("Did not find any exemplar trip with usable Shape and StopTimes for pattern " + tripPattern); + return; + } + // This is assigned outside the loop only to make it final, as required by lambdas below. + final Shape shape = gtfsFeed.getShape(trip.shape_id); + if (shape == null) { + LOG.error("Shape {} for trip {} was missing", trip.shape_id, trip.trip_id); + return; + } + tripPattern.shape = shape.geometry; + + // Project stop locations onto the shape geometry. + + boolean stopsHaveShapeDistTraveled = StreamSupport.stream(stopTimes.spliterator(), false) + .noneMatch(st -> Double.isNaN(st.shape_dist_traveled)); + boolean shapePointsHaveDistTraveled = DoubleStream.of(shape.shape_dist_traveled) + .noneMatch(Double::isNaN); + + LinearLocation[] locations; + + if (stopsHaveShapeDistTraveled && shapePointsHaveDistTraveled) { + // Create linear locations from the distance traveled along the shape. + final Shape finalShape = shape; + locations = StreamSupport.stream(stopTimes.spliterator(), false) + .map(st -> { + double dist = st.shape_dist_traveled; + + int segment = 0; + + while (segment < finalShape.shape_dist_traveled.length - 2 && + dist > finalShape.shape_dist_traveled[segment + 1] + ) segment++; + + double endSegment = finalShape.shape_dist_traveled[segment + 1]; + double beginSegment = finalShape.shape_dist_traveled[segment]; + double proportion = (dist - beginSegment) / (endSegment - beginSegment); + + return new LinearLocation(segment, proportion); + }).toArray(LinearLocation[]::new); + } else { + // naive snapping + LocationIndexedLineInLocalCoordinateSystem line = + new LocationIndexedLineInLocalCoordinateSystem(shape.geometry.getCoordinates()); + + locations = StreamSupport.stream(stopTimes.spliterator(), false) + .map(st -> { + Stop stop = gtfsFeed.stops.get(st.stop_id); + return line.project(new Coordinate(stop.stop_lon, stop.stop_lat)); + }) + .toArray(LinearLocation[]::new); + } + + tripPattern.stopShapeSegment = new int[locations.length]; + tripPattern.stopShapeFraction = new float[locations.length]; + + for (int i = 0; i < locations.length; i++) { + tripPattern.stopShapeSegment[i] = locations[i].getSegmentIndex(); + tripPattern.stopShapeFraction[i] = (float) locations[i].getSegmentFraction(); + } + } + // The median of all stopTimes would be best but that involves sorting a huge list of numbers. // So we just use the mean of all stops for now. private void findCenter (Collection stops) { diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java index 3e3cb3720..f9014e78a 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java @@ -4,6 +4,7 @@ import com.conveyal.osmlib.OSM; import com.conveyal.r5.analyst.LinkageCache; import com.conveyal.r5.analyst.WebMercatorGridPointSet; +import com.conveyal.r5.analyst.cluster.SelectedLink; import com.conveyal.r5.analyst.error.TaskError; import com.conveyal.r5.analyst.fare.InRoutingFareCalculator; import com.conveyal.r5.analyst.scenario.Scenario; @@ -91,6 +92,15 @@ public class TransportNetwork implements Serializable { /** Information about the effects of apparently correct scenario application, null on a base network */ public transient List scenarioApplicationInfo; + /** + * If non-null, CSV path outputs will be filtered down to only include paths passing through this one specific area. + * This is not really a characteristic of the network, it's more like a request-scoped parameter like + * AnalysisRequest.recordPaths. However, it requires some data structures that are slow to build and need to be + * retained across many requests, so it's modeled as a scenario modification. This is not ideal but it works. + * Recall that modifications can be applied at network build time. + */ + public transient SelectedLink selectedLink; + /** * Build some simple derived index tables that are not serialized with the network. * Distance tables and street spatial indexes are now serialized with the network. diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java b/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java index b7f1f1d8f..14fadf4f8 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java @@ -6,11 +6,13 @@ import com.conveyal.file.FileStorageKey; import com.conveyal.file.FileUtils; import com.conveyal.gtfs.GTFSCache; +import com.conveyal.gtfs.GTFSFeed; import com.conveyal.r5.analyst.cluster.ScenarioCache; import com.conveyal.r5.analyst.cluster.TransportNetworkConfig; import com.conveyal.r5.analyst.scenario.Modification; import com.conveyal.r5.analyst.scenario.RasterCost; import com.conveyal.r5.analyst.scenario.Scenario; +import com.conveyal.r5.analyst.scenario.SelectLink; import com.conveyal.r5.analyst.scenario.ShapefileLts; import com.conveyal.r5.common.JsonUtilities; import com.conveyal.r5.kryo.KryoNetworkSerializer; @@ -32,6 +34,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import java.util.zip.ZipEntry; @@ -133,6 +136,25 @@ public synchronized TransportNetwork getNetworkForScenario (String networkId, St LOG.debug("Applying scenario to base network..."); // Fetch the full scenario if an ID was specified. Scenario scenario = resolveScenario(networkId, scenarioId); + + // EXPERIMENTAL: Allow select-link modification type to read GTFSFeeds, to see all shape geometries. + for (Modification modification : scenario.modifications) { + if (modification instanceof SelectLink) { + // This seems to be the only way to see the original bundle-scoped feed IDs and reverse-map them. + TransportNetworkConfig networkConfig = loadNetworkConfig(networkId); + Map feedForUnscopedId = new HashMap<>(); + for (String bundleScopedFeedId : networkConfig.gtfsIds) { + GTFSFeed feed = gtfsCache.get(bundleScopedFeedId); + String unscopedFeedId = feed.feedId; // The unscoped ID known inside the TripPatterns + GTFSFeed existingValue = feedForUnscopedId.put(unscopedFeedId, feed); + if (existingValue != null) { + LOG.warn("Feed ID collision when removing bundle/feedGroup scope from IDs."); + } + } + ((SelectLink) modification).injectGtfs(feedForUnscopedId); + } + } + // Apply any scenario modifications to the network before use, performing protective copies where necessary. // We used to prepend a filter to the scenario, removing trips that are not running during the search time window. // However, because we are caching transportNetworks with scenarios already applied to them, we can’t use @@ -159,7 +181,14 @@ private static FileStorageKey getR5NetworkFileStorageKey (String networkId) { return new FileStorageKey(BUNDLES, getR5NetworkFilename(networkId)); } - /** @return the network configuration (AKA manifest) for the given network ID, or null if no config file exists. */ + /** + * Each bundle has a corresponding JSON file listing its OSM and GTFS IDs among other things. Its base name is the + * UUID of the bundle, which is also the UUID of any derived TransportNetworks. + * There is a one-to-one correspondence between bundles and TransportNetworks: the ID of a TransportNetwork, as + * well as the base of its filename, are identical to the ID of its corresponding bundle of input data. + * TransportNetworks are always built on the workers, based on the contents of the JSON network config. + * @return the network configuration (AKA manifest) for the given network ID, or null if no config file exists. + */ private TransportNetworkConfig loadNetworkConfig (String networkId) { FileStorageKey configFileKey = new FileStorageKey(BUNDLES, getNetworkConfigFilename(networkId)); if (!fileStorage.exists(configFileKey)) { @@ -167,7 +196,7 @@ private TransportNetworkConfig loadNetworkConfig (String networkId) { } File configFile = fileStorage.getFile(configFileKey); try { - // Use lenient mapper to mimic behavior in objectFromRequestBody. + // Use lenient mapper to mimic behavior in objectFromRequestBody. Method closes the file when complete. return JsonUtilities.lenientObjectMapper.readValue(configFile, TransportNetworkConfig.class); } catch (IOException e) { throw new RuntimeException("Error reading TransportNetworkConfig. Does it contain new unrecognized fields?", e); @@ -305,7 +334,7 @@ private TransportNetwork buildNetworkFromConfig (TransportNetworkConfig config) // Apply modifications embedded in the TransportNetworkConfig JSON final Set> ACCEPT_MODIFICATIONS = Set.of( - RasterCost.class, ShapefileLts.class + RasterCost.class, ShapefileLts.class, SelectLink.class ); if (config.modifications != null) { // Scenario scenario = new Scenario(); diff --git a/src/main/java/com/conveyal/r5/transit/TripPattern.java b/src/main/java/com/conveyal/r5/transit/TripPattern.java index 7c7e08224..8c9abe2d8 100644 --- a/src/main/java/com/conveyal/r5/transit/TripPattern.java +++ b/src/main/java/com/conveyal/r5/transit/TripPattern.java @@ -3,6 +3,7 @@ import com.conveyal.gtfs.model.StopTime; import com.conveyal.r5.common.GeometryUtils; import com.conveyal.r5.streets.VertexStore; +import com.google.common.collect.Lists; import gnu.trove.list.TIntList; import gnu.trove.map.TObjectIntMap; import org.locationtech.jts.geom.Coordinate; @@ -92,6 +93,7 @@ public TripPattern (TIntList intStopIds) { } public TripPattern(String routeId, Iterable stopTimes, TObjectIntMap indexForUnscopedStopId) { + // FIXME We don't need to explicitly use a spliterator to make a list. Lists.newArrayList(stopTimes); List stopTimeList = StreamSupport.stream(stopTimes.spliterator(), false).collect(Collectors.toList()); int nStops = stopTimeList.size(); stops = new int[nStops]; diff --git a/src/main/java/com/conveyal/r5/transit/path/PatternSequence.java b/src/main/java/com/conveyal/r5/transit/path/PatternSequence.java index 215def067..3e377e4e0 100644 --- a/src/main/java/com/conveyal/r5/transit/path/PatternSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/PatternSequence.java @@ -9,8 +9,11 @@ * A door-to-door path that includes the patterns ridden between stops */ public class PatternSequence { - /** Pattern indexes (those used in R5 transit layer) for each transit leg */ + + /** Pattern indexes (those used in R5 TransitLayer) for each transit leg in the itinerary. */ public final TIntList patterns; + + /** Other information associated with each of the legs: board and alight stops, ride times etc. */ public final StopSequence stopSequence; /** 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..f79f273a3 100644 --- a/src/main/java/com/conveyal/r5/transit/path/StopSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/StopSequence.java @@ -11,6 +11,7 @@ /** * A door-to-door path, i.e. access/egress characteristics and transit legs (keyed on characteristics including per-leg * in-vehicle times but not specific trips/patterns/routes), which may be repeated at different departure times. + * The integer stop indexes here are the indexes within the TransitLayer, not within the TripPattern or Trip ridden. * * Instances are constructed initially from transit legs, with access and egress set in successive operations. */