From 8989e15935d8dfe3233abd1dffc750e449eda42b Mon Sep 17 00:00:00 2001 From: Michael Bell Date: Tue, 8 Jun 2021 23:39:31 +0100 Subject: [PATCH] Add support for non-round-trips with a single fixed endpoint Currently /trip supports finding round-trip routes where only the start or end location is fixed. This PR extends this feature to non-round-trip requests. We do this by a new table manipulation that simulates non-round-trip fixed endpoint requests as a round-trip request. --- CHANGELOG.md | 2 + docs/http.md | 4 +- features/step_definitions/trip.js | 6 +- features/testbot/trip.feature | 82 ++++++++++++++++++--- features/testbot/zero-speed-updates.feature | 4 +- src/engine/plugins/trip.cpp | 73 ++++++++++++------ test/nodejs/trip.js | 55 +++++++------- unit_tests/library/trip.cpp | 8 +- 8 files changed, 164 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9ed1703ec7..ac1f89f873d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ - FIXED: Fixed Node docs generation check in CI. [#6058](https://github.com/Project-OSRM/osrm-backend/pull/6058) - CHANGED: Docker build, enabled arm64 build layer [#6172](https://github.com/Project-OSRM/osrm-backend/pull/6172) - CHANGED: Docker build, enabled apt-get update/install caching in separate layer for build phase [#6175](https://github.com/Project-OSRM/osrm-backend/pull/6175) + - Routing: + - ADDED: Add support for non-round-trips with a single fixed endpoint. [#6050](https://github.com/Project-OSRM/osrm-backend/pull/6050) # 5.26.0 - Changes from 5.25.0 diff --git a/docs/http.md b/docs/http.md index 60149856bc8..acdc1cd4f98 100644 --- a/docs/http.md +++ b/docs/http.md @@ -507,8 +507,8 @@ Right now, the following combinations are possible: | true | any | last | **yes** | | true | any | any | **yes** | | false | first | last | **yes** | -| false | first | any | no | -| false | any | last | no | +| false | first | any | **yes** | +| false | any | last | **yes** | | false | any | any | no | #### Example Requests diff --git a/features/step_definitions/trip.js b/features/step_definitions/trip.js index e7204401890..ab2f7c35ecf 100644 --- a/features/step_definitions/trip.js +++ b/features/step_definitions/trip.js @@ -61,7 +61,8 @@ module.exports = function () { var subTrips; var trip_durations; var trip_distance; - if (res.statusCode === 200) { + var ok = res.statusCode === 200; + if (ok) { if (headers.has('trips')) { subTrips = json.trips.filter(t => !!t).map(t => t.legs).map(tl => Array.prototype.concat.apply([], tl.map((sl, i) => { var toAdd = []; @@ -84,8 +85,7 @@ module.exports = function () { } } - var ok = true, - encodedResult = ''; + var encodedResult = ''; if (json.trips) row.trips.split(',').forEach((sub, si) => { if (si >= subTrips.length) { diff --git a/features/testbot/trip.feature b/features/testbot/trip.feature index 9fe3c23ae47..840aadb9cf2 100644 --- a/features/testbot/trip.feature +++ b/features/testbot/trip.feature @@ -5,7 +5,7 @@ Feature: Basic trip planning Given the profile "testbot" Given a grid size of 10 meters - Scenario: Testbot - Trip: Roundtrip with one waypoint + Scenario: Testbot - Trip: Roundtrip between same waypoint Given the node map """ a b @@ -21,7 +21,7 @@ Feature: Basic trip planning When I plan a trip I should get | waypoints | trips | - | a | aa | + | a,a | aa | Scenario: Testbot - Trip: Roundtrip with waypoints (less than 10) Given the node map @@ -69,7 +69,37 @@ Feature: Basic trip planning | waypoints | trips | | a,b,c,d,e,f,g,h,i,j,k,l | alkjihgfedcba | - Scenario: Testbot - Trip: Roundtrip FS waypoints (more than 10) + Scenario: Testbot - Trip: FS waypoints (less than 10) + Given the query options + | source | first | + Given the node map + """ + a b c d + l e + + j i g + """ + + And the ways + | nodes | + | ab | + | bc | + | de | + | eg | + | gi | + | ij | + | jl | + | la | + + When I plan a trip I should get + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,g,i,j,l | abcdegijla | true | 22 | + | a,b,c,d,e,g,i,j,l | abcljiged | false | 13 | + + + Scenario: Testbot - Trip: FS waypoints (more than 10) + Given the query options + | source | first | Given the node map """ a b c d @@ -93,12 +123,41 @@ Feature: Basic trip planning | la | When I plan a trip I should get - | waypoints | source | trips | - | a,b,c,d,e,f,g,h,i,j,k,l | first | alkjihgfedcba | + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,f,g,h,i,j,k,l | alkjihgfedcba | true | 22 | + | a,b,c,d,e,f,g,h,i,j,k,l | acblkjihgfed | false | 13 | - Scenario: Testbot - Trip: Roundtrip FE waypoints (more than 10) + + Scenario: Testbot - Trip: FE waypoints (less than 10) + Given the query options + | destination | last | + Given the node map + """ + a b c d + l e + + j i g + """ + + And the ways + | nodes | + | ab | + | bc | + | de | + | eg | + | gi | + | ij | + | jl | + | la | + + When I plan a trip I should get + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,g,i,j,l | labcdegijl | true | 22 | + | a,b,c,d,e,g,i,j,l | degijabcl | false | 14 | + + Scenario: Testbot - Trip: FE waypoints (more than 10) Given the query options - | source | last | + | destination | last | Given the node map """ a b c d @@ -122,8 +181,9 @@ Feature: Basic trip planning | la | When I plan a trip I should get - | waypoints | trips | - | a,b,c,d,e,f,g,h,i,j,k,l | lkjihgfedcbal | + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,f,g,h,i,j,k,l | lkjihgfedcbal | true | 22 | + | a,b,c,d,e,f,g,h,i,j,k,l | cbakjihgfedl | false | 19 | Scenario: Testbot - Trip: Unroutable roundtrip with waypoints (less than 10) Given the node map @@ -274,7 +334,7 @@ Feature: Basic trip planning | a,b,d,e,c | first | last | true | abedca | - Scenario: Testbot - Trip: midway points in isoldated roads should return no trips + Scenario: Testbot - Trip: midway points in isolated roads should return no trips Given the node map """ a 1 b @@ -370,4 +430,4 @@ Feature: Basic trip planning When I plan a trip I should get | waypoints | trips | durations | geometry | | a,b,c,d | abcda | 7.6 | 1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009,1,1 | - | d,b,c,a | dbcad | 7.6 | 0.99991,1.00009,1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009 | \ No newline at end of file + | d,b,c,a | dbcad | 7.6 | 0.99991,1.00009,1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009 | diff --git a/features/testbot/zero-speed-updates.feature b/features/testbot/zero-speed-updates.feature index c2bc82c94dd..ae5a99c7f78 100644 --- a/features/testbot/zero-speed-updates.feature +++ b/features/testbot/zero-speed-updates.feature @@ -187,5 +187,5 @@ Feature: Check zero speed updates When I plan a trip I should get | waypoints | trips | code | - | a,b,c,d | abcda | NoTrips | - | d,b,c,a | dbcad | NoTrips | + | a,b,c,d | | NoTrips | + | d,b,c,a | | NoTrips | diff --git a/src/engine/plugins/trip.cpp b/src/engine/plugins/trip.cpp index d949fcb6063..20daf95d917 100644 --- a/src/engine/plugins/trip.cpp +++ b/src/engine/plugins/trip.cpp @@ -36,18 +36,7 @@ bool IsSupportedParameterCombination(const bool fixed_start, const bool fixed_end, const bool roundtrip) { - if (fixed_start && fixed_end && !roundtrip) - { - return true; - } - else if (roundtrip) - { - return true; - } - else - { - return false; - } + return roundtrip || fixed_start || fixed_end; } // given the node order in which to visit, compute the actual route (with geometry, travel time and @@ -142,6 +131,32 @@ void ManipulateTableForFSE(const std::size_t source_id, //********* End of changes to table ************************************* } +void ManipulateTableForNonRoundtripFS(const std::size_t source_id, + util::DistTableWrapper &result_table) +{ + // We can use the round-trip calculation to simulate non-round-trip fixed start + // by making all paths to the source location zero. Effectively finding an 'optimal' + // round-trip path that ignores the cost of getting back from any destination to the + // source. + for (const auto i : util::irange(0, result_table.GetNumberOfNodes())) + { + result_table.SetValue(i, source_id, 0); + } +} + +void ManipulateTableForNonRoundtripFE(const std::size_t destination_id, + util::DistTableWrapper &result_table) +{ + // We can use the round-trip calculation to simulate non-round-trip fixed end + // by making all paths from the destination to other locations zero. + // Effectively, finding an 'optimal' round-trip path that ignores the cost of getting + // from the destination to any source. + for (const auto i : util::irange(0, result_table.GetNumberOfNodes())) + { + result_table.SetValue(destination_id, i, 0); + } +} + Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, const api::TripParameters ¶meters, osrm::engine::api::ResultT &result) const @@ -225,7 +240,7 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, return Status::Error; } - const constexpr std::size_t BF_MAX_FEASABLE = 10; + const constexpr std::size_t BF_MAX_FEASIBLE = 10; BOOST_ASSERT_MSG(result_duration_table.size() == number_of_locations * number_of_locations, "Distance Table has wrong size"); @@ -238,11 +253,19 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, { ManipulateTableForFSE(source_id, destination_id, result_duration_table); } + else if (!parameters.roundtrip && fixed_start) + { + ManipulateTableForNonRoundtripFS(source_id, result_duration_table); + } + else if (!parameters.roundtrip && fixed_end) + { + ManipulateTableForNonRoundtripFE(destination_id, result_duration_table); + } std::vector duration_trip; duration_trip.reserve(number_of_locations); // get an optimized order in which the destinations should be visited - if (number_of_locations < BF_MAX_FEASABLE) + if (number_of_locations < BF_MAX_FEASIBLE) { duration_trip = trip::BruteForceTrip(number_of_locations, result_duration_table); } @@ -251,20 +274,28 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, duration_trip = trip::FarthestInsertionTrip(number_of_locations, result_duration_table); } - // rotate result such that roundtrip starts at node with index 0 - // thist first if covers scenarios: !fixed_end || fixed_start || (fixed_start && fixed_end) if (!fixed_end || fixed_start) { + // rotate result such that trip starts at node with index 0 auto desired_start_index = std::find(std::begin(duration_trip), std::end(duration_trip), 0); BOOST_ASSERT(desired_start_index != std::end(duration_trip)); std::rotate(std::begin(duration_trip), desired_start_index, std::end(duration_trip)); } - else if (fixed_end && !fixed_start && parameters.roundtrip) - { - auto desired_start_index = + else + { // fixed_end + auto destination_index = std::find(std::begin(duration_trip), std::end(duration_trip), destination_id); - BOOST_ASSERT(desired_start_index != std::end(duration_trip)); - std::rotate(std::begin(duration_trip), desired_start_index, std::end(duration_trip)); + BOOST_ASSERT(destination_index != std::end(duration_trip)); + if (!parameters.roundtrip) + { + // We want the location after destination to be at the front + std::advance(destination_index, 1); + if (destination_index == std::end(duration_trip)) + { + destination_index = std::begin(duration_trip); + } + } + std::rotate(std::begin(duration_trip), destination_index, std::end(duration_trip)); } // get the route when visiting all destinations in optimized order diff --git a/test/nodejs/trip.js b/test/nodejs/trip.js index f26d97b83ca..683032b3c22 100644 --- a/test/nodejs/trip.js +++ b/test/nodejs/trip.js @@ -262,33 +262,19 @@ test('trip: routes Monaco with null hints', function(assert) { }); test('trip: service combinations that are not implemented', function(assert) { - assert.plan(3); + assert.plan(1); var osrm = new OSRM(data_path); - // fixed start, non-roundtrip + // no fixed start, no fixed end, non-roundtrip var options = { coordinates: two_test_coordinates, - source: 'first', + source: 'any', + destination: 'any', roundtrip: false }; osrm.trip(options, function(err, second) { assert.equal('NotImplemented', err.message); }); - - // fixed start, fixed end, non-roundtrip - options.source = 'any'; - options.destination = 'any'; - osrm.trip(options, function(err, second) { - assert.equal('NotImplemented', err.message); - }); - - // fixed end, non-roundtrip - delete options.source; - options.destination = 'last'; - osrm.trip(options, function(err, second) { - assert.equal('NotImplemented', err.message); - }); - }); test('trip: fixed start and end combinations', function(assert) { @@ -302,16 +288,17 @@ test('trip: fixed start and end combinations', function(assert) { geometries: 'geojson' }; - // fixed start and end, non-roundtrip - osrm.trip(options, function(err, fseTrip) { - assert.ifError(err); - assert.equal(1, fseTrip.trips.length); - var coordinates = fseTrip.trips[0].geometry.coordinates; - assert.notEqual(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1])); - }); + // variations of non roundtrip + var nonRoundtripChecks = function(options) { + osrm.trip(options, function(err, fseTrip) { + assert.ifError(err); + assert.equal(1, fseTrip.trips.length); + var coordinates = fseTrip.trips[0].geometry.coordinates; + assert.notEqual(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1])); + }); + }; // variations of roundtrip - var roundtripChecks = function(options) { osrm.trip(options, function(err, trip) { assert.ifError(err); @@ -319,7 +306,20 @@ test('trip: fixed start and end combinations', function(assert) { var coordinates = trip.trips[0].geometry.coordinates; assert.equal(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1])); }); - } + }; + + // fixed start and end, non-roundtrip + nonRoundtripChecks(options); + + // fixed start, non-roundtrip + delete options.destination; + options.source = 'first'; + nonRoundtripChecks(options); + + // fixed end, non-roundtrip + delete options.source; + options.destination = 'last'; + nonRoundtripChecks(options); // roundtrip, source and destination not specified roundtripChecks({coordinates: options.coordinates, geometries: options.geometries}); @@ -327,6 +327,7 @@ test('trip: fixed start and end combinations', function(assert) { // roundtrip, fixed destination options.roundtrip = true; delete options.source; + options.destination = 'last'; roundtripChecks(options); //roundtrip, fixed source diff --git a/unit_tests/library/trip.cpp b/unit_tests/library/trip.cpp index 6eb500e8160..52308f2e0f0 100644 --- a/unit_tests/library/trip.cpp +++ b/unit_tests/library/trip.cpp @@ -362,7 +362,7 @@ void test_tfse_illegal_parameters(bool use_json_only_api) ResetParams(locations, params); params.source = TripParameters::SourceType::First; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); ResetParams(locations, params); params.destination = TripParameters::DestinationType::Any; @@ -372,7 +372,7 @@ void test_tfse_illegal_parameters(bool use_json_only_api) ResetParams(locations, params); params.destination = TripParameters::DestinationType::Last; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); // three parameters set params.source = TripParameters::SourceType::Any; @@ -383,12 +383,12 @@ void test_tfse_illegal_parameters(bool use_json_only_api) params.source = TripParameters::SourceType::Any; params.destination = TripParameters::DestinationType::Last; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); params.source = TripParameters::SourceType::First; params.destination = TripParameters::DestinationType::Any; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); } BOOST_AUTO_TEST_CASE(test_tfse_illegal_parameters_old_api) { test_tfse_illegal_parameters(true); } BOOST_AUTO_TEST_CASE(test_tfse_illegal_parameters_new_api) { test_tfse_illegal_parameters(false); }