Allow single edge paths in MLD alternatives, #4691

cucumber.js

@@ -1,6 +1,6 @@
 module.exports = {
-    default: '--strict --tags ~@stress --tags ~@todo --require features/support --require features/step_definitions',
+    default: '--strict --tags ~@stress --tags ~@todo --tags ~@mld-only --require features/support --require features/step_definitions',
-    verify: '--strict --tags ~@stress --tags ~@todo -f progress --require features/support --require features/step_definitions',
+    verify: '--strict --tags ~@stress --tags ~@todo --tags ~@mld-only -f progress --require features/support --require features/step_definitions',
     todo: '--strict --tags @todo --require features/support --require features/step_definitions',
     all: '--strict --require features/support --require features/step_definitions',
     mld: '--strict --tags ~@stress --tags ~@todo --require features/support --require features/step_definitions -f progress'

features/testbot/alternative_loop.feature

@@ -31,40 +31,32 @@ Feature: Alternative route
             | 5    | 6  | dc,ca,ab,bd,dc,dc |             |
             | 7    | 8  | ca,ab,bd,dc,ca,ca |             |
 
-    # This test case does not work in a platform independent way
+
-    # since it depends on a specific CH structure that is only
+    @mld-only
-    # present on linux it seems.
+    Scenario: Alternative loop paths on a single node with an asymmetric circle
-    @4111 @todo
+        # The test checks only MLD implementation, alternatives results are unpredictable for CH on windows (#4691, #4693)
-    Scenario: Alternative Loop Paths with single node path
+        Given a grid size of 10 meters
         Given the node map
             """
-            a1b2c3d
+              a b  c
-
+            l       d
-
+            k       e
-            e     f
+            j       f
+              i h g
             """
 
+        And the nodes
+            | node | barrier |
+            | i    | bollard |
+            | g    | bollard |
+
         And the ways
-            | nodes | maxspeed |
+            | nodes         | oneway |
-            | ab    |      30  |
+            | abcdefghijkla | no     |
-            | bc    |       3  |
-            | cd    |      30  |
-            | ae    |      30  |
-            | ef    |      30  |
-            | fd    |      30  |
 
         And the query options
             | alternatives | true |
 
         When I route I should get
-            | from | to | route    | alternative       |
+            | from | to | route                       | alternative                 | weight |
-            | b    | c  | bc,bc    | ab,ae,ef,fd,cd,cd |
+            | e    | k  | abcdefghijkla,abcdefghijkla | abcdefghijkla,abcdefghijkla |    6.8 |
-            #| c    | b  | bc,bc    | cd,fd,ef,ae,ab,ab | # alternative path depends on phantom snapping order
-            | 1    | c  | ab,bc,bc | ab,ae,ef,fd,cd,cd |
-            #| c    | 1  | bc,ab    | cd,fd,ef,ae,ab    | # alternative path depends on phantom snapping order
-            | 2 | c | bc,bc          |                   |
-            | c | 2 | bc,bc          |                   |
-            | 1 | 3 | ab,ae,ef,fd,cd | ab,bc,cd          |
-            #| 3 | 1 | cd,fd,ef,ae,ab | cd,bc,ab          | # alternative path depends on phantom snapping order
-            | b | 3 | bc,cd          | ab,ae,ef,fd,cd    |
-            #| 3 | b | cd,bc,bc       | cd,fd,ef,ae,ab,ab | # alternative path depends on phantom snapping order

src/engine/routing_algorithms/alternative_path_mld.cpp

@@ -233,6 +233,12 @@ RandIt filterPackedPathsByCellSharing(RandIt first, RandIt last, const Partition
         cells.insert(get_cell(std::get<1>(edge)));
 
     const auto over_sharing_limit = [&](const auto &packed) {
+
+        if (packed.path.empty())
+        { // don't remove routes with single-node (empty) path
+            return false;
+        }
+
         const auto not_seen = [&](const PackedEdge edge) {
             const auto source_cell = get_cell(std::get<0>(edge));
             const auto target_cell = get_cell(std::get<1>(edge));
@@ -310,7 +316,7 @@ RandIt filterPackedPathsByLocalOptimality(const WeightedViaNodePackedPath &path,
         // Check plateaux edges towards the target. Terminates at the source / target
         // at the latest, since parent(target)==target for the reverse heap and
         // parent(target) != target in the forward heap (and vice versa).
-        while (has_plateaux_at_node(node, fst, snd))
+        while (node != fst.GetData(node).parent && has_plateaux_at_node(node, fst, snd))
             node = fst.GetData(node).parent;
 
         return node;
@@ -320,7 +326,11 @@ RandIt filterPackedPathsByLocalOptimality(const WeightedViaNodePackedPath &path,
         BOOST_ASSERT(packed.via.node != path.via.node);
         BOOST_ASSERT(packed.via.weight != INVALID_EDGE_WEIGHT);
         BOOST_ASSERT(packed.via.node != SPECIAL_NODEID);
-        BOOST_ASSERT(!packed.path.empty());
+
+        if (packed.path.empty())
+        { // the edge case when packed.via.node is both source and target node
+            return false;
+        }
 
         const NodeID via = packed.via.node;
 
@@ -395,6 +405,12 @@ template <typename RandIt> RandIt filterUnpackedPathsBySharing(RandIt first, Ran
     edges.insert(begin(shortest_path.edges), begin(shortest_path.edges));
 
     const auto over_sharing_limit = [&](const auto &unpacked) {
+
+        if (unpacked.edges.empty())
+        { // don't remove routes with single-node (empty) path
+            return false;
+        }
+
         const auto not_seen = [&](const EdgeID edge) { return edges.count(edge) < 1; };
         const auto different = std::count_if(begin(unpacked.edges), end(unpacked.edges), not_seen);