Fix distance, time and weight for edge-based CH alternatives, fix graphhopper#2936

Author: easbar

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

@@ -21,6 +21,7 @@
 import com.carrotsearch.hppc.IntIndexedContainer;
 import com.carrotsearch.hppc.cursors.IntCursor;
 import com.carrotsearch.hppc.predicates.IntObjectPredicate;
+import com.graphhopper.routing.weighting.Weighting;
 import com.graphhopper.storage.Graph;
 import com.graphhopper.storage.RoutingCHGraph;
 import com.graphhopper.util.EdgeIteratorState;
@@ -136,7 +137,7 @@ List<AlternativeInfo> calcAlternatives(final int s, final int t) {
                 // at u->v->x in which case uvtPath is not found. If we do not stop here we might return
                 // an alternative that does not even reach t, and has a lower weight than the best path.
                 continue;
-            Path path = concat(graph.getBaseGraph(), suvPath, uvtPath);
+            Path path = concat(graph.getBaseGraph(), graph.getBaseGraph().wrapWeighting(graph.getWeighting()), suvPath, uvtPath);
             extraVisitedNodes += vtRouter.getVisitedNodes();
 
             double sharedDistanceWithShortest = sharedDistanceWithShortest(path);
@@ -242,22 +243,22 @@ private EdgeIteratorState getNextNodeTMetersAway(Path path, int vIndex, double T
         return edges.get(i - 1);
     }
 
-    private static Path concat(Graph graph, Path suvPath, Path uvtPath) {
+    private static Path concat(Graph graph, Weighting weighting, Path suvPath, Path uvtPath) {
         assert suvPath.isFound();
         assert uvtPath.isFound();
         Path path = new Path(graph);
-        path.setFromNode(suvPath.calcNodes().get(0));
+        path.setFromNode(suvPath.getFromNode());
         path.getEdges().addAll(suvPath.getEdges());
+        if (uvtPath.getEdges().isEmpty())
+            throw new IllegalStateException("uvtPath.getEdges() should not be empty");
         Iterator<IntCursor> uvtPathI = uvtPath.getEdges().iterator();
-        if (!uvtPathI.hasNext()) { // presumably v == t, has been known to happen, no test yet
-            return suvPath;
-        }
-        uvtPathI.next(); // skip u-v edge
+        int uvEdge = uvtPathI.next().value;// skip u-v edge
         uvtPathI.forEachRemaining(edge -> path.addEdge(edge.value));
+        EdgeIteratorState vuEdgeState = graph.getEdgeIteratorState(uvEdge, uvtPath.getFromNode());
         path.setEndNode(uvtPath.getEndNode());
-        path.setWeight(suvPath.getWeight() + uvtPath.getWeight());
-        path.setDistance(suvPath.getDistance() + uvtPath.getDistance());
-        path.addTime(suvPath.getTime() + uvtPath.getTime());
+        path.setWeight(suvPath.getWeight() + uvtPath.getWeight() - weighting.calcEdgeWeight(vuEdgeState, true));
+        path.setDistance(suvPath.getDistance() + uvtPath.getDistance() - vuEdgeState.getDistance());
+        path.addTime(suvPath.getTime() + uvtPath.getTime() - weighting.calcEdgeMillis(vuEdgeState, true));
         path.setFound(true);
         return path;
     }

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

@@ -101,10 +101,7 @@ public Path setEndNode(int end) {
         return this;
     }
 
-    /**
-     * @return the first node of this Path.
-     */
-    private int getFromNode() {
+    public int getFromNode() {
         if (fromNode < 0)
             throw new IllegalStateException("fromNode < 0 should not happen");
 

core/src/test/java/com/graphhopper/routing/AlternativeRouteEdgeCHTest.java

@@ -32,8 +32,7 @@
 import java.util.List;
 
 import static com.graphhopper.util.EdgeIterator.ANY_EDGE;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 public class AlternativeRouteEdgeCHTest {
     private final DecimalEncodedValue speedEnc = new DecimalEncodedValueImpl("speed", 5, 5, false);
@@ -183,4 +182,46 @@ void turnRestrictionAtConnectingNode() {
             assertEquals(IntArrayList.from(s, t), pathInfos.get(j).path.calcNodes(), "alternatives must start/end at start/end node");
         }
     }
+
+    @Test
+    void distanceTimeAndWeight() {
+        final BaseGraph graph = new BaseGraph.Builder(em).withTurnCosts(true).create();
+        //   0-1-2-3---|
+        //        \    |
+        //         5-6-7-8
+        graph.edge(0, 1).setDistance(500).set(speedEnc, 60);
+        graph.edge(1, 2).setDistance(500).set(speedEnc, 60);
+        graph.edge(2, 3).setDistance(500).set(speedEnc, 60);
+        graph.edge(2, 5).setDistance(1000).set(speedEnc, 60);
+        graph.edge(3, 7).setDistance(1500).set(speedEnc, 60);
+        graph.edge(5, 6).setDistance(500).set(speedEnc, 60);
+        graph.edge(6, 7).setDistance(500).set(speedEnc, 60);
+        graph.edge(7, 8).setDistance(500).set(speedEnc, 60);
+        graph.freeze();
+        CHConfig chConfig = CHConfig.edgeBased("profile", new SpeedWeighting(speedEnc, turnCostEnc, graph.getTurnCostStorage(), Double.POSITIVE_INFINITY));
+        PrepareContractionHierarchies contractionHierarchies = PrepareContractionHierarchies.fromGraph(graph, chConfig);
+        contractionHierarchies.useFixedNodeOrdering(NodeOrderingProvider.fromArray(7, 6, 0, 2, 4, 5, 1, 3, 8));
+        PrepareContractionHierarchies.Result res = contractionHierarchies.doWork();
+        RoutingCHGraph routingCHGraph = RoutingCHGraphImpl.fromGraph(graph, res.getCHStorage(), res.getCHConfig());
+        final int s = 0;
+        final int t = 8;
+        PMap hints = new PMap();
+        AlternativeRouteEdgeCH altDijkstra = new AlternativeRouteEdgeCH(routingCHGraph, hints);
+        List<AlternativeRouteEdgeCH.AlternativeInfo> pathInfos = altDijkstra.calcAlternatives(s, t);
+        assertTrue(pathInfos.size() > 1, "the graph, contraction order and alternative route algorith must be such that " +
+                "there is at least one alternative path, otherwise this test makes no sense");
+        AlternativeRouteEdgeCH.AlternativeInfo best = pathInfos.get(0);
+        assertEquals(3500, best.path.getDistance());
+        assertEquals(58.3333, best.path.getWeight(), 1.e-3);
+        assertEquals(58333, best.path.getTime(), 10);
+        assertEquals(IntArrayList.from(0, 1, 2, 5, 6, 7, 8), best.path.calcNodes());
+        for (int j = 1; j < pathInfos.size(); j++) {
+            Path alternative = pathInfos.get(j).path;
+            assertEquals(3500, alternative.getDistance());
+            assertEquals(58.3333, alternative.getWeight(), 1.e-3);
+            assertEquals(58333, alternative.getTime(), 1);
+            assertEquals(IntArrayList.from(0, 1, 2, 3, 7, 8), alternative.calcNodes());
+        }
+    }
+
 }