Merge pull request #972 from conveyal/car-elevation-fix

Ignore custom cost fields in CAR routing

Author: abyrd

src/main/java/com/conveyal/r5/rastercost/CostField.java

@@ -13,7 +13,16 @@
 import java.util.List;
 
 /**
- * Subclasses or plugin functions will provide costs for hills, sun, noise, pollution etc.
+ * This models a field of traversal costs, in the sense of a cost that varies over geographic space having different
+ * effects on different edges. Subclasses serve as plugins that provide costs for hills, sun, noise, pollution etc.
+ *
+ * CostFields could in principle apply costs to any mode of travel, and might yield different costs for different modes,
+ * but currently they are not mode-aware. No existing implementations apply to cars, so for now CostFields are simply
+ * ignored for car routing. Ideally CostFields would know which modes they applied to, and the list of CostFields would
+ * be filtered before the street search happens. The logic currently in the StreetRouter constructor could be factored
+ * into a mode-aware factory method that would yield the correct TraversalTimeCalculator. StreetRouter is explicitly
+ * single-use with a single StreetMode. That mode could be specified up front and the TraversalTimeCalculator 
+ * chosen accordingly (avoiding the MultistageTraversalTimeCalculator construct entirely when no CostFields apply).
  *
  * Interface-wise, a CostField is like a TraversalTimeCalculator with no turn costs, which transforms an existing
  * traversal time rather than starting from scratch. So TraversalTimeCalculator is like a special case of CostField

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

@@ -40,12 +40,15 @@ public MultistageTraversalTimeCalculator (TraversalTimeCalculator base, List<Cos
     public int traversalTimeSeconds (EdgeStore.Edge currentEdge, StreetMode streetMode, ProfileRequest req) {
         final int baseTraversalTimeSeconds = base.traversalTimeSeconds(currentEdge, streetMode, req);
         int t = baseTraversalTimeSeconds;
-        for (CostField costField : costFields) {
-            t += costField.additionalTraversalTimeSeconds(currentEdge, baseTraversalTimeSeconds);
-        }
-        if (t < 1) {
-            LOG.warn("Cost was negative or zero. Clamping to 1 second.");
-            t = 1;
+        // Currently no CostField implementations apply to CAR. See Javadoc on CostField if costs need to vary by mode.
+        if (!streetMode.equals(StreetMode.CAR)) {
+            for (CostField costField : costFields) {
+                t += costField.additionalTraversalTimeSeconds(currentEdge, baseTraversalTimeSeconds);
+            }
+            if (t < 1) {
+                LOG.warn("Cost was negative or zero. Clamping to 1 second.");
+                t = 1;
+            }
         }
         return t;
     }