clamp for overflow error

Project-OSRM · Dec 8, 2018 · e9a5471 · e9a5471
1 parent 5a0aaa9
commit e9a5471
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 16 deletions.
diff --git a/docs/http.md b/docs/http.md
@@ -238,7 +238,7 @@ In addition to the [general options](#general-options) the following options are
 |annotations |`duration` (default), `distance`, or `duration,distance`|Return the requested table or tables in response. |
 |fallback_speed|`double > 0`| If no route found between a source/destination pair, calculate the as-the-crow-flies distance, then use this speed to estimate duration.|
 |fallback_coordinate|`input` (default), or `snapped`| When using a `fallback_speed`, use the user-supplied coordinate (`input`), or the snapped location (`snapped`) for calculating distances.|
-|scale_factor|`double`| Multiplies the table `duration` values by this number.|
+|scale_factor|`double > 0`| Scales the table `duration` values by this number. |
 
 Unlike other array encoded options, the length of `sources` and `destinations` can be **smaller or equal**
 to number of input locations;

diff --git a/features/testbot/duration_matrix.feature b/features/testbot/duration_matrix.feature
@@ -582,6 +582,24 @@ Feature: Basic Duration Matrix
             | f | 18 | 12 | 0  | 30 |
             | 1 | 24 | 18 | 30 | 0  |
 
+    Scenario: Testbot - Travel time matrix of minimal network with scale factor
+         Given the query options
+            | scale_factor | 2 |
+
+        Given the node map
+            """
+            a b
+            """
+
+        And the ways
+            | nodes |
+            | ab    |
+
+        When I request a travel time matrix I should get
+            |   | a  | b  |
+            | a | 0  | 20 |
+            | b | 20 | 0  |
+
     Scenario: Testbot - Test fallback speeds and scale factor
         Given a grid size of 300 meters
         Given the extract extra arguments "--small-component-size 4"
@@ -608,9 +626,9 @@ Feature: Basic Duration Matrix
             | f | 36 | 24 | 0  | 60 |
             | 1 | 48 | 36 | 60 | 0  |
 
-    Scenario: Testbot - Travel time matrix of minimal network with scale factor
+    Scenario: Testbot - Travel time matrix of minimal network with overflow scale factor
          Given the query options
-            | scale_factor | 2 |
+            | scale_factor | 2147483647 |
 
         Given the node map
             """
@@ -623,12 +641,12 @@ Feature: Basic Duration Matrix
 
         When I request a travel time matrix I should get
             |   | a  | b  |
-            | a | 0  | 20 |
-            | b | 20 | 0  |
+            | a | 0  | 214748364.6 |
+            | b | 214748364.6 | 0  |
 
-    Scenario: Testbot - Travel time matrix of minimal network with overflow factor
+    Scenario: Testbot - Travel time matrix of minimal network with fraction scale factor
          Given the query options
-            | scale_factor | 2147483647 |
+            | scale_factor | 0.5 |
 
         Given the node map
             """
@@ -641,5 +659,5 @@ Feature: Basic Duration Matrix
 
         When I request a travel time matrix I should get
             |   | a  | b  |
-            | a | 0  | 20 |
-            | b | 20 | 0  |
+            | a | 0  | 5  |
+            | b | 5  | 0  |
diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp
@@ -1238,6 +1238,11 @@ argumentsToTableParameter(const Nan::FunctionCallbackInfo<v8::Value> &args,
             Nan::ThrowError("scale_factor must be a number");
             return table_parameters_ptr();
         }
+        else if (scale_factor->NumberValue() < 0)
+        {
+            Nan::ThrowError("scale_factor must be > 0");
+            return table_parameters_ptr();
+        }
 
         params->scale_factor = static_cast<double>(scale_factor->NumberValue());
     }

diff --git a/src/engine/plugins/table.cpp b/src/engine/plugins/table.cpp
@@ -96,7 +96,7 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
     }
 
     // Scan table for null results - if any exist, replace with distance estimates
-    if (params.fallback_speed > 0 || params.scale_factor != 1.0)
+    if (params.fallback_speed > 0 || params.scale_factor > 0)
     {
         for (std::size_t row = 0; row < num_sources; row++)
         {
@@ -128,14 +128,16 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
                         result_tables_pair.second[table_index] = distance_estimate;
                     }
                 }
-                if (params.scale_factor != 1.0 &&
-                    result_tables_pair.first[table_index] != MAXIMAL_EDGE_DURATION)
+                if (params.scale_factor > 0 && params.scale_factor != 1 &&
+                    result_tables_pair.first[table_index] != MAXIMAL_EDGE_DURATION &&
+                    result_tables_pair.first[table_index] != 0)
                 {
-                    double result = std::lround(result_tables_pair.first[table_index] * params.scale_factor);
-                    if (result > MAXIMAL_EDGE_DURATION) {
-                      result_tables_pair.first[table_index] = MAXIMAL_EDGE_DURATION - 1;
+                    EdgeDuration diff = MAXIMAL_EDGE_DURATION / result_tables_pair.first[table_index];
+
+                    if (params.scale_factor >= diff) {
+                        result_tables_pair.first[table_index] = MAXIMAL_EDGE_DURATION - 1;
                     } else {
-                      result_tables_pair.first[table_index] = result;
+                        result_tables_pair.first[table_index] = std::lround(result_tables_pair.first[table_index] * params.scale_factor);
                     }
                 }
             }

diff --git a/src/server/service/table_service.cpp b/src/server/service/table_service.cpp
@@ -61,6 +61,11 @@ std::string getWrongOptionHelp(const engine::api::TableParameters &parameters)
         help = "fallback_speed must be > 0";
     }
 
+    if (parameters.scale_factor < 0)
+    {
+        help = "scale_factor must be > 0";
+    }
+
     return help;
 }
 } // anon. ns