Merge branch 'master' into unit_test_discontinuity_matching

* master: missed leg weight to 3 decimal places (#2227) Zero Distance Match Leg Causes Nan Duration (#2229) Fixed the sharp turn phrase (#2226) Cleanup in test Cleanup whitespace in directions.proto Cleanup unused argument etp in narrativebuilder Code review feedback: test vector references, style nit Update to front and back. j/k: move TrimBuild to other geometry utilities. Test coverage, export TrimShape into header namespace; check for empty shape. Strip const from primitive types. Fix pointer invalidation bug in TrimShape. prevent calling path_map_match when path_edges are empty. (#2219)
valhalla · Feb 11, 2020 · bf9c126 · bf9c126
2 parents 5e4e1b6 + b1efedc
commit bf9c126
Show file tree

Hide file tree

Showing 17 changed files with 216 additions and 151 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,8 @@
      - Duration over-trimming at the terminating edge of a match.
    * FIXED: Increased internal precision of time tracking per edge and maneuver so that maneuver times sum to the same time represented in the leg summary [#2195](https://github.com/valhalla/valhalla/pull/2195)
    * FIXED: Tagged speeds were not properly marked. We were not using forward and backward speeds to flag if a speed is tagged or not.  Should not update turn channel speeds if we are not inferring them.  Added additional logic to handle PH in the conditional restrictions. Do not update stop impact for ramps if they are marked as internal. [#2198](https://github.com/valhalla/valhalla/pull/2198)
+   * FIXED: Fixed the sharp turn phrase [#2226](https://github.com/valhalla/valhalla/pull/2226)
+   * FIXED: Protect against duplicate points in the input or points that snap to the same location resulting in `nan` times for the legs of the map match (of a 0 distance route) [#2229](https://github.com/valhalla/valhalla/pull/2229)
 
 * **Enhancement**
    * ADDED: Return the coordinates of the nodes isochrone input locations snapped to [#2111](https://github.com/valhalla/valhalla/pull/2111)

diff --git a/locales/de-DE.json b/locales/de-DE.json
@@ -1446,16 +1446,16 @@
       ],
       "example_phrases": {
         "0": [
-          "Turn sharp right."
+          "Make a sharp right."
         ],
         "1": [
-          "Turn sharp left onto Arlen Road."
+          "Make a sharp left onto Arlen Road."
         ],
         "2": [
-          "Turn sharp right onto Belair Road/US 1 Business. Continue on US 1 Business."
+          "Make a sharp right onto Belair Road/US 1 Business. Continue on US 1 Business."
         ],
         "3": [
-          "Turn sharp left to stay on US 15 South."
+          "Make a sharp left to stay on US 15 South."
         ]
       }
     },
@@ -1477,16 +1477,16 @@
       ],
       "example_phrases": {
         "0": [
-          "Turn sharp right."
+          "Make a sharp right."
         ],
         "1": [
-          "Turn sharp left onto Arlen Road."
+          "Make a sharp left onto Arlen Road."
         ],
         "2": [
-          "Turn sharp right onto Belair Road, U.S. 1 Business."
+          "Make a sharp right onto Belair Road, U.S. 1 Business."
         ],
         "3": [
-          "Turn sharp left to stay on U.S. 15 South."
+          "Make a sharp left to stay on U.S. 15 South."
         ]
       }
     },

diff --git a/locales/en-US-x-pirate.json b/locales/en-US-x-pirate.json
@@ -1430,10 +1430,10 @@
     },
     "sharp": {
       "phrases": {
-        "0": "Turn sharp <RELATIVE_DIRECTION> bucko.",
-        "1": "Turn sharp <RELATIVE_DIRECTION> onto <STREET_NAMES> ye scallywag.",
-        "2": "Arrr, turn sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>. Continue on <STREET_NAMES>.",
-        "3": "Turn sharp <RELATIVE_DIRECTION> matey to stay on <STREET_NAMES>."
+        "0": "Make a sharp <RELATIVE_DIRECTION> bucko.",
+        "1": "Make a sharp <RELATIVE_DIRECTION> onto <STREET_NAMES> ye scallywag.",
+        "2": "Arrr, make a sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>. Continue on <STREET_NAMES>.",
+        "3": "Make a sharp <RELATIVE_DIRECTION> matey to stay on <STREET_NAMES>."
       },
       "empty_street_name_labels": [
         "solid ground",
@@ -1446,25 +1446,25 @@
       ],
       "example_phrases": {
         "0": [
-          "Turn sharp left bucko."
+          "Make a sharp left bucko."
         ],
         "1": [
-          "Turn sharp right onto Flatbush Avenue ye scallywag."
+          "Make a sharp right onto Flatbush Avenue ye scallywag."
         ],
         "2": [
-          "Arrr, turn sharp left onto North Bond Street/US 1 Business/MD 924. Continue on MD 924."
+          "Arrr, make a sharp left onto North Bond Street/US 1 Business/MD 924. Continue on MD 924."
         ],
         "3": [
-          "Turn sharp right matey to stay on Sunstone Drive."
+          "Make a sharp right matey to stay on Sunstone Drive."
         ]
       }
     },
     "sharp_verbal": {
       "phrases": {
-        "0": "Turn sharp <RELATIVE_DIRECTION> bucko.",
-        "1": "Turn sharp <RELATIVE_DIRECTION> onto <STREET_NAMES> ye scallywag.",
-        "2": "Arrr, turn sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
-        "3": "Turn sharp <RELATIVE_DIRECTION> matey to stay on <STREET_NAMES>."
+        "0": "Make a sharp <RELATIVE_DIRECTION> bucko.",
+        "1": "Make a sharp <RELATIVE_DIRECTION> onto <STREET_NAMES> ye scallywag.",
+        "2": "Arrr, make a sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
+        "3": "Make a sharp <RELATIVE_DIRECTION> matey to stay on <STREET_NAMES>."
       },
       "empty_street_name_labels": [
         "solid ground",
@@ -1477,16 +1477,16 @@
       ],
       "example_phrases": {
         "0": [
-          "Turn sharp left bucko."
+          "Make a sharp left bucko."
         ],
         "1": [
-          "Turn sharp right onto Flatbush Avenue ye scallywag."
+          "Make a sharp right onto Flatbush Avenue ye scallywag."
         ],
         "2": [
-          "Arrr, turn sharp left onto North Bond Street, U.S. 1 Business."
+          "Arrr, make a sharp left onto North Bond Street, U.S. 1 Business."
         ],
         "3": [
-          "Turn sharp right matey to stay on Sunstone Drive."
+          "Make a sharp right matey to stay on Sunstone Drive."
         ]
       }
     },

diff --git a/locales/en-US.json b/locales/en-US.json
@@ -1430,10 +1430,10 @@
     },
     "sharp": {
       "phrases": {
-        "0": "Turn sharp <RELATIVE_DIRECTION>.",
-        "1": "Turn sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
-        "2": "Turn sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>. Continue on <STREET_NAMES>.",
-        "3": "Turn sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
+        "0": "Make a sharp <RELATIVE_DIRECTION>.",
+        "1": "Make a sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
+        "2": "Make a sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>. Continue on <STREET_NAMES>.",
+        "3": "Make a sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
       },
       "empty_street_name_labels": [
         "the walkway",
@@ -1446,25 +1446,25 @@
       ],
       "example_phrases": {
         "0": [
-          "Turn sharp left."
+          "Make a sharp left."
         ],
         "1": [
-          "Turn sharp right onto Flatbush Avenue."
+          "Make a sharp right onto Flatbush Avenue."
         ],
         "2": [
-          "Turn sharp left onto North Bond Street/US 1 Business/MD 924. Continue on MD 924."
+          "Make a sharp left onto North Bond Street/US 1 Business/MD 924. Continue on MD 924."
         ],
         "3": [
-          "Turn sharp right to stay on Sunstone Drive."
+          "Make a sharp right to stay on Sunstone Drive."
         ]
       }
     },
     "sharp_verbal": {
       "phrases": {
-        "0": "Turn sharp <RELATIVE_DIRECTION>.",
-        "1": "Turn sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
-        "2": "Turn sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
-        "3": "Turn sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
+        "0": "Make a sharp <RELATIVE_DIRECTION>.",
+        "1": "Make a sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
+        "2": "Make a sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
+        "3": "Make a sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
       },
       "empty_street_name_labels": [
         "the walkway",
@@ -1477,16 +1477,16 @@
       ],
       "example_phrases": {
         "0": [
-          "Turn sharp left."
+          "Make a sharp left."
         ],
         "1": [
-          "Turn sharp right onto Flatbush Avenue."
+          "Make a sharp right onto Flatbush Avenue."
         ],
         "2": [
-          "Turn sharp left onto North Bond Street, U.S. 1 Business."
+          "Make a sharp left onto North Bond Street, U.S. 1 Business."
         ],
         "3": [
-          "Turn sharp right to stay on Sunstone Drive."
+          "Make a sharp right to stay on Sunstone Drive."
         ]
       }
     },

