record transfers from gtfs transfers.txt

they are not yet used in routing
deprecate point-to-point routing uses of transfer finder for clarity

Author: abyrd

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

@@ -87,13 +87,13 @@ public class TransportNetworkConfig {
      *   Should this be configurable or interact with the transfer entries?
      *
      */
-    public GtfsTransferConfig gtfsTransfers;
+    public TransferConfig transfers;
 
-    enum GtfsTransferConfig {
+    public enum TransferConfig {
         OSM_ONLY,  // Find all transfers by searching through the OSM street network
         GTFS_ONLY, // Load transfers only from transfers.txt, do not use the street network
         PER_STOP_PAIR, // Find transfers via streets for stop pairs not connected by GTFS transfers
         PER_STOP, // Find transfers via streets only from and to stops not referenced by GTFS transfers
-        PER_FEED // GTFS_ONLY for feeds with transfers.txt, OSM_ONLY for those without
+        PER_FEED // TODO: PER_FEED options, allowing different behavior for intra- and inter-feed transfers
     }
 }

src/main/java/com/conveyal/r5/kryo/KryoNetworkSerializer.java

@@ -46,14 +46,15 @@ public abstract class KryoNetworkSerializer {
      * We considered using an ISO date string as the version but that could get confusing when seen in filenames.
      *
      * History of Network Version (NV) changes (in production releases):
+     * nv4 since 2025-11: network config and transfer config retained in TransportNetwork, new GTFS transfer handling
      * nv3 since v7.0: switched to Kryo 5 serialization, WebMercatorGridPointSet now contains nested WebMercatorExtents
      * nv2 since 2022-04-05
      * nv1 since 2021-04-30: stopped rebuilding networks for every new r5 version, manually setting this version string
      *
      * When prototyping new features, use a unique identifier such as the branch or a commit ID, not sequential nvX ones.
      * This avoids conflicts when multiple changes are combined in a single production release, or some are abandoned.
      */
-    public static final String NETWORK_FORMAT_VERSION = "nv3";
+    public static final String NETWORK_FORMAT_VERSION = "nv4-testing"; // TODO change to nv4 for release
 
     public static final byte[] HEADER = "R5NETWORK".getBytes();
 

src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java

@@ -62,6 +62,7 @@
  * It can build point to point TransportNetwork and start a server with API for point to point searches
  *
  */
+@Deprecated
 public class PointToPointRouterServer {
     private static final Logger LOG = LoggerFactory.getLogger(PointToPointRouterServer.class);
 

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

@@ -0,0 +1,161 @@
+package com.conveyal.r5.transit;
+
+import com.conveyal.gtfs.GTFSFeed;
+import com.conveyal.gtfs.model.Transfer;
+import gnu.trove.list.TIntList;
+import gnu.trove.list.array.TIntArrayList;
+import gnu.trove.map.TIntObjectMap;
+import gnu.trove.map.TObjectIntMap;
+import gnu.trove.map.hash.TIntObjectHashMap;
+import gnu.trove.map.hash.TObjectIntHashMap;
+
+import java.util.BitSet;
+import java.util.HashSet;
+import java.util.Set;
+
+import static com.conveyal.r5.analyst.cluster.TransportNetworkConfig.TransferConfig;
+import static com.conveyal.r5.analyst.cluster.TransportNetworkConfig.TransferConfig.*;
+import static com.google.common.base.Strings.isNullOrEmpty;
+
+/**
+ * Transfers between transit stops can come from two sources: transfers.txt in GTFS inputs and
+ * routing through the OSM-derived street network. This class handles loading from transfers.txt and
+ * retains enough information so the subsequent street routing approach can adopt the behavior
+ * specified with TransportNetworkConfig.TransferConfig. The way in which GTFS transfers take
+ * priority over transfers found through the street network can be configured.
+ * <p>
+ * There can be multiple GTFS feeds and they are loaded in a streaming fashion, with only one open
+ * and consuming memory at a time. Therefore we lose a lot of context by the time the street routing
+ * happens.
+ * <p>
+ * Use a single instance across all GTFS feeds. Call the load method once on each feed in turn. This
+ * accumulates information from all feeds that will later be important for finding transfers through
+ * the OSM street network.
+ * <p>
+ * The GTFS transfers we load are specified in terms of minimum time, while the street transfers are
+ * stored as distances and resolved to time during searches based on the specified walk speed. On
+ * the downside this requires two separate data structures, but on the upside it simplifies loading
+ * because GTFS must be loaded one feed at a time, while on-street transfers must be found later
+ * after all GTFS feeds are loaded and all stops linked (because street transfers are expected to
+ * connect stops from different feeds).
+ * <p>
+ * TODO Further increase transfer time accuracy using GTFS pathways.txt
+ */
+public class GtfsTransferLoader {
+
+    final TransitLayer transit;
+    final TransferConfig transferConfig;
+
+    int nMissingStop = 0;
+    int nUnsupportedSpecificity = 0;
+    int nUnsupportedTransferType = 0;
+    TObjectIntMap<String> otherErrors = new TObjectIntHashMap<>();
+
+    public record StopPair(int fromStop, int toStop) { }
+
+    final BitSet stopsWithTransfers = new BitSet();
+    final Set<StopPair> stopPairsWithTransfers = new HashSet<>();
+    // Transfer keys are from-stop indexes, values are packed lists of (to-stop, distance) pairs.
+    final TIntObjectMap<TIntList> transfers = new TIntObjectHashMap<>();
+
+    public GtfsTransferLoader (TransitLayer transit, TransferConfig transferConfig) {
+        this.transit = transit;
+        this.transferConfig = transferConfig;
+    }
+
+    public void loadTransfersTxt (GTFSFeed feed) {
+        if (transferConfig == OSM_ONLY) return;
+        if (feed.transfers == null || feed.transfers.isEmpty()) return;
+        // The keys of GtfsFeed.transfers are just arbitrary unique numbers (the input line numbers).
+        for (Transfer transfer : feed.transfers.values()) {
+            if (shouldSkipTransfer(transfer)) continue;
+            int from = transit.indexForStopId.get(transfer.from_stop_id);
+            int to = transit.indexForStopId.get(transfer.to_stop_id);
+            if (untrue(from < 0 || to < 0, "Transfer references stop that was not loaded.")) continue;
+            if (untrue(transfer.min_transfer_time < 0, "Negative transfer times not allowed.")) continue;
+            if (untrue(transfer.min_transfer_time > 3600, "Transfer time suspiciously high.")) continue;
+            TIntList packedTransfers = transfers.get(from);
+            if (packedTransfers == null) {
+                packedTransfers = new TIntArrayList();
+                transfers.put(from, packedTransfers);
+            }
+            packedTransfers.add(to);
+            packedTransfers.add(transfer.min_transfer_time);
+            stopsWithTransfers.set(from);
+            stopsWithTransfers.set(to);
+            // Conditional to avoid excessive object instance bloat when unused.
+            if (transferConfig == PER_STOP_PAIR) {
+                stopPairsWithTransfers.add(new StopPair(from, to));
+            }
+        }
+        if (transferConfig == PER_FEED) {
+            throw new UnsupportedOperationException();
+            // Prevent later on-street transfer calculation for every stop in this feed.
+            // However, this behavior probably needs to be different for inter- and intra-feed transfers.
+            // And it may need to be manually set independently for each individual feed.
+            // for (String stopId : feed.stops.keySet()) {
+            //     int stopIndex = transit.indexForStopId.get(stopId);
+            //     if (stopIndex > 0) stopsWithTransfers.set(stopIndex);
+            // }
+        }
+    }
+
+    // TODO encapsulate and reuse elsewhere.
+    private boolean untrue (boolean condition, String errorMessage) {
+        if (condition) otherErrors.adjustOrPutValue(errorMessage, 1, 1);
+        return !condition;
+    }
+
+    /**
+     * Validate one transfer and decide whether it should be processed by this class, maintaining
+     * some counts and flags for debugging and status reporting.
+     */
+    private boolean shouldSkipTransfer (Transfer transfer) {
+        boolean skip = false;
+        if (!(isNullOrEmpty(transfer.from_route_id) && isNullOrEmpty(transfer.from_trip_id) &&
+              isNullOrEmpty(transfer.to_route_id) && isNullOrEmpty(transfer.to_trip_id))) {
+            nUnsupportedSpecificity += 1;
+            skip = true;
+        }
+        if (isNullOrEmpty(transfer.from_stop_id) || isNullOrEmpty(transfer.to_stop_id)) {
+            nMissingStop += 1;
+            skip = true;
+        }
+        if (transfer.transfer_type < 2) {
+            nUnsupportedTransferType += 1;
+            skip = true;
+        }
+        if (transfer.transfer_type > 3) {
+            // In-seat transfer information may be "supported", but not consumed by this class.
+            skip = true;
+        }
+        return skip;
+    }
+
+    /**
+     * This is an optimization to avoid a slow street search when every transfer it yields will be ignored (because
+     * every stop pair involving this source stop is slated to be skipped).
+     * @return whether the osm street transfer generation should skip producing any transfers from the given stop.
+     */
+    public boolean shouldSkipFromStop (int fromStopIndex) {
+        if (transferConfig == PER_STOP) {
+            return stopsWithTransfers.get(fromStopIndex);
+        }
+        return false;
+    }
+
+    /**
+     * A GTFS transfer between a specific pair of stops or involving particular stops may take priority over any
+     * transfer found by routing through the OSM street network.
+     * @return whether osm street transfer generation should skip making transfers between the given pair of stops.
+     */
+    public boolean shouldSkipStopPair (int fromStopIndex, int toStopIndex) {
+        if (transferConfig == PER_STOP_PAIR) {
+            return stopPairsWithTransfers.contains(new StopPair(fromStopIndex, toStopIndex));
+        } else if (transferConfig == PER_STOP) {
+            return stopsWithTransfers.get(fromStopIndex) || stopsWithTransfers.get(toStopIndex);
+        }
+        return false;
+    }
+
+}

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

@@ -16,36 +16,37 @@
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
+import static com.conveyal.r5.analyst.cluster.TransportNetworkConfig.*;
 import static com.conveyal.r5.streets.StreetRouter.State.RoutingVariable;
 import static com.conveyal.r5.transit.TransitLayer.PARKRIDE_DISTANCE_LIMIT_METERS;
 import static com.conveyal.r5.transit.TransitLayer.TRANSFER_DISTANCE_LIMIT_METERS;
 
 /**
  * Pre-compute walking transfers between transit stops via the street network, up to a given distance limit.
  * TODO optimization: combine TransferFinder with stop-to-vertex distance table builder.
+ * TODO rename to OsmStreetTransferFinder by contrast with GtfsTransferLoader
  */
 public class TransferFinder {
 
     private static final Logger LOG = LoggerFactory.getLogger(TransferFinder.class);
 
     // Optimization: use the same empty list for all stops with no transfers
     private static final TIntArrayList EMPTY_INT_LIST = new TIntArrayList();
-
     // Optimization: use the same empty list for all stops with no transfers
     private static final TIntObjectMap<StreetRouter.State> EMPTY_STATE_MAP = new TIntObjectHashMap<>();
-
     TransitLayer transitLayer;
-
     StreetLayer streetLayer;
+    GtfsTransferLoader gtfsTransferLoader;
 
     /**
      * Eventually this should choose whether to search via the street network or straight line distance based on the
      * presence of OSM street data (whether the street layer is null). However the street layer will always be present,
      * at least to contain transit stop vertices, so the choice cannot be made based only on the absence of a streetLayer.
      */
-    public TransferFinder(TransportNetwork network) {
+    public TransferFinder(TransportNetwork network, GtfsTransferLoader gtfsTransferLoader) {
         this.transitLayer = network.transitLayer;
         this.streetLayer = network.streetLayer;
+        this.gtfsTransferLoader = gtfsTransferLoader;
     }
 
     public void findParkRideTransfer() {
@@ -100,22 +101,34 @@ public void findParkRideTransfer() {
      * However, existing transfer lists will be extended if new stops are reachable from existing stops.
      */
     public void findTransfers () {
+        if (gtfsTransferLoader.transferConfig == TransferConfig.GTFS_ONLY) {
+            LOG.info("Not finding transfers through street network due to GTFS_ONLY in TransportNetworkConfig.");
+            return;
+        }
         // Look at the existing list of transfers (if any) and do enough iterations to make that transfer list as long
         // as the list of stops.
         int firstStopToProcess = transitLayer.transfersForStop.size();
         int nStopsTotal = transitLayer.getStopCount();
-        int nStopsToProcess =  nStopsTotal - firstStopToProcess;
-        LOG.info("Finding transfers through the street network from {} stops...", nStopsToProcess);
+        int nStopsToProcess = nStopsTotal - firstStopToProcess;
+        LOG.info("Finding transfers through the street network from {} new transit stops...", nStopsToProcess);
         LambdaCounter stopCounter = new LambdaCounter(LOG, nStopsToProcess, 10_000,
-                "Found transfers from {} of {} transit stops.");
+              "Processed OSM street transfers from {} of {} new transit stops.");
         LambdaCounter unconnectedCounter = new LambdaCounter(LOG, nStopsToProcess, 1_000,
-                "{} of {} transit stops are unlinked.");
+              "{} of {} new transit stops are not linked to the street network.");
+        LambdaCounter skippedFromStopCounter = new LambdaCounter(LOG, nStopsToProcess, 1_000,
+              "Deferred to GTFS transfers for {} of {} transfer source stops.");
+        LambdaCounter skippedPairCounter = new LambdaCounter(LOG, 1_000,
+              "Deferred to GTFS for {} stop pairs.");
 
         // Create transfers for all new stops, appending them to the list of transfers for any existing stops.
         // This handles both newly built networks and the case where a scenario adds stops to an existing network.
         transitLayer.transfersForStop.addAll(
                 IntStream.range(firstStopToProcess, nStopsTotal).parallel().mapToObj(sourceStopIndex -> {
             stopCounter.increment();
+            if (gtfsTransferLoader.shouldSkipFromStop(sourceStopIndex)) {
+                skippedFromStopCounter.increment();
+                return EMPTY_INT_LIST;
+            }
             // From each stop, run a street search looking for other transit stops.
             int originStreetVertex = transitLayer.streetVertexForStop.get(sourceStopIndex);
             if (originStreetVertex == -1) {
@@ -131,29 +144,38 @@ public void findTransfers () {
             streetRouter.quantityToMinimize = RoutingVariable.DISTANCE_MILLIMETERS;
 
             streetRouter.route();
+            // A map from stop indexes to values of the routing objective variable (DISTANCE_MILLIMETERS).
             TIntIntMap distancesToReachedStops = streetRouter.getReachedStops();
             // Same-stop "transfers" are handled in the router and do not need to be materialized in our list of
             // transfer distances. It's actually important to remove the source stop to handle certain cases with
             // loop routes (see CTA Brown Line to Purple Line example in discussion on #763).
             distancesToReachedStops.remove(sourceStopIndex);
+            // TODO consider how this interferes with deferring to GTFS-specified transfers on stop pairs
             retainClosestStopsOnPatterns(distancesToReachedStops);
             // At this point we have the distances to all stops that are the closest one on some pattern.
             // Make transfers to them, packed as pairs of (target stop index, distance).
             TIntList packedTransfers = new TIntArrayList();
             distancesToReachedStops.forEachEntry((targetStopIndex, distance) -> {
-                packedTransfers.add(targetStopIndex);
-                packedTransfers.add(distance);
+                if (gtfsTransferLoader.shouldSkipStopPair(sourceStopIndex, targetStopIndex)) {
+                    skippedPairCounter.increment();
+                } else {
+                    packedTransfers.add(targetStopIndex);
+                    packedTransfers.add(distance);
+                }
                 return true;
             });
             // Record this list of transfers as leading out of the stop with index sourceStopIndex.
-            // Deduplicate empty lists.
             if (packedTransfers.size() > 0) {
                 return packedTransfers;
             } else {
                 return EMPTY_INT_LIST;
             }
         }).collect(Collectors.toList()));
-        LOG.info("Done finding transfers. {} stops were not linked to the street network.", unconnectedCounter.getCount());
+
+        stopCounter.done();
+        unconnectedCounter.logIfNonZero();
+        skippedFromStopCounter.logIfNonZero();
+        skippedPairCounter.logIfNonZero();
 
         // If we are applying a scenario (extending the transfers list rather than starting from scratch), for
         // all transfers out of a scenario stop into a base network stop we must also create the reverse transfer.

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

@@ -112,7 +112,7 @@ public class TransitLayer implements Serializable, Cloneable {
     // Inverse map of streetVertexForStop, and reconstructed from that list.
     public transient TIntIntMap stopForStreetVertex;
 
-    // For each stop, a packed list of transfers to other stops
+    // For each stop, a packed list of transfers to other stops in the form (stopIndex, distance, stopIndex, distance...)
     // FIXME we may currently be storing weight or time to reach other stop, which we did to avoid floating point division. Instead, store distances in millimeters, and divide by speed in mm/sec.
     public List<TIntList> transfersForStop = new ArrayList<>();