Configurable linking distance and subgraph size

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

@@ -62,4 +62,18 @@ public class TransportNetworkConfig {
      */
     public boolean saveShapes;
 
+    /**
+     * Specifies the distance to link to stops to streets, null to use default.
+     */
+    public Double stopLinkRadiusMeters = null;
+
+    /**
+     * Specifies the distance to link to origins and destinations to streets, null to use default.
+     */
+    public Double pointsetLinkRadiusMeters = null;
+
+    /**
+     * Minimum subgraph (island) size to retain when building networks. 
+     */
+    public Integer minSubgraphSize = null;
 }

src/main/java/com/conveyal/r5/streets/LinkedPointSet.java

@@ -323,7 +323,7 @@ private void linkPointsToStreets (boolean all) {
                 double pointLatFixed = floatingDegreesToFixed(pointSet.getLat(p));
                 double pointLonFixed = floatingDegreesToFixed(pointSet.getLon(p));
                 Envelope pointEnvelopeFixed = new Envelope(pointLonFixed, pointLonFixed, pointLatFixed, pointLatFixed);
-                double radiusMeters = StreetLayer.LINK_RADIUS_METERS;
+                double radiusMeters = streetLayer.pointsetLinkRadiusMeters;
                 if (edges[p] != -1) {
                     radiusMeters = this.distancesToEdge_mm[p] / 1000.0;
                 }
@@ -335,7 +335,7 @@ private void linkPointsToStreets (boolean all) {
             if (relinkThisPoint) {
                 // Use radius from StreetLayer such that maximum origin and destination walk distances are symmetric.
                 Split split = streetLayer.findSplit(pointSet.getLat(p), pointSet.getLon(p),
-                        StreetLayer.LINK_RADIUS_METERS, streetMode);
+                        streetLayer.pointsetLinkRadiusMeters, streetMode);
                 if (split == null) {
                     edges[p] = -1;
                 } else {

src/main/java/com/conveyal/r5/streets/StreetLayer.java

@@ -80,6 +80,13 @@ public class StreetLayer implements Serializable, Cloneable {
 
     private static final Logger LOG = LoggerFactory.getLogger(StreetLayer.class);
 
+    /**
+     * The radius below which we will not split a street, and will instead connect to an existing intersection.
+     * i.e. if the requested split point is less than this distance from an existing vertex (edge endpoint) we'll just
+     * return that existing endpoint.
+     */
+    private static final int SNAP_RADIUS_MM = 5 * 1000;
+
     /**
      * Minimum allowable size (in number of vertices) for a disconnected subgraph; subgraphs smaller than these will be removed.
      * There are several reasons why one might have a disconnected subgraph. The most common is poor quality
@@ -88,23 +95,21 @@ public class StreetLayer implements Serializable, Cloneable {
      * to mind), or islands that are isolated by infrastructure (for example, airport terminals reachable
      * only by transit or driving, for instance BWI or SFO).
      */
-    public static final int MIN_SUBGRAPH_SIZE = 40;
+    public int minSubgraphSize = 40;
 
     /**
-     * The radius below which we will not split a street, and will instead connect to an existing intersection.
-     * i.e. if the requested split point is less than this distance from an existing vertex (edge endpoint) we'll just
-     * return that existing endpoint.
+     * The radius of a circle in meters within which to search for nearby streets when linking stops/park and rides.
+     * 
+     * The default, 1.6km, is really far to walk off a street. But some places have offices in the middle of big parking lots.
      */
-    private static final int SNAP_RADIUS_MM = 5 * 1000;
+    public double stopLinkRadiusMeters = 1600;
 
     /**
-     * The radius of a circle in meters within which to search for nearby streets.
-     * This should not necessarily be a constant, but even if it's made settable it should be stored in a field on this
-     * class to avoid cluttering method signatures. Generally you'd set this once at startup and always use the same
-     * value afterward.
-     * 1.6km is really far to walk off a street. But some places have offices in the middle of big parking lots.
+     * The radius of a circle in meters within which to search for nearby streets when linking pointsets/origins/destinations.
+     * 
+     * The default, 1.6km is really far to walk off a street. But some places have offices in the middle of big parking lots.
      */
-    public static final double LINK_RADIUS_METERS = 1600;
+    public double pointsetLinkRadiusMeters = 1600;
 
     /**
      * Searching for streets takes a fair amount of computation, and the number of streets examined grows roughly as
@@ -222,8 +227,14 @@ public StreetLayer(TransportNetworkConfig config) {
                         "Unknown traversal permission labeler: " + config.traversalPermissionLabeler
                 );
             };
+
+            if (config.pointsetLinkRadiusMeters != null) this.pointsetLinkRadiusMeters = config.pointsetLinkRadiusMeters;
+            if (config.stopLinkRadiusMeters != null) this.stopLinkRadiusMeters = config.stopLinkRadiusMeters;
+            if (config.minSubgraphSize != null) this.minSubgraphSize = config.minSubgraphSize;
         } else {
             permissionLabeler = new USTraversalPermissionLabeler();
+
+            // leave link radii at defaults
         }
     }
 
@@ -383,10 +394,10 @@ void loadFromOsm (OSM osm, boolean removeIslands, boolean saveVertexIndex) {
         buildEdgeLists();
         stressLabeler.applyIntersectionCosts(this);
         if (removeIslands) {
-            new TarjanIslandPruner(this, MIN_SUBGRAPH_SIZE, StreetMode.CAR).run();
+            new TarjanIslandPruner(this, minSubgraphSize, StreetMode.CAR).run();
             // due to bike walking, walk must go before bike, see comment in TarjanIslandPruner javadoc
-            new TarjanIslandPruner(this, MIN_SUBGRAPH_SIZE, StreetMode.WALK).run();
-            new TarjanIslandPruner(this, MIN_SUBGRAPH_SIZE, StreetMode.BICYCLE).run();
+            new TarjanIslandPruner(this, minSubgraphSize, StreetMode.WALK).run();
+            new TarjanIslandPruner(this, minSubgraphSize, StreetMode.BICYCLE).run();
         }
 
         // index the streets, we need the index to connect things to them.
@@ -1290,6 +1301,9 @@ public void buildEdgeLists() {
      *
      * This uses {@link #findSplit(double, double, double, StreetMode)} and {@link Split} which require the spatial
      * index to already be built. In other works {@link #indexStreets()} needs to be called before this is used.
+     * 
+     * This method is used to link stops and other similar things (both in base networks and
+     * modifications), so it uses stopLinkRadiusMeters rather than pointsetLinkRadiusMeters.
      *
      * TODO potential refactor: rename this method Split.perform(), and store a ref to streetLayer in Split.
      * @param lat latitude in floating point geographic (not fixed point) degrees.
@@ -1300,7 +1314,7 @@ public void buildEdgeLists() {
      */
     public int getOrCreateVertexNear(double lat, double lon, StreetMode streetMode) {
 
-        Split split = findSplit(lat, lon, LINK_RADIUS_METERS, streetMode);
+        Split split = findSplit(lat, lon, stopLinkRadiusMeters, streetMode);
         if (split == null) {
             // No linking site was found within range.
             return -1;
@@ -1501,6 +1515,7 @@ public Split findSplit(double lat, double lon, double radiusMeters, StreetMode s
      * between the two.
      */
     public void associateStops (TransitLayer transitLayer) {
+        LOG.info("Linking stops to streets within {} meters", stopLinkRadiusMeters);
         for (Stop stop : transitLayer.stopForIndex) {
             int stopVertex = createAndLinkVertex(stop.stop_lat, stop.stop_lon);
             transitLayer.streetVertexForStop.add(stopVertex); // This is always a valid, unique vertex index.

src/main/java/com/conveyal/r5/streets/StreetRouter.java

@@ -323,7 +323,7 @@ public StreetRouter (StreetLayer streetLayer) {
      * @return true if an edge was found near the specified coordinate
      */
     public boolean setOrigin (double lat, double lon) {
-        Split split = streetLayer.findSplit(lat, lon, StreetLayer.LINK_RADIUS_METERS, streetMode);
+        Split split = streetLayer.findSplit(lat, lon, streetLayer.pointsetLinkRadiusMeters, streetMode);
         if (split == null) {
             LOG.info("No street was found near the specified origin point of {}, {}.", lat, lon);
             return false;
@@ -433,7 +433,7 @@ public void setOrigin(TIntObjectMap<State> previousStates, int switchTime, int s
      * @return true if edge was found near wanted coordinate
      */
     public boolean setDestination (double lat, double lon) {
-        this.destinationSplit = streetLayer.findSplit(lat, lon, StreetLayer.LINK_RADIUS_METERS, streetMode);
+        this.destinationSplit = streetLayer.findSplit(lat, lon, streetLayer.pointsetLinkRadiusMeters, streetMode);
         return this.destinationSplit != null;
     }
 
@@ -830,7 +830,7 @@ public Split getDestinationSplit() {
      * @return
      */
     public State getState(double lat, double lon) {
-        Split split = streetLayer.findSplit(lat, lon, StreetLayer.LINK_RADIUS_METERS, streetMode);
+        Split split = streetLayer.findSplit(lat, lon, streetLayer.pointsetLinkRadiusMeters, streetMode);
         if (split == null) {
             LOG.info("No street was found near the specified origin point of {}, {}.", lat, lon);
             return null;

src/main/java/com/conveyal/r5/streets/TarjanIslandPruner.java

@@ -279,6 +279,11 @@ public void forEachOutgoingEdge (int vertex, Consumer<EdgeStore.Edge> consumer)
             edgeCursor.seek(eidx);
 
             // filter by mode
+            // NB by not including e.g. NO_THRU_TRAFFIC_PEDESTRIAN here, NO_THRU_TRAFFIC edges
+            // will create separate subgraphs. Prima facie, this seems pretty sensible, but it does
+            // mean that e.g. driveways marked access=private (and thus no thru traffic) will get removed
+            // by the island removal process, because the node at the end of the driveway will be considered
+            // an island of size one.
             switch (mode) {
                 case WALK:
                     if (!edgeCursor.getFlag(EdgeStore.EdgeFlag.ALLOWS_PEDESTRIAN)) return true;

src/test/java/com/conveyal/r5/streets/StreetLayerTest.java

@@ -1,7 +1,11 @@
 package com.conveyal.r5.streets;
 
 import com.conveyal.osmlib.OSM;
+import com.conveyal.r5.analyst.FreeFormPointSet;
+import com.conveyal.r5.analyst.cluster.TransportNetworkConfig;
 import com.conveyal.r5.profile.StreetMode;
+import com.conveyal.r5.streets.EdgeStore.Edge;
+import com.conveyal.r5.streets.EdgeStore.EdgeFlag;
 import com.conveyal.r5.streets.VertexStore.VertexFlag;
 import gnu.trove.TIntCollection;
 import gnu.trove.iterator.TIntIterator;
@@ -16,6 +20,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -420,4 +425,69 @@ public void testMissingNodes () {
         });
     }
 
+    /**
+     * Make sure that the stop linking and split distance calculations are affected by their configuration options.
+     */
+    @Test
+    public void testConfigurableSplitDistancesAndIslandRemoval() {
+        int[] exponents = {0, 1, 2};
+        // to test different options, do this three times with values doubling each time
+        for (int exponent : exponents) {
+            int multiplier = (int) Math.pow(2, exponent);
+
+            OSM osm = new OSM(null);
+            osm.intersectionDetection = true;
+            // re-use one of our existing resources, rather than bloating the repo with a new one just for this test
+            osm.readFromUrl(StreetLayerTest.class.getResource("reisterstown-via-restriction.pbf").toString());
+
+            TransportNetworkConfig cfg = new TransportNetworkConfig();
+            cfg.stopLinkRadiusMeters = 50d * multiplier;
+            cfg.pointsetLinkRadiusMeters = 100d * multiplier;
+            cfg.minSubgraphSize = 5 * multiplier;
+
+            StreetLayer sl = new StreetLayer(cfg);
+            sl.loadFromOsm(osm, true, true);
+            sl.buildEdgeLists();
+            sl.indexStreets();
+
+            assertEquals(sl.stopLinkRadiusMeters, 50d * multiplier);
+            assertEquals(sl.pointsetLinkRadiusMeters, 100d * multiplier);
+            assertEquals(sl.minSubgraphSize, 5 * multiplier);
+
+            // This is an island with size 6: https://www.openstreetmap.org/way/140476882#map=19/39.409392/-76.642615&layers=D
+            // Because it is at the edge of the PBF, West Joppa Road is not in the PBF, so Riderwood Stn and
+            // connected driveways form an island. Because the driveways are marked access=private and thus No Thru Traffic,
+            // they are not considered part of the island, and the nodes at the ends of them are separate islands, so the
+            // only nodes are the end of the street, where it intersects itself, and where it intersects each driveway.
+            // there should still be permissions around it
+            int eid = sl.edgeStore.osmids.indexOf(140476882L);
+            // seek to the forward edge (edge pair * 2)
+            Edge e = sl.edgeStore.getCursor(eid * 2);
+            // this island should be removed if exponent equals 1 or 2 (min subgraph size equals 10 or 20)
+            // island removal is done by removing permissions
+            assertEquals(e.allowsStreetMode(StreetMode.WALK), exponent == 0);
+            // make sure we are looking at the correct edge
+            assertEquals(e.getOSMID(), 140476882L);
+
+            // this location is about 150 m from road
+            double lat = 39.408907;
+            double lon = -76.736807;
+
+            // getOrCreateVertexNear is used to link stops, so when exponent is 0 or 1 (stop distance is 50 or 100),
+            // should not link
+            if (exponent == 0 || exponent == 1) assertEquals(sl.getOrCreateVertexNear(lat, lon, StreetMode.WALK), -1);
+            else assertNotEquals(sl.getOrCreateVertexNear(lat, lon, StreetMode.WALK), -1);
+
+            FreeFormPointSet ps = new FreeFormPointSet(new Coordinate(lon, lat));
+            LinkedPointSet lps = new LinkedPointSet(ps, sl, StreetMode.WALK, null);
+            // linked point sets should only be unlinked when exponent is 0 (pointset distance is 100)
+            if (exponent == 0) assertEquals(lps.edges[0], -1);
+            else assertNotEquals(lps.edges[0], -1);
+
+            // street router linking should behave same way
+            StreetRouter r = new StreetRouter(sl);
+            assertEquals(r.setOrigin(lat, lon), exponent != 0);
+            assertEquals(r.setDestination(lat, lon), exponent != 0);
+        }
+    }
 }