use maxWalkDistance in search, #2140.

src/main/java/org/opentripplanner/profile/ProfileRequest.java

@@ -42,7 +42,23 @@ public class ProfileRequest implements Serializable, Cloneable {
     /** Maximum time to reach the destination without using transit */
     public int    streetTime;
 
-    /** Maximum walk time before and after using transit */
+    /**
+     * Maximum walk time before and after using transit, in minutes
+     *
+     * NB the time to reach the destination after leaving transit is considered separately from the time to reach
+     * transit at the start of the search; e.g. if you set maxWalkTime to 600 (ten minutes), you could potentially walk
+     * up to ten minutes to reach transit, and up to _another_ ten minutes to reach the destination after leaving transit.
+     *
+     * This is required because hard resource limiting on non-objective variables is algorithmically invalid. Consider
+     * a case where there is a destination 10 minutes from transit and an origin 5 minutes walk from a feeder bus and
+     * 15 minutes walk from a fast train, and the walk budget is 20 minutes. If an intermediate point in the search
+     * (for example, a transfer station) is reached by the fast train before it is reached by the bus, the route using
+     * the bus will be dominated. When we leave transit, though, we have already used up 15 minutes of our walk budget
+     * and don't have enough remaining to reach the destination.
+     *
+     * This is solved by using separate walk budgets at the origin and destination. It could also be solved (although this
+     * would slow the algorithm down) by retaining all Pareto-optimal combinations of (travel time, walk distance).
+     */
     public int    maxWalkTime;
 
     /** Maximum bike time when using transit */

src/main/java/org/opentripplanner/profile/RaptorWorkerData.java

@@ -441,39 +441,30 @@ public RaptorWorkerData (Graph graph, TimeWindow window, ProfileRequest req, Sam
         // Record times to nearby intersections for all used stops.
         // We use times rather than distances to avoid a costly floating-point divide during propagation
         if (sampleSet == null) {
+            int maxWalkDistance = (int) (req.maxWalkTime * 60 * req.walkSpeed);
             for (TIntIterator stopIt = stopForIndex.iterator(); stopIt.hasNext();) {
                 int stop = stopIt.next();
 
                 // permanent stop
                 Vertex tstop = graph.getVertexById(stop);
-                if (tstop != null && TransitStop.class.isInstance(tstop)) {
-                    // permanent stop
-                    // convert distance to time
-                    int[] distancesForStop = stc.distancesForStop.get(tstop);
-                    int[] timesForStop = new int[distancesForStop.length];
-
-                    for (int i = 0; i < distancesForStop.length; i++) {
-                        timesForStop[i] = distancesForStop[i];
-                        i++; // advance to distance for stop i
+                boolean isPermanentStop = tstop != null && TransitStop.class.isInstance(tstop);
+                // convert distance to time
+                int[] distancesForStop = isPermanentStop ? stc.distancesForStop.get(tstop) : temporaryStopTreeCache.get(stop);
+                TIntList timesForStop = new TIntArrayList();
+
+                for (int i = 0; i < distancesForStop.length; i += 2) {
+                    int vidx = distancesForStop[i];
+                    int dist = distancesForStop[i + 1];
+
+                    // only add if it's less than the max walk distance
+                    if (dist <= maxWalkDistance) {
+                        timesForStop.add(vidx);
                         // convert meters to seconds by dividing by meters / second
-                        timesForStop[i] = (int) (distancesForStop[i] / req.walkSpeed);
+                        timesForStop.add((int) (dist / req.walkSpeed));
                     }
-
-                    targetsForStop.add(timesForStop);
                 }
-                else {
-                    // temporary stop
-                    int[] distancesForStop = temporaryStopTreeCache.get(stop);
-                    int[] timesForStop = new int[distancesForStop.length];
-
-                    for (int i = 0; i < distancesForStop.length; i++) {
-                        timesForStop[i] = distancesForStop[i];
-                        i++; // advance to distance for stop i
-                        // convert meters to seconds by dividing by meters / second
-                        timesForStop[i] = (int) (distancesForStop[i] / req.walkSpeed);
-                    }
 
-                    targetsForStop.add(timesForStop);                }
+                targetsForStop.add(timesForStop.toArray());
             }
 
             // TODO memory leak when many graphs have been built

src/main/java/org/opentripplanner/profile/RepeatedRaptorProfileRouter.java

@@ -19,6 +19,7 @@
 import org.opentripplanner.routing.core.State;
 import org.opentripplanner.routing.core.TraverseMode;
 import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.routing.graph.GraphIndex;
 import org.opentripplanner.routing.graph.Vertex;
 import org.opentripplanner.routing.pathparser.InitialStopSearchPathParser;
 import org.opentripplanner.routing.pathparser.PathParser;
@@ -255,7 +256,10 @@ private TIntIntMap findInitialStops (boolean dest, RaptorWorkerData data) {
                 // suffer from roundoff. Great care is taken when splitting to preserve sums.
                 // When cycling, this is not an issue; we already have an explicitly asymmetrical search (cycling at the origin, walking at the destination),
                 // so we need not preserve symmetry.
-                rr.maxWalkDistance = 2000; // FIXME kind of arbitrary
+                // We use the max walk time for the search at the origin, but we clamp it to MAX_WALK_METERS so that we don;t
+                // have every transit stop in the state as an initial transit stop if someone sets max walk time to four days,
+                // and so that symmetry is preserved.
+                rr.maxWalkDistance = Math.min(request.maxWalkTime * 60 * request.walkSpeed, GraphIndex.MAX_WALK_METERS); // FIXME kind of arbitrary
                 rr.softWalkLimiting = false;
                 rr.dominanceFunction = new DominanceFunction.LeastWalk();
             }

src/main/java/org/opentripplanner/routing/graph/GraphIndex.java

@@ -55,6 +55,9 @@ public class GraphIndex {
     private static final Logger LOG = LoggerFactory.getLogger(GraphIndex.class);
     private static final int CLUSTER_RADIUS = 400; // meters
 
+    /** maximum distance to walk after leaving transit in Analyst */
+    public static final int MAX_WALK_METERS = 2000;
+
     // TODO: consistently key on model object or id string
     public final Map<String, Vertex> vertexForId = Maps.newHashMap();
     public final Map<String, Agency> agencyForId = Maps.newHashMap();
@@ -441,7 +444,7 @@ public StopTreeCache getStopTreeCache() {
         if (stopTreeCache == null) {
             synchronized (this) {
                 if (stopTreeCache == null) {
-                    stopTreeCache = new StopTreeCache(graph, 2000); // TODO make this max-distance variable
+                    stopTreeCache = new StopTreeCache(graph, MAX_WALK_METERS); // TODO make this max-distance variable
                 }
             }
         }