diff --git a/proto/directions.proto b/proto/directions.proto
@@ -85,7 +85,7 @@ message DirectionsLeg {
       kWest = 6;
       kNorthWest = 7;
     }
-  
+
     // TODO - add others later
     enum Type {
       kNone = 0;
@@ -128,7 +128,7 @@ message DirectionsLeg {
       kMergeRight = 37;
       kMergeLeft = 38;
     }
-  
+
     message SignElement {
       optional string text = 1;               // The actual sign element text, examples: I 95 North or Derry Street
       optional bool is_route_number = 2;      // true if sign element is a reference route number such as: I 81 South or US 322 West
@@ -151,7 +151,7 @@ message DirectionsLeg {
     optional float length = 4;                               // kilometers or miles based on units
     optional double time = 5;                                // seconds
     optional CardinalDirection begin_cardinal_direction = 6; // CardinalDirection
-    optional uint32 begin_heading = 7;                       // 0-360 
+    optional uint32 begin_heading = 7;                       // 0-360
     optional uint32 begin_shape_index = 8;                   // inclusive index
     optional uint32 end_shape_index = 9;                     // inclusive index
     optional bool portions_toll = 10;                        // has portions toll
@@ -181,7 +181,7 @@ message DirectionsLeg {
     optional bool has_time_restrictions = 34;                // Whether edge has any time restrictions or not
     repeated GuidanceView guidance_views = 35;               // List of guidance views
   }
-  
+
   optional uint64 trip_id = 1;
   optional uint32 leg_id = 2;
   optional uint32 leg_count = 3;
@@ -197,4 +197,4 @@ message DirectionsRoute {
 
 message Directions {
   repeated DirectionsRoute routes = 1;
-}
+}
diff --git a/src/midgard/util.cc b/src/midgard/util.cc
@@ -89,6 +89,45 @@ template <class container_t> container_t trim_front(container_t& pts, const floa
   return result;
 }
 
+void trim_shape(float start,
+                PointLL start_vertex,
+                float end,
+                PointLL end_vertex,
+                std::vector<PointLL>& shape) {
+  // clip up to the start point if the start_vertex is valid
+  float along = 0.f;
+  auto current = shape.begin();
+  if (start_vertex.IsValid()) {
+    while (!shape.empty() && (current != shape.end() - 1)) {
+      along += (current + 1)->Distance(*current);
+      // just crossed it, replace the current vertex with the start position and erase
+      // shape up to the current vertex
+      if (along > start) {
+        along = start;
+        *current = start_vertex;
+        shape.erase(shape.begin(), current);
+        break;
+      }
+      ++current;
+    }
+  }
+  // clip after the end point if the end vertex is valid
+  current = shape.begin();
+  if (end_vertex.IsValid()) {
+    while (!shape.empty() && (current != shape.end() - 1)) {
+      along += (current + 1)->Distance(*current);
+      // just crossed it, replace the current vertex with the end vertex and erase
+      // shape after the current vertex
+      if (along > end) {
+        *(++current) = end_vertex;
+        shape.erase(++current, shape.end());
+        break;
+      }
+      ++current;
+    }
+  }
+}
+
 float tangent_angle(size_t index,
                     const PointLL& point,
                     const std::vector<PointLL>& shape,

diff --git a/src/odin/directionsbuilder.cc b/src/odin/directionsbuilder.cc
@@ -95,7 +95,7 @@ void DirectionsBuilder::Build(Api& api) {
         if (options.directions_type() == DirectionsType::instructions) {
           std::unique_ptr<NarrativeBuilder> narrative_builder =
               NarrativeBuilderFactory::Create(options, &etp);
-          narrative_builder->Build(options, &etp, maneuvers);
+          narrative_builder->Build(options, maneuvers);
         }
       }
 

diff --git a/src/odin/narrativebuilder.cc b/src/odin/narrativebuilder.cc
@@ -45,9 +45,7 @@ NarrativeBuilder::NarrativeBuilder(const Options& options,
       articulated_preposition_enabled_(false) {
 }
 
-void NarrativeBuilder::Build(const Options& options,
-                             const EnhancedTripLeg* etp,
-                             std::list<Maneuver>& maneuvers) {
+void NarrativeBuilder::Build(const Options& options, std::list<Maneuver>& maneuvers) {
   Maneuver* prev_maneuver = nullptr;
   for (auto& maneuver : maneuvers) {
     switch (maneuver.type()) {
@@ -913,11 +911,11 @@ std::string NarrativeBuilder::FormVerbalContinueInstruction(Maneuver& maneuver,
 }
 
 std::string NarrativeBuilder::FormTurnInstruction(Maneuver& maneuver) {
-  // "0": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION>.",
-  // "1": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
-  // "2": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>. Continue on
+  // "0": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION>.",
+  // "1": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
+  // "2": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>. Continue on
   // <STREET_NAMES>.",
-  // "3": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
+  // "3": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
   const TurnSubset* subset = nullptr;
   switch (maneuver.type()) {
     case DirectionsLeg_Maneuver_Type_kSlightRight:
@@ -978,21 +976,21 @@ std::string NarrativeBuilder::FormTurnInstruction(Maneuver& maneuver) {
 std::string NarrativeBuilder::FormVerbalAlertTurnInstruction(Maneuver& maneuver,
                                                              uint32_t element_max_count,
                                                              const std::string& delim) {
-  // "0": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION>.",
-  // "1": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
-  // "2": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
-  // "3": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
+  // "0": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION>.",
+  // "1": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
+  // "2": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
+  // "3": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
 
   return FormVerbalTurnInstruction(maneuver, element_max_count, delim);
 }
 
 std::string NarrativeBuilder::FormVerbalTurnInstruction(Maneuver& maneuver,
                                                         uint32_t element_max_count,
                                                         const std::string& delim) {
-  // "0": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION>.",
-  // "1": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
-  // "2": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
-  // "3": "Turn/Bear/Turn sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
+  // "0": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION>.",
+  // "1": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> onto <STREET_NAMES>.",
+  // "2": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> onto <BEGIN_STREET_NAMES>.",
+  // "3": "Turn/Bear/Make a sharp <RELATIVE_DIRECTION> to stay on <STREET_NAMES>."
 
   const TurnSubset* subset = nullptr;
   switch (maneuver.type()) {

diff --git a/src/thor/trace_route_action.cc b/src/thor/trace_route_action.cc
@@ -183,7 +183,7 @@ thor_worker_t::map_match(Api& request) {
                                            mode, disconnected_edges, options);
 
     // If we want a route but there actually isnt a path, we cant give you one
-    if (path_edges.empty() && options.action() == Options::trace_route) {
+    if (path_edges.empty()) {
       throw std::exception{};
     }
 
@@ -632,8 +632,10 @@ thor_worker_t::map_match(Api& request) {
             // we need to scale the elapsed time of the current edge to undo what FormPath did
             double begin_pct = begin_trimmed ? std::get<2>(edge_group)->distance_along : 0;
             double end_pct = end_trimmed ? std::get<3>(edge_group)->distance_along : 1;
-            double begin_edge_scale = 1.0 / ((trivial_group ? end_pct : 1) - begin_pct);
-            double end_edge_scale = 1.0 / (end_pct - (trivial_group ? begin_pct : 0));
+            double dist_from_begin = (trivial_group ? end_pct : 1) - begin_pct;
+            double dist_to_end = end_pct - (trivial_group ? begin_pct : 0);
+            double begin_edge_scale = dist_from_begin > 0 ? 1 / dist_from_begin : 0;
+            double end_edge_scale = dist_to_end > 0 ? 1 / dist_to_end : 0;
 
             // we get the time up to the last edge before this begin edge if any. we also remove
             // the turn cost at the begging of this edge if there is any