Removes TraversalMode.EDGE_BASED_2DIR_UTURN (#1640)

U-turn prevention is now done entirely via TurnWeighting#calcWeight().

core/files/changelog.txt

@@ -9,7 +9,8 @@
     removed MappedDecimalEncodedValue: should be replaced with FactoredDecimalEncodedValue
     DataFlagEncoder: it is now required to add EncodedValues before. graph.encoded_values: max_speed,road_class,road_environment,road_access
     instead of EncodingManager.ROUNDABOUT use Roundabout.KEY
-    removed TraversalMode.EDGE_BASED_1DIR
+    removed TraversalMode.EDGE_BASED_1DIR and TraversalMode.EDGE_BASED_2DIR_UTURN, renamed TraversalMode.EDGE_BASED_2DIR to TraversalMode.EDGE_BASED
+    to prevent u-turns when using edge-based algorithms it is now required to use TurnWeighting, #1640
 
 0.12
     renamed VirtualEdgeIteratorState.getOriginalEdgeKey to more precise getOriginalEdgeKey #1549

core/src/main/java/com/graphhopper/GraphHopper.java

@@ -167,7 +167,7 @@ public GraphHopper setEncodingManager(EncodingManager em) {
         ensureNotLoaded();
         this.encodingManager = em;
         if (em.needsTurnCostsSupport())
-            traversalMode = TraversalMode.EDGE_BASED_2DIR;
+            traversalMode = TraversalMode.EDGE_BASED;
 
         return this;
     }
@@ -970,7 +970,7 @@ public List<Path> calcPaths(GHRequest request, GHResponse ghRsp) {
             String tModeStr = hints.get("traversal_mode", traversalMode.toString());
             TraversalMode tMode = TraversalMode.fromString(tModeStr);
             if (hints.has(Routing.EDGE_BASED))
-                tMode = hints.getBool(Routing.EDGE_BASED, false) ? TraversalMode.EDGE_BASED_2DIR : TraversalMode.NODE_BASED;
+                tMode = hints.getBool(Routing.EDGE_BASED, false) ? TraversalMode.EDGE_BASED : TraversalMode.NODE_BASED;
 
             FlagEncoder encoder = encodingManager.getEncoder(vehicle);
 

core/src/main/java/com/graphhopper/routing/AbstractBidirAlgo.java

@@ -215,11 +215,12 @@ private void fillEdges(SPTEntry currEdge, PriorityQueue<SPTEntry> prioQueue,
             if (!accept(iter, currEdge, reverse))
                 continue;
 
-            final int origEdgeId = getOrigEdgeId(iter, reverse);
-            final int traversalId = getTraversalId(iter, origEdgeId, reverse);
             final double weight = calcWeight(iter, currEdge, reverse);
-            if (Double.isInfinite(weight))
+            if (Double.isInfinite(weight)) {
                 continue;
+            }
+            final int origEdgeId = getOrigEdgeId(iter, reverse);
+            final int traversalId = getTraversalId(iter, origEdgeId, reverse);
             SPTEntry entry = bestWeightMap.get(traversalId);
             if (entry == null) {
                 entry = createEntry(iter, origEdgeId, weight, currEdge, reverse);

core/src/main/java/com/graphhopper/routing/AbstractBidirectionEdgeCHNoSOD.java

@@ -25,7 +25,6 @@
 import com.graphhopper.routing.weighting.TurnWeighting;
 import com.graphhopper.storage.Graph;
 import com.graphhopper.storage.SPTEntry;
-import com.graphhopper.storage.TurnCostExtension;
 import com.graphhopper.util.EdgeExplorer;
 import com.graphhopper.util.EdgeIterator;
 import com.graphhopper.util.EdgeIteratorState;
@@ -38,20 +37,18 @@ public abstract class AbstractBidirectionEdgeCHNoSOD extends AbstractBidirAlgo {
     private final EdgeExplorer innerInExplorer;
     private final EdgeExplorer innerOutExplorer;
     private final TurnWeighting turnWeighting;
-    private final TurnCostExtension turnCostExtension;
 
     public AbstractBidirectionEdgeCHNoSOD(Graph graph, TurnWeighting weighting) {
-        super(graph, weighting, TraversalMode.EDGE_BASED_2DIR);
+        super(graph, weighting, TraversalMode.EDGE_BASED);
         this.turnWeighting = weighting;
         // we need extra edge explorers, because they get called inside a loop that already iterates over edges
         // important: we have to use different filter ids, otherwise this will not work with QueryGraph's edge explorer
         // cache, see #1623.
         innerInExplorer = graph.createEdgeExplorer(DefaultEdgeFilter.inEdges(flagEncoder).setFilterId(1));
         innerOutExplorer = graph.createEdgeExplorer(DefaultEdgeFilter.outEdges(flagEncoder).setFilterId(1));
-        if (!(graph.getExtension() instanceof TurnCostExtension)) {
-            throw new IllegalArgumentException("edge-based CH algorithms require a turn cost extension");
+        if (!Double.isInfinite(turnWeighting.getUTurnCost())) {
+            throw new IllegalArgumentException("edge-based CH does not support finite u-turn costs at the moment");
         }
-        turnCostExtension = (TurnCostExtension) graph.getExtension();
     }
 
     @Override
@@ -111,18 +108,15 @@ protected void updateBestPath(EdgeIteratorState edgeState, SPTEntry entry, int t
         while (iter.next()) {
             final int edgeId = getOrigEdgeId(iter, !reverse);
             final int prevOrNextOrigEdgeId = getOrigEdgeId(edgeState, reverse);
-            if (!traversalMode.hasUTurnSupport() && turnCostExtension.isUTurn(edgeId, prevOrNextOrigEdgeId)) {
-                continue;
-            }
             int key = GHUtility.getEdgeKey(graph, edgeId, iter.getBaseNode(), !reverse);
             SPTEntry entryOther = bestWeightMapOther.get(key);
             if (entryOther == null) {
                 continue;
             }
 
             double turnCostsAtBridgeNode = reverse ?
-                    turnWeighting.calcTurnWeight(iter.getOrigEdgeLast(), iter.getBaseNode(), prevOrNextOrigEdgeId) :
-                    turnWeighting.calcTurnWeight(prevOrNextOrigEdgeId, iter.getBaseNode(), iter.getOrigEdgeFirst());
+                    turnWeighting.calcTurnWeight(edgeId, iter.getBaseNode(), prevOrNextOrigEdgeId) :
+                    turnWeighting.calcTurnWeight(prevOrNextOrigEdgeId, iter.getBaseNode(), edgeId);
 
             double newWeight = entry.getWeightOfVisitedPath() + entryOther.getWeightOfVisitedPath() + turnCostsAtBridgeNode;
             if (newWeight < bestPath.getWeight()) {
@@ -160,9 +154,12 @@ protected int getTraversalId(EdgeIteratorState edge, int origEdgeId, boolean rev
     protected boolean accept(EdgeIteratorState edge, SPTEntry currEdge, boolean reverse) {
         final int incEdge = getIncomingEdge(currEdge);
         final int prevOrNextEdgeId = getOrigEdgeId(edge, !reverse);
-        if (!traversalMode.hasUTurnSupport() && turnCostExtension.isUTurn(incEdge, prevOrNextEdgeId))
+        double turnWeight = reverse
+                ? turnWeighting.calcTurnWeight(prevOrNextEdgeId, edge.getBaseNode(), incEdge)
+                : turnWeighting.calcTurnWeight(incEdge, edge.getBaseNode(), prevOrNextEdgeId);
+        if (Double.isInfinite(turnWeight)) {
             return false;
-
+        }
         return additionalEdgeFilter == null || additionalEdgeFilter.accept(edge);
     }
 

core/src/main/java/com/graphhopper/routing/AbstractRoutingAlgorithm.java

@@ -71,7 +71,9 @@ public RoutingAlgorithm setEdgeFilter(EdgeFilter additionalEdgeFilter) {
     }
 
     protected boolean accept(EdgeIteratorState iter, int prevOrNextEdgeId) {
-        if (!traversalMode.hasUTurnSupport() && iter.getEdge() == prevOrNextEdgeId)
+        // for edge-based traversal we leave it for TurnWeighting to decide whether or not a u-turn is acceptable,
+        // but for node-based traversal we exclude such a turn for performance reasons already here
+        if (!traversalMode.isEdgeBased() && iter.getEdge() == prevOrNextEdgeId)
             return false;
 
         return additionalEdgeFilter == null || additionalEdgeFilter.accept(iter);

core/src/main/java/com/graphhopper/routing/Dijkstra.java

@@ -80,10 +80,11 @@ protected void runAlgo() {
                 if (!accept(iter, currEdge.edge))
                     continue;
 
-                int traversalId = traversalMode.createTraversalId(iter, false);
                 double tmpWeight = weighting.calcWeight(iter, false, currEdge.edge) + currEdge.weight;
-                if (Double.isInfinite(tmpWeight))
+                if (Double.isInfinite(tmpWeight)) {
                     continue;
+                }
+                int traversalId = traversalMode.createTraversalId(iter, false);
 
                 SPTEntry nEdge = fromMap.get(traversalId);
                 if (nEdge == null) {

core/src/main/java/com/graphhopper/routing/PathBidirRef.java

@@ -24,8 +24,6 @@
 import com.graphhopper.util.EdgeIterator;
 import com.graphhopper.util.EdgeIteratorState;
 
-import static com.graphhopper.util.EdgeIterator.NO_EDGE;
-
 /**
  * This class creates a DijkstraPath from two Edge's resulting from a BidirectionalDijkstra
  * <p>
@@ -118,19 +116,7 @@ private void processTurnAtMeetingPoint() {
     protected void processEdgeBwd(int edgeId, int adjNode, int nextEdgeId) {
         EdgeIteratorState edge = graph.getEdgeIteratorState(edgeId, adjNode);
         distance += edge.getDistance();
-        // special case for loop edges: since they do not have a meaningful direction we always need to read them
-        // in the 'fwd' direction, but be careful the turn costs have to be applied with reverse = true, see also
-        // same comment in EdgeBasedPath4CH
-        // todonow: consolidate this!
-        if (edge.getBaseNode() == edge.getAdjNode()) {
-            long millis = weighting.calcMillis(edge, false, NO_EDGE);
-            if (weighting instanceof TurnWeighting && EdgeIterator.Edge.isValid(nextEdgeId)) {
-                millis += 1000 * (long) ((TurnWeighting) weighting).calcTurnWeight(edge.getEdge(), edge.getBaseNode(), nextEdgeId);
-            }
-            time += millis;
-        } else {
-            time += weighting.calcMillis(edge, true, nextEdgeId);
-        }
+        time += weighting.calcMillis(edge, true, nextEdgeId);
         addEdge(edgeId);
     }
 
@@ -140,10 +126,6 @@ private void processTurn(int inEdge, int viaNode, int outEdge) {
         }
         if (weighting instanceof TurnWeighting) {
             time += ((TurnWeighting) weighting).calcTurnWeight(inEdge, viaNode, outEdge) * 1000;
-            // bugfix: should be cleaned up with u-turn refactoring #1520
-            if (inEdge == outEdge) {
-                time += ((TurnWeighting) weighting).getDefaultTurnCost() * 1000;
-            }
         }
     }
 

core/src/main/java/com/graphhopper/routing/ch/CHAlgoFactoryDecorator.java

@@ -320,7 +320,7 @@ public void createPreparations(GraphHopperStorage ghStorage) {
             addPreparation(createCHPreparation(ghStorage, weighting, TraversalMode.NODE_BASED));
         }
         for (Weighting weighting : edgeBasedWeightings) {
-            addPreparation(createCHPreparation(ghStorage, weighting, TraversalMode.EDGE_BASED_2DIR));
+            addPreparation(createCHPreparation(ghStorage, weighting, TraversalMode.EDGE_BASED));
         }
     }
 

core/src/main/java/com/graphhopper/routing/ch/EdgeBasedNodeContractor.java

@@ -24,8 +24,6 @@
 import com.graphhopper.routing.util.FlagEncoder;
 import com.graphhopper.routing.weighting.TurnWeighting;
 import com.graphhopper.storage.CHGraph;
-import com.graphhopper.storage.GraphHopperStorage;
-import com.graphhopper.storage.IntsRef;
 import com.graphhopper.util.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -89,6 +87,9 @@ public EdgeBasedNodeContractor(CHGraph prepareGraph,
         this.encoder = turnWeighting.getFlagEncoder();
         this.pMap = pMap;
         extractParams(pMap);
+        if (!Double.isInfinite(turnWeighting.getUTurnCost())) {
+            throw new IllegalArgumentException("edge-based CH currently does not support finite u-turn costs");
+        }
     }
 
     private void extractParams(PMap pMap) {
@@ -239,7 +240,7 @@ private void handleShortcuts(CHEntry chEntry, CHEntry root) {
     /**
      * A given potential loop shortcut is only necessary if there is at least one pair of original in- & out-edges for
      * which taking the loop is cheaper than doing the direct turn. However this is almost always the case, because
-     * doing a u-turn at any of the incoming edges is forbidden, i.e. he costs of the direct turn will be infinite.
+     * doing a u-turn at any of the incoming edges is forbidden, i.e. the costs of the direct turn will be infinite.
      */
     private boolean loopShortcutNecessary(int node, int firstOrigEdge, int lastOrigEdge, double loopWeight) {
         EdgeIterator inIter = loopAvoidanceInEdgeExplorer.setBaseNode(node);
@@ -319,9 +320,6 @@ private boolean isSameShortcut(CHEdgeIteratorState iter, int adjNode, int firstO
     }
 
     private double getTurnCost(int inEdge, int node, int outEdge) {
-        if (illegalUTurn(outEdge, inEdge)) {
-            return Double.POSITIVE_INFINITY;
-        }
         return turnWeighting.calcTurnWeight(inEdge, node, outEdge);
     }
 
@@ -332,10 +330,6 @@ private void resetEdgeCounters() {
         numPrevOrigEdges = 0;
     }
 
-    private boolean illegalUTurn(int inEdge, int outEdge) {
-        return outEdge == inEdge;
-    }
-
     private Stats stats() {
         return activeShortcutHandler.getStats();
     }

core/src/main/java/com/graphhopper/routing/ch/EdgeBasedPathCH.java

@@ -5,12 +5,8 @@
 import com.graphhopper.storage.Graph;
 import com.graphhopper.storage.SPTEntry;
 import com.graphhopper.storage.ShortcutUnpacker;
-import com.graphhopper.util.CHEdgeIteratorState;
-import com.graphhopper.util.EdgeIterator;
 import com.graphhopper.util.EdgeIteratorState;
 
-import static com.graphhopper.util.EdgeIterator.NO_EDGE;
-
 
 public class EdgeBasedPathCH extends Path4CH {
 
@@ -30,19 +26,7 @@ protected ShortcutUnpacker getShortcutUnpacker(Graph routingGraph, final Weighti
             @Override
             public void visit(EdgeIteratorState edge, boolean reverse, int prevOrNextEdgeId) {
                 distance += edge.getDistance();
-                // a one-way loop that is not a shortcut cannot possibly be read in the 'right' direction, because
-                // there is no way to distinguish the two directions. therefore we always read it in fwd direction.
-                // reverse still has to be considered to decide how to calculate the turn weight
-                // todo: turn cost clean-up, should we move this inside calcMillis ?
-                if (reverse && edge.getBaseNode() == edge.getAdjNode() && !((CHEdgeIteratorState) edge).isShortcut()) {
-                    long millis = weighting.calcMillis(edge, false, NO_EDGE);
-                    if (EdgeIterator.Edge.isValid(prevOrNextEdgeId)) {
-                        millis += 1000 * (long) turnWeighting.calcTurnWeight(edge.getEdge(), edge.getBaseNode(), prevOrNextEdgeId);
-                    }
-                    time += millis;
-                } else {
-                    time += weighting.calcMillis(edge, reverse, prevOrNextEdgeId);
-                }
+                time += turnWeighting.calcMillis(edge, reverse, prevOrNextEdgeId);
                 addEdge(edge.getEdge());
             }
         }, true);

core/src/main/java/com/graphhopper/routing/ch/PreparationWeighting.java

@@ -54,6 +54,9 @@ public double calcWeight(EdgeIteratorState edgeState, boolean reverse, int prevO
 
     @Override
     public long calcMillis(EdgeIteratorState edgeState, boolean reverse, int prevOrNextEdgeId) {
+        if (edgeState instanceof CHEdgeIteratorState && ((CHEdgeIteratorState) edgeState).isShortcut()) {
+            throw new IllegalStateException("calcMillis should only be called on original edges");
+        }
         return userWeighting.calcMillis(edgeState, reverse, prevOrNextEdgeId);
     }
 

core/src/main/java/com/graphhopper/routing/ch/WitnessPathSearcher.java

@@ -229,10 +229,6 @@ public CHEntry runSearch(int targetNode, int targetEdge) {
                 if (isContracted(iter.getAdjNode())) {
                     continue;
                 }
-                // do not allow u-turns
-                if (iter.getOrigEdgeFirst() == incEdges[currKey]) {
-                    continue;
-                }
                 double edgeWeight = turnWeighting.calcWeight(iter, false, incEdges[currKey]);
                 double weight = edgeWeight + weights[currKey];
                 if (isInfinite(weight)) {
@@ -479,9 +475,6 @@ private int getEdgeKey(int edge, int adjNode) {
     }
 
     private double calcTurnWeight(int inEdge, int viaNode, int outEdge) {
-        if (inEdge == outEdge) {
-            return Double.POSITIVE_INFINITY;
-        }
         return turnWeighting.calcTurnWeight(inEdge, viaNode, outEdge);
     }
 

core/src/main/java/com/graphhopper/routing/util/TestAlgoCollector.java

@@ -54,7 +54,7 @@ public TestAlgoCollector assertDistance(EncodingManager encodingManager, AlgoHel
         FlagEncoder encoder = opts.getWeighting().getFlagEncoder();
         if (encoder.supports(TurnWeighting.class)) {
             if (!opts.getTraversalMode().isEdgeBased()) {
-                errors.add("Cannot use TurnWeighting with a node based traversal");
+                errors.add("Cannot use TurnWeighting with node based traversal");
                 return this;
             }
             algoEntry.setAlgorithmOptions(AlgorithmOptions.start(opts).weighting(new TurnWeighting(opts.getWeighting(), (TurnCostExtension) queryGraph.getExtension())).build());

core/src/main/java/com/graphhopper/routing/util/TraversalMode.java

@@ -37,25 +37,16 @@ public enum TraversalMode {
     /**
      * The simplest traversal mode but without turn restrictions or cost support.
      */
-    NODE_BASED(false, false),
+    NODE_BASED(false),
     /**
-     * The bidirectional edged-based traversal mode with turn restriction and cost support. Without
-     * u-turn support. 2 times slower than node based.
+     * The edged-based traversal mode with turn restriction and cost support. 2 times slower than node based.
      */
-    EDGE_BASED_2DIR(true, false),
-    /**
-     * Not recommended as it leads to strange routes that outsmart the turn costs. The most feature
-     * rich edge-based traversal mode with turn restriction and cost support, including u-turns. 4
-     * times slower than node based.
-     */
-    EDGE_BASED_2DIR_UTURN(true, true);
+    EDGE_BASED(true);
 
     private final boolean edgeBased;
-    private final boolean uTurnSupport;
 
-    TraversalMode(boolean edgeBased, boolean uTurnSupport) {
+    TraversalMode(boolean edgeBased) {
         this.edgeBased = edgeBased;
-        this.uTurnSupport = uTurnSupport;
     }
 
     public static TraversalMode fromString(String name) {
@@ -102,7 +93,4 @@ public boolean isEdgeBased() {
         return edgeBased;
     }
 
-    public final boolean hasUTurnSupport() {
-        return uTurnSupport;
-    }
 }

core/src/main/java/com/graphhopper/routing/weighting/AbstractWeighting.java

@@ -46,6 +46,12 @@ protected AbstractWeighting(FlagEncoder encoder) {
 
     @Override
     public long calcMillis(EdgeIteratorState edgeState, boolean reverse, int prevOrNextEdgeId) {
+        // special case for loop edges: since they do not have a meaningful direction we always need to read them in
+        // forward direction
+        if (edgeState.getBaseNode() == edgeState.getAdjNode()) {
+            reverse = false;
+        }
+
         if (reverse && !edgeState.getReverse(accessEnc) || !reverse && !edgeState.get(accessEnc))
             throw new IllegalStateException("Calculating time should not require to read speed from edge in wrong direction. " +
                     "(" + edgeState.getBaseNode() + " - " + edgeState.getAdjNode() + ") "