bump to rc 4

Include information on estimates in table response (#5259 )
* Revert "Remove estimated_cells value in the response." This reverts commit 364e35af06. * Update changelog. * fix linting * adjust fallback_speed check * change [].includes to [].indexOf !== -1 for compatibility with node 4 * change param name * more cuke tests * fix formatting
2018-12-11 13:20:18 -05:00 · 2018-12-11 13:17:34 -05:00 · 2018-12-11 13:16:44 -05:00
15 changed files with 201 additions and 44 deletions
@@ -1,7 +1,8 @@
-# 5.20.1 RC 3
+# 5.20.1 RC 4
  - Changes from 5.20.0
    - Features:
      - ADDED: all waypoints in responses now contain a distance property between the original coordinate and the snapped location. [#5255](https://github.com/Project-OSRM/osrm-backend/pull/5255)
+      - ADDED: if `fallback_speed` is used, a new structure `fallback_speed_cells` will describe which cells contain estimated values [#5259](https://github.com/Project-OSRM/osrm-backend/pull/5259)
    - Table:
      - ADDED: new parameter `scale_factor` which will scale the cell `duration` values by this factor. [#5298](https://github.com/Project-OSRM/osrm-backend/pull/5298)
      - FIXED: only trigger `scale_factor` code to scan matrix when necessary. [#5303](https://github.com/Project-OSRM/osrm-backend/pull/5303)
@@ -119,7 +119,6 @@ In addition to the [general options](#general-options) the following options are

 - `code` if the request was successful `Ok` otherwise see the service dependent and general status codes.
 - `waypoints` array of `Waypoint` objects sorted by distance to the input coordinate. Each object has at least the following additional properties:
-  - `distance`: Distance in meters to the supplied input coordinate.
  - `nodes`: Array of OpenStreetMap node ids.

 #### Example Requests
@@ -285,6 +284,7 @@ curl 'http://router.project-osrm.org/table/v1/driving/13.388860,52.517037;13.397
  the i-th waypoint to the j-th waypoint. Values are given in meters. Can be `null` if no route between `i` and `j` can be found. Note that computing the `distances` table is currently only implemented for CH. If `annotations=distance` or `annotations=duration,distance` is requested when running a MLD router, a `NotImplemented` error will be returned.
 - `sources` array of `Waypoint` objects describing all sources in order
 - `destinations` array of `Waypoint` objects describing all destinations in order
+- `fallback_speed_cells` (optional) array of arrays containing `i,j` pairs indicating which cells contain estimated values based on `fallback_speed`.  Will be absent if `fallback_speed` is not used.

 In case of error the following `code`s are supported in addition to the general ones:

@@ -385,6 +385,10 @@ All other properties might be undefined.
      2361.73,
      0
    ]
+  ],
+  "fallback_speed_cells": [
+    [ 0, 1 ],
+    [ 1, 0 ]
  ]
 }
 ```
@@ -907,6 +911,7 @@ Object used to describe waypoint on a route.

 - `name` Name of the street the coordinate snapped to
 - `location` Array that contains the `[longitude, latitude]` pair of the snapped coordinate
+- `distance` The distance, in metres, from the input coordinate to the snapped coordinate
 - `hint` Unique internal identifier of the segment (ephemeral, not constant over data updates)
   This can be used on subsequent request to significantly speed up the query and to connect multiple services.
   E.g. you can use the `hint` value obtained by the `nearest` query as `hint` values for `route` inputs.
@@ -157,6 +157,7 @@ Returns **[Object](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refer
                 Values are given in seconds.
 **`sources`**: array of [`Ẁaypoint`](#waypoint) objects describing all sources in order.
 **`destinations`**: array of [`Ẁaypoint`](#waypoint) objects describing all destinations in order.
+**`fallback_speed_cells`**: (optional) if `fallback_speed` is used, will be an array of arrays of `row,column` values, indicating which cells contain estimated values.

 ### tile

@@ -3,22 +3,25 @@ var util = require('util');
 module.exports = function () {
    const durationsRegex = new RegExp(/^I request a travel time matrix I should get$/);
    const distancesRegex = new RegExp(/^I request a travel distance matrix I should get$/);
+    const estimatesRegex = new RegExp(/^I request a travel time matrix I should get estimates for$/);

    const DURATIONS_NO_ROUTE = 2147483647;     // MAX_INT
    const DISTANCES_NO_ROUTE = 3.40282e+38;    // MAX_FLOAT

    this.When(durationsRegex, function(table, callback) {tableParse.call(this, table, DURATIONS_NO_ROUTE, 'durations', callback);}.bind(this));
    this.When(distancesRegex, function(table, callback) {tableParse.call(this, table, DISTANCES_NO_ROUTE, 'distances', callback);}.bind(this));
+    this.When(estimatesRegex, function(table, callback) {tableParse.call(this, table, DISTANCES_NO_ROUTE, 'fallback_speed_cells', callback);}.bind(this));
 };

 const durationsParse = function(v) { return isNaN(parseInt(v)); };
 const distancesParse = function(v) { return isNaN(parseFloat(v)); };
+const estimatesParse = function(v) { return isNaN(parseFloat(v)); };

 function tableParse(table, noRoute, annotation, callback) {

-    const parse = annotation == 'distances' ? distancesParse : durationsParse;
+    const parse = annotation == 'distances' ? distancesParse : (annotation == 'durations' ? durationsParse : estimatesParse);
    const params = this.queryParams;
-    params.annotations = annotation == 'distances' ? 'distance' : 'duration';
+    params.annotations = ['durations','fallback_speed_cells'].indexOf(annotation) !== -1 ? 'duration' : 'distance';

    var tableRows = table.raw();

@@ -61,11 +64,26 @@ function tableParse(table, noRoute, annotation, callback) {

            var json = JSON.parse(response.body);

-            var result = json[annotation].map(row => {
-                var hashes = {};
-                row.forEach((v, i) => { hashes[tableRows[0][i+1]] = parse(v) ? '' : v; });
-                return hashes;
-            });
+            var result = {};
+            if (annotation === 'fallback_speed_cells') {
+                result = table.raw().map(row => row.map(() => ''));
+                json[annotation].forEach(pair => {
+                    result[pair[0]+1][pair[1]+1] = 'Y';
+                });
+                result = result.slice(1).map(row => {
+                    var hashes = {};
+                    row.slice(1).forEach((v,i) => {
+                        hashes[tableRows[0][i+1]] = v;
+                    });
+                    return hashes;
+                });
+            } else {
+                result = json[annotation].map(row => {
+                    var hashes = {};
+                    row.forEach((v, i) => { hashes[tableRows[0][i+1]] = parse(v) ? '' : v; });
+                    return hashes;
+                });
+            }

            var testRow = (row, ri, cb) => {
                for (var k in result[ri]) {
@@ -545,6 +545,24 @@ Feature: Basic Duration Matrix
            | f | 18 |
            | 1 | 30 |

+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+            | b |   |   | Y | Y |
+            | f | Y | Y |   |   |
+            | 1 | Y | Y |   |   |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a |
+            | a |   |
+            | b |   |
+            | f | Y |
+            | 1 | Y |
+
    Scenario: Testbot - Filling in noroutes with estimates - use input coordinate
        Given a grid size of 300 meters
        Given the extract extra arguments "--small-component-size 4"
@@ -580,6 +598,25 @@ Feature: Basic Duration Matrix
            | f | 18 |
            | 1 | 30 |

+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+            | b |   |   | Y | Y |
+            | f | Y | Y |   |   |
+            | 1 | Y | Y |   |   |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a |
+            | a |   |
+            | b |   |
+            | f | Y |
+            | 1 | Y |
+
+
    Scenario: Testbot - Filling in noroutes with estimates - use snapped coordinate
        Given a grid size of 300 meters
        Given the extract extra arguments "--small-component-size 4"
@@ -615,18 +652,35 @@ Feature: Basic Duration Matrix
            | f | 18 |
            | 1 | 24 |

+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+            | b |   |   | Y | Y |
+            | f | Y | Y |   |   |
+            | 1 | Y | Y |   |   |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a |
+            | a |   |
+            | b |   |
+            | f | Y |
+            | 1 | Y |

    Scenario: Testbot - Travel time matrix of minimal network with scale factor
-         Given the query options
+        Given the query options
            | scale_factor | 2 |
-         Given the node map
+        Given the node map
            """
            a b
            """
-         And the ways
+        And the ways
            | nodes |
            | ab    |
-         When I request a travel time matrix I should get
+        When I request a travel time matrix I should get
            |   | a  | b  |
            | a | 0  | 20 |
            | b | 20 | 0  |
@@ -638,67 +692,86 @@ Feature: Basic Duration Matrix
            | fallback_speed | 5 |
            | fallback_coordinate | snapped |

-         Given the node map
+        Given the node map
            """
            a b   f h 1
            d e   g i
            """

-         And the ways
+        And the ways
            | nodes |
            | abeda |
            | fhigf |

-         When I request a travel time matrix I should get
+        When I request a travel time matrix I should get
            |   | a  | b  | f  | 1  |
            | a | 0  | 60 | 36 | 48 |
            | b | 60 | 0  | 24 | 36 |
            | f | 36 | 24 | 0  | 60 |
            | 1 | 48 | 36 | 60 | 0  |

-         When I request a travel time matrix I should get
+        When I request a travel time matrix I should get
            |   | a  | b  | f  | 1  |
            | a | 0  | 60 | 36 | 48 |

-         When I request a travel time matrix I should get
+        When I request a travel time matrix I should get
            |   | a  |
            | a | 0  |
            | b | 60 |
            | f | 36 |
            | 1 | 48 |

+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+            | b |   |   | Y | Y |
+            | f | Y | Y |   |   |
+            | 1 | Y | Y |   |   |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a | b | f | 1 |
+            | a |   |   | Y | Y |
+
+        When I request a travel time matrix I should get estimates for
+            |   | a |
+            | a |   |
+            | b |   |
+            | f | Y |
+            | 1 | Y |
+
+
     Scenario: Testbot - Travel time matrix of minimal network with overflow scale factor
        Given the query options
            | scale_factor | 2147483647 |

-         Given the node map
+        Given the node map
            """
            a b
            """

-         And the ways
+        And the ways
            | nodes |
            | ab    |

-         When I request a travel time matrix I should get
+        When I request a travel time matrix I should get
            |   | a           | b           |
            | a | 0           | 214748364.6 |
            | b | 214748364.6 | 0           |

     Scenario: Testbot - Travel time matrix of minimal network with fraction scale factor
-         Given the query options
+        Given the query options
            | scale_factor | 0.5 |

-         Given the node map
+        Given the node map
            """
            a b
            """

-         And the ways
+        And the ways
            | nodes |
            | ab    |

-         When I request a travel time matrix I should get
+        When I request a travel time matrix I should get
            |   | a  | b  |
            | a | 0  | 5  |
            | b | 5  | 0  |
@@ -6,6 +6,7 @@

 #include "engine/api/json_factory.hpp"
 #include "engine/hint.hpp"
+#include "util/coordinate_calculation.hpp"

 #include <boost/assert.hpp>
 #include <boost/range/algorithm/transform.hpp>
@@ -53,6 +54,8 @@ class BaseAPI
            // TODO: check forward/reverse
            return json::makeWaypoint(
                phantom.location,
+                util::coordinate_calculation::fccApproximateDistance(phantom.location,
+                                                                     phantom.input_location),
                facade.GetNameForID(facade.GetNameIndex(phantom.forward_segment_id.id)).to_string(),
                Hint{phantom, facade.GetCheckSum()});
        }
@@ -61,6 +64,8 @@ class BaseAPI
            // TODO: check forward/reverse
            return json::makeWaypoint(
                phantom.location,
+                util::coordinate_calculation::fccApproximateDistance(phantom.location,
+                                                                     phantom.input_location),
                facade.GetNameForID(facade.GetNameIndex(phantom.forward_segment_id.id))
                    .to_string());
        }
@@ -33,7 +33,7 @@ namespace json
 namespace detail
 {

-util::json::Array coordinateToLonLat(const util::Coordinate coordinate);
+util::json::Array coordinateToLonLat(const util::Coordinate &coordinate);

 /**
 * Ensures that a bearing value is a whole number, and clamped to the range 0-359
@@ -86,11 +86,14 @@ util::json::Object makeRoute(const guidance::Route &route,
                             const char *weight_name);

 // Creates a Waypoint without Hint, see the Hint overload below
-util::json::Object makeWaypoint(const util::Coordinate location, std::string name);
+util::json::Object
+makeWaypoint(const util::Coordinate &location, const double &distance, std::string name);

 // Creates a Waypoint with Hint, see the overload above when Hint is not needed
-util::json::Object
-makeWaypoint(const util::Coordinate location, std::string name, const Hint &hint);
+util::json::Object makeWaypoint(const util::Coordinate &location,
+                                const double &distance,
+                                std::string name,
+                                const Hint &hint);

 util::json::Object makeRouteLeg(guidance::RouteLeg leg, util::json::Array steps);

@@ -41,7 +41,6 @@ class NearestAPI final : public BaseAPI
            [this](const PhantomNodeWithDistance &phantom_with_distance) {
                auto &phantom_node = phantom_with_distance.phantom_node;
                auto waypoint = MakeWaypoint(phantom_node);
-                waypoint.values["distance"] = phantom_with_distance.distance;

                util::json::Array nodes;

@@ -31,6 +31,15 @@ namespace api
 class TableAPI final : public BaseAPI
 {
  public:
+    struct TableCellRef
+    {
+        TableCellRef(const std::size_t &row, const std::size_t &column) : row{row}, column{column}
+        {
+        }
+        std::size_t row;
+        std::size_t column;
+    };
+
    TableAPI(const datafacade::BaseDataFacade &facade_, const TableParameters &parameters_)
        : BaseAPI(facade_, parameters_), parameters(parameters_)
    {
@@ -39,6 +48,7 @@ class TableAPI final : public BaseAPI
    virtual void
    MakeResponse(const std::pair<std::vector<EdgeDuration>, std::vector<EdgeDistance>> &tables,
                 const std::vector<PhantomNode> &phantoms,
+                 const std::vector<TableCellRef> &fallback_speed_cells,
                 util::json::Object &response) const
    {
        auto number_of_sources = parameters.sources.size();
@@ -77,6 +87,11 @@ class TableAPI final : public BaseAPI
                MakeDistanceTable(tables.second, number_of_sources, number_of_destinations);
        }

+        if (parameters.fallback_speed != INVALID_FALLBACK_SPEED && parameters.fallback_speed > 0)
+        {
+            response.values["fallback_speed_cells"] = MakeEstimatesTable(fallback_speed_cells);
+        }
+
        response.values["code"] = "Ok";
    }

@@ -163,6 +178,20 @@ class TableAPI final : public BaseAPI
        return json_table;
    }

+    virtual util::json::Array
+    MakeEstimatesTable(const std::vector<TableCellRef> &fallback_speed_cells) const
+    {
+        util::json::Array json_table;
+        std::for_each(
+            fallback_speed_cells.begin(), fallback_speed_cells.end(), [&](const auto &cell) {
+                util::json::Array row;
+                row.values.push_back(util::json::Number(cell.row));
+                row.values.push_back(util::json::Number(cell.column));
+                json_table.values.push_back(std::move(row));
+            });
+        return json_table;
+    }
+
    const TableParameters &parameters;
 };

@@ -1,6 +1,6 @@
 {
  "name": "osrm",
-  "version": "5.20.1-rc.3",
+  "version": "5.20.1-rc.4",
  "private": false,
  "description": "The Open Source Routing Machine is a high performance routing engine written in C++14 designed to run on OpenStreetMap data.",
  "dependencies": {
@@ -94,7 +94,7 @@ std::string waypointTypeToString(const guidance::WaypointType waypoint_type)
    return waypoint_type_names[static_cast<std::size_t>(waypoint_type)];
 }

-util::json::Array coordinateToLonLat(const util::Coordinate coordinate)
+util::json::Array coordinateToLonLat(const util::Coordinate &coordinate)
 {
    util::json::Array array;
    array.values.push_back(static_cast<double>(util::toFloating(coordinate.lon)));
@@ -240,17 +240,22 @@ util::json::Object makeRoute(const guidance::Route &route,
    return json_route;
 }

-util::json::Object makeWaypoint(const util::Coordinate location, std::string name)
+util::json::Object
+makeWaypoint(const util::Coordinate &location, const double &distance, std::string name)
 {
    util::json::Object waypoint;
    waypoint.values["location"] = detail::coordinateToLonLat(location);
    waypoint.values["name"] = std::move(name);
+    waypoint.values["distance"] = distance;
    return waypoint;
 }

-util::json::Object makeWaypoint(const util::Coordinate location, std::string name, const Hint &hint)
+util::json::Object makeWaypoint(const util::Coordinate &location,
+                                const double &distance,
+                                std::string name,
+                                const Hint &hint)
 {
-    auto waypoint = makeWaypoint(location, name);
+    auto waypoint = makeWaypoint(location, distance, name);
    waypoint.values["hint"] = hint.ToBase64();
    return waypoint;
 }
@@ -95,6 +95,8 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
        return Error("NoTable", "No table found", result);
    }

+    std::vector<api::TableAPI::TableCellRef> estimated_pairs;
+
    // Scan table for null results - if any exist, replace with distance estimates
    if (params.fallback_speed != INVALID_FALLBACK_SPEED || params.scale_factor != 1)
    {
@@ -127,6 +129,8 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
                    {
                        result_tables_pair.second[table_index] = distance_estimate;
                    }
+
+                    estimated_pairs.emplace_back(row, column);
                }
                if (params.scale_factor > 0 && params.scale_factor != 1 &&
                    result_tables_pair.first[table_index] != MAXIMAL_EDGE_DURATION &&
@@ -150,7 +154,7 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
    }

    api::TableAPI table_api{facade, params};
-    table_api.MakeResponse(result_tables_pair, snapped_phantoms, result);
+    table_api.MakeResponse(result_tables_pair, snapped_phantoms, estimated_pairs, result);

    return Status::Ok;
 }
@@ -153,7 +153,7 @@ double perpendicularDistance(const Coordinate segment_source,
                              web_mercator::fromWGS84(query_location));
    nearest_location = web_mercator::toWGS84(projected_nearest);

-    const double approximate_distance = greatCircleDistance(query_location, nearest_location);
+    const double approximate_distance = fccApproximateDistance(query_location, nearest_location);
    BOOST_ASSERT(0.0 <= approximate_distance);
    return approximate_distance;
 }
@@ -234,7 +234,7 @@ tables.forEach(function(annotation) {
    });

    test('table: ' + annotation + ' table in Monaco without motorways', function(assert) {
-        assert.plan(1);
+        assert.plan(2);
        var osrm = new OSRM({path: mld_data_path, algorithm: 'MLD'});
        var options = {
            coordinates: two_test_coordinates,
@@ -243,11 +243,12 @@ tables.forEach(function(annotation) {
        };
        osrm.table(options, function(err, response) {
            assert.equal(response[annotation].length, 2);
+            assert.strictEqual(response.fallback_speed_cells, undefined);
        });
    });

    test('table: ' + annotation + ' table in Monaco with fallback speeds', function(assert) {
-        assert.plan(1);
+        assert.plan(2);
        var osrm = new OSRM({path: mld_data_path, algorithm: 'MLD'});
        var options = {
            coordinates: two_test_coordinates,
@@ -257,6 +258,7 @@ tables.forEach(function(annotation) {
        };
        osrm.table(options, function(err, response) {
            assert.equal(response[annotation].length, 2);
+            assert.equal(response['fallback_speed_cells'].length, 0);
        });
    });

@@ -1,6 +1,8 @@
 #include <boost/test/test_case_template.hpp>
 #include <boost/test/unit_test.hpp>

+#include <cmath>
+
 #include "coordinates.hpp"
 #include "equal_json.hpp"
 #include "fixture.hpp"
@@ -32,18 +34,28 @@ BOOST_AUTO_TEST_CASE(test_route_same_coordinates_fixture)

    // unset snapping dependent hint
    for (auto &itr : result.values["waypoints"].get<json::Array>().values)
+    {
+        // Hint values aren't stable, so blank it out
        itr.get<json::Object>().values["hint"] = "";

+        // Round value to 6 decimal places for double comparison later
+        itr.get<json::Object>().values["distance"] =
+            round(itr.get<json::Object>().values["distance"].get<json::Number>().value * 1000000);
+    }
+
    const auto location = json::Array{{{7.437070}, {43.749248}}};

    json::Object reference{
        {{"code", "Ok"},
         {"waypoints",
-          json::Array{
-              {json::Object{
-                   {{"name", "Boulevard du Larvotto"}, {"location", location}, {"hint", ""}}},
-               json::Object{
-                   {{"name", "Boulevard du Larvotto"}, {"location", location}, {"hint", ""}}}}}},
+          json::Array{{json::Object{{{"name", "Boulevard du Larvotto"},
+                                     {"location", location},
+                                     {"distance", round(0.137249 * 1000000)},
+                                     {"hint", ""}}},
+                       json::Object{{{"name", "Boulevard du Larvotto"},
+                                     {"location", location},
+                                     {"distance", round(0.137249 * 1000000)},
+                                     {"hint", ""}}}}}},
         {"routes",
          json::Array{{json::Object{
              {{"distance", 0.},
Author	SHA1	Message	Date
Kajari Ghosh	fb343352bc	bump to rc 4	2018-12-11 13:20:18 -05:00
Daniel Patterson	4632ec3b67	Include information on estimates in table response (#5259 ) * Revert "Remove estimated_cells value in the response." This reverts commit `364e35af06`. * Update changelog. * fix linting * adjust fallback_speed check * change [].includes to [].indexOf !== -1 for compatibility with node 4 * change param name * more cuke tests * fix formatting	2018-12-11 13:17:34 -05:00
Daniel Patterson	7f9ac81028	Include distance from input to snapped for all waypoints, not just on the `nearest` service.	2018-12-11 13:16:44 -05:00