default to OSM_ONLY, fix GTFS_ONLY implementation

clarified some docs, set network version to nv4
as a precaution, restore original StreetLayer field initialization order

Author: abyrd

src/main/java/com/conveyal/r5/analyst/scenario/Scenario.java

@@ -159,7 +159,7 @@ public TransportNetwork applyToTransportNetwork (TransportNetwork originalNetwor
 
         // Find the transfers originating at or terminating at new stops.
         // TODO also rebuild transfers which are near street network changes but which do not connect to new stops.
-        // The original GtfsTransferLoader is not retained after network build time. Here, for making transfers
+        // The GtfsTransferLoader instance is not retained after network build time. Here, for making transfers
         // necessitated by scenario changes, we use a no-op instance that completely ignores anything from GTFS.
         var osmOnlyLoader = new GtfsTransferLoader(copiedNetwork.transitLayer, OSM_ONLY);
         var transferFinder = new TransferFinder(copiedNetwork, osmOnlyLoader);

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

@@ -54,7 +54,7 @@ public abstract class KryoNetworkSerializer {
      * 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 = "nv4-testing"; // TODO change to nv4 for release
+    public static final String NETWORK_FORMAT_VERSION = "nv4"; // TODO change to nv4 for release
 
     public static final byte[] HEADER = "R5NETWORK".getBytes();
 

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

@@ -21,30 +21,28 @@
 import static com.conveyal.r5.analyst.cluster.TransportNetworkConfig.TransferConfig.STOP_TO_PATTERN;
 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
- */
+/// 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.
+///
+/// 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.
+///
+/// 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.
+///
+/// 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).
+///
+/// TODO Further increase transfer time accuracy using GTFS pathways.txt
 public class GtfsTransferLoader {
 
     private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@@ -78,7 +76,7 @@ public record StopPair(int fromStop, int toStop) { }
 
     public GtfsTransferLoader (TransitLayer transit, TransferConfig transferConfig) {
         this.transit = transit;
-        this.transferConfig = transferConfig != null ? transferConfig : STOP_TO_PATTERN;
+        this.transferConfig = transferConfig != null ? transferConfig : OSM_ONLY;
     }
 
     ///  @param indexForUnscopedStopId Map of stop IDs not yet scoped by feed ID, which exists only during GTFS loading

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

@@ -11,14 +11,12 @@
 import gnu.trove.map.TIntObjectMap;
 import gnu.trove.map.hash.TIntIntHashMap;
 import gnu.trove.map.hash.TIntObjectHashMap;
-import gnu.trove.set.TIntSet;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
-import static com.conveyal.r5.analyst.cluster.TransportNetworkConfig.*;
+import static com.conveyal.r5.analyst.cluster.TransportNetworkConfig.TransferConfig.GTFS_ONLY;
 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;
@@ -106,15 +104,19 @@ 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.streetTransfers.size();
         int nStopsTotal = transitLayer.getStopCount();
         int nStopsToProcess = nStopsTotal - firstStopToProcess;
+        if (gtfsTransferLoader.transferConfig == GTFS_ONLY) {
+            LOG.info("Not finding transfers through street network due to GTFS_ONLY in TransportNetworkConfig.");
+            // Unlike the GTFS transfers which are in a map (because they may be very sparse),
+            // street transfers are in a list because they are expected to exist at most stops.
+            // We must extend that list to avoid out of bounds index exceptions.
+            for (int i = 0; i < nStopsToProcess; i++) transitLayer.streetTransfers.add(null);
+            return; // Do not add any street transfers, leave them null for all new stops.
+        }
         LOG.info("Finding transfers through the street network from {} new transit stops...", nStopsToProcess);
         LambdaCounter stopCounter = new LambdaCounter(LOG, nStopsToProcess, 10_000,
               "Processed OSM street transfers from {} of {} new transit stops.");
@@ -229,10 +231,10 @@ public void findTransfers () {
      * street network. TODO we could check stop positions in the target pattern to help handle a case like this -- for
      * any continuous sequence of stops, retain only the closest.
      *
-     * @param timesToReachedStops map from target stop index to distance (along the street network) to it. The method
- *                           mutates this parameter.
-     * @param ignorePatterns used to filter out stop-pattern pairs that already have a transfer from GTFS. May be null
-     *                       or empty in other situations.
+     * @param timesToReachedStops map from target stop index to distance (along the street network) to it.
+     *                       The method mutates this parameter.
+     * @param ignorePatterns used to filter out stop-pattern pairs that already have a transfer from GTFS.
+     *                       May be null or empty in other situations.
      */
     private void retainClosestStopsOnPatterns(TIntIntMap timesToReachedStops, TIntCollection ignorePatterns) {
         TIntIntMap bestStopOnPattern = new TIntIntHashMap(50, 0.5f, -1, -1);

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

@@ -247,8 +247,8 @@ private TransportNetwork buildNetworkFromConfig (TransportNetworkConfig config)
         //  gtfs1, gtfs2...) (see various methods in TransportNetwork).
         TransportNetwork network = new TransportNetwork();
         network.streetLayer = new StreetLayer(config);
-        network.streetLayer.parentNetwork = network;
         network.streetLayer.loadFromOsm(osmCache.get(config.osmId));
+        network.streetLayer.parentNetwork = network;
         network.streetLayer.indexStreets();
 
         // The GTFS transfer loader persists across all loaded feeds so we can feed information about all the transfers