Skip to content

Commit

Permalink
Include information on estimates in table response (#5259)
Browse files Browse the repository at this point in the history
* Revert "Remove estimated_cells value in the response."

This reverts commit 364e35a.

* 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
  • Loading branch information
danpat authored and ghoshkaj committed Dec 11, 2018
1 parent 92d3ce7 commit 06e010b
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
- 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)
Expand Down
5 changes: 5 additions & 0 deletions docs/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,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:

Expand Down Expand Up @@ -384,6 +385,10 @@ All other properties might be undefined.
2361.73,
0
]
],
"fallback_speed_cells": [
[ 0, 1 ],
[ 1, 0 ]
]
}
```
Expand Down
1 change: 1 addition & 0 deletions docs/nodejs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
32 changes: 25 additions & 7 deletions features/step_definitions/distance_matrix.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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]) {
Expand Down
105 changes: 89 additions & 16 deletions features/testbot/duration_matrix.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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 |
Expand All @@ -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 |
29 changes: 29 additions & 0 deletions include/engine/api/table_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_)
{
Expand All @@ -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();
Expand Down Expand Up @@ -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";
}

Expand Down Expand Up @@ -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;
};

Expand Down
6 changes: 5 additions & 1 deletion src/engine/plugins/table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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 &&
Expand All @@ -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;
}
Expand Down
6 changes: 4 additions & 2 deletions test/nodejs/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
});
});

Expand Down

0 comments on commit 06e010b

Please sign in to comment.