From ffc39b8ad236ef36b7448ac5a7af660c1fc7c330 Mon Sep 17 00:00:00 2001 From: Michael Bell Date: Fri, 10 May 2024 22:00:24 +0100 Subject: [PATCH 1/9] Clarify use of forcing routing steps (#6866) The change clarifies the conditions for forcing routing steps and simplifies the codebase to support it. - Makes explicity the search runtime condition for forcing a routing step. Namely, the node is a source of the forward and reverse searches, and it's one of the pre-identified nodes that requires a step to be forced. - Consolidate the two lists of force nodes into one. Not only is there no algorithmic value in separating the nodes by geometric direction, the improvements to via-routes with u-turns mean atleast one of these lists will be empty for any search. - Rename 'force loop' to 'force step'. This moves the code away from the original CH-specific language for checking for self-loops in the case where this condition is met. MLD does not have loops. Additional cucumber tests are added to cover the logic related to negative search weights and forcing routing steps on via-route paths. --- CHANGELOG.md | 1 + features/testbot/force_step.feature | 101 ++++++++++++++++++ .../routing_algorithms/routing_base.hpp | 23 ++-- .../routing_algorithms/routing_base_ch.hpp | 28 +++-- .../routing_algorithms/routing_base_mld.hpp | 79 ++++---------- .../routing_algorithms/shortest_path_impl.hpp | 7 +- .../alternative_path_ch.cpp | 8 +- .../alternative_path_mld.cpp | 3 - .../direct_shortest_path.cpp | 2 - .../routing_algorithms/routing_base.cpp | 28 +++-- .../routing_algorithms/routing_base_ch.cpp | 20 ++-- unit_tests/engine/offline_facade.cpp | 6 +- 12 files changed, 180 insertions(+), 126 deletions(-) create mode 100644 features/testbot/force_step.feature diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f0fa3949..56e2da05e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ - FIXED: Correctly handle compressed traffic signals. [#6724](https://github.com/Project-OSRM/osrm-backend/pull/6724) - FIXED: Fix bug when searching for maneuver overrides [#6739](https://github.com/Project-OSRM/osrm-backend/pull/6739) - FIXED: Remove force-loop checks for routes with u-turns [#6858](https://github.com/Project-OSRM/osrm-backend/pull/6858) + - FIXED: Correctly check runtime search conditions for forcing routing steps [#6866](https://github.com/Project-OSRM/osrm-backend/pull/6866) - Debug tiles: - FIXED: Ensure speed layer features have unique ids. [#6726](https://github.com/Project-OSRM/osrm-backend/pull/6726) diff --git a/features/testbot/force_step.feature b/features/testbot/force_step.feature new file mode 100644 index 000000000..8ad220df1 --- /dev/null +++ b/features/testbot/force_step.feature @@ -0,0 +1,101 @@ +@routing @testbot @via +Feature: Force routing steps + Background: + Given the profile "testbot" + + Scenario: Direct routes with waypoints on same edge + Given the node map + """ + 1 2 + a-------b + | | + d-------c + | | + e-------f + 3 4 + """ + + And the ways + | nodes | oneway | + | ab | no | + | ad | no | + | bc | no | + | cf | no | + | dc | no | + | de | no | + | ef | yes | + + When I route I should get + | waypoints | approaches | weight | route | + | 1,2 | | 20 | ab,ab | + | 1,2 | curb curb | 100 | ab,ad,dc,bc,ab | + | 2,1 | | 20 | ab,ab | + | 2,1 | opposite opposite | 100 | ab,bc,dc,ad,ab | + | 3,4 | | 20 | ef,ef | + | 4,3 | | 100 | ef,cf,dc,de,ef | + + Scenario: Via routes with waypoints on same edge + Given the node map + """ + 1 2 + a-------b + | | + d-5-----c + | | + e-------f + 3 4 + """ + + And the ways + | nodes | oneway | + | ab | no | + | ad | no | + | bc | no | + | cf | no | + | dc | no | + | de | no | + | ef | yes | + + When I route I should get + | waypoints | approaches | weight | route | + | 5,1,2 | | 59.8 | dc,ad,ab,ab,ab | + | 5,1,2 | unrestricted curb curb | 180.2 | dc,bc,ab,ab,ab,ad,dc,bc,ab | + | 5,2,1 | | 80.2 | dc,bc,ab,ab,ab | + | 5,2,1 | unrestricted opposite opposite | 159.8 | dc,ad,ab,ab,ab,bc,dc,ad,ab | + | 5,3,4 | | 59.8 | dc,de,ef,ef,ef | + | 5,4,3 | | 159.8 | dc,de,ef,ef,ef,cf,dc,de,ef | + + + Scenario: [U-turns allowed] Via routes with waypoints on same edge + Given the node map + """ + 1 2 + a-------b + | | + d-5-----c + | | + e-------f + 3 4 + """ + + And the ways + | nodes | oneway | + | ab | no | + | ad | no | + | bc | no | + | cf | no | + | dc | no | + | de | no | + | ef | yes | + + And the query options + | continue_straight | false | + + When I route I should get + | waypoints | approaches | weight | route | + | 5,1,2 | | 59.8 | dc,ad,ab,ab,ab | + | 5,1,2 | unrestricted curb curb | 180.2 | dc,bc,ab,ab,ab,ad,dc,bc,ab | + | 5,2,1 | | 79.8 | dc,ad,ab,ab,ab,ab | + | 5,2,1 | unrestricted opposite opposite | 159.8 | dc,ad,ab,ab,ab,bc,dc,ad,ab | + | 5,3,4 | | 59.8 | dc,de,ef,ef,ef | + | 5,4,3 | | 159.8 | dc,de,ef,ef,ef,cf,dc,de,ef | diff --git a/include/engine/routing_algorithms/routing_base.hpp b/include/engine/routing_algorithms/routing_base.hpp index 15c5ea55a..0c61c1c37 100644 --- a/include/engine/routing_algorithms/routing_base.hpp +++ b/include/engine/routing_algorithms/routing_base.hpp @@ -71,24 +71,27 @@ void insertTargetInReverseHeap(Heap &reverse_heap, const PhantomNode &target) static constexpr bool FORWARD_DIRECTION = true; static constexpr bool REVERSE_DIRECTION = false; -// Identify nodes in the forward(reverse) search direction that will require loop forcing +// Identify nodes in the forward(reverse) search direction that will require step forcing // e.g. if source and destination nodes are on the same segment. -std::vector getForwardLoopNodes(const PhantomEndpointCandidates &candidates); -std::vector getForwardLoopNodes(const PhantomCandidatesToTarget &candidates); -std::vector getBackwardLoopNodes(const PhantomEndpointCandidates &candidates); -std::vector getBackwardLoopNodes(const PhantomCandidatesToTarget &candidates); +std::vector getForwardForceNodes(const PhantomEndpointCandidates &candidates); +std::vector getForwardForceNodes(const PhantomCandidatesToTarget &candidates); +std::vector getBackwardForceNodes(const PhantomEndpointCandidates &candidates); +std::vector getBackwardForceNodes(const PhantomCandidatesToTarget &candidates); // Find the specific phantom node endpoints for a given path from a list of candidates. PhantomEndpoints endpointsFromCandidates(const PhantomEndpointCandidates &candidates, const std::vector &path); template -inline bool force_loop(const std::vector &force_nodes, const HeapNodeT &heap_node) +inline bool shouldForceStep(const std::vector &force_nodes, + const HeapNodeT &forward_heap_node, + const HeapNodeT &reverse_heap_node) { - // if loops are forced, they are so at the source - return !force_nodes.empty() && - std::find(force_nodes.begin(), force_nodes.end(), heap_node.node) != force_nodes.end() && - heap_node.data.parent == heap_node.node; + // routing steps are forced when the node is a source of both forward and reverse search heaps. + return forward_heap_node.data.parent == forward_heap_node.node && + reverse_heap_node.data.parent == reverse_heap_node.node && + std::find(force_nodes.begin(), force_nodes.end(), forward_heap_node.node) != + force_nodes.end(); } template diff --git a/include/engine/routing_algorithms/routing_base_ch.hpp b/include/engine/routing_algorithms/routing_base_ch.hpp index 2714e025b..b3da739de 100644 --- a/include/engine/routing_algorithms/routing_base_ch.hpp +++ b/include/engine/routing_algorithms/routing_base_ch.hpp @@ -112,8 +112,7 @@ void routingStep(const DataFacade &facade, NodeID &middle_node_id, EdgeWeight &upper_bound, EdgeWeight min_edge_offset, - const std::vector &force_loop_forward_nodes, - const std::vector &force_loop_reverse_nodes) + const std::vector &force_step_nodes) { auto heapNode = forward_heap.DeleteMinGetHeapNode(); const auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node); @@ -123,13 +122,13 @@ void routingStep(const DataFacade &facade, const EdgeWeight new_weight = reverseHeapNode->weight + heapNode.weight; if (new_weight < upper_bound) { - if (force_loop(force_loop_forward_nodes, heapNode) || - force_loop(force_loop_reverse_nodes, heapNode) || + if (shouldForceStep(force_step_nodes, heapNode, reverseHeapNode.get()) || // in this case we are looking at a bi-directional way where the source // and target phantom are on the same edge based node new_weight < EdgeWeight{0}) { - // check whether there is a loop present at the node + // Before forcing step, check whether there is a loop present at the node. + // We may find a valid weight path by following the loop. for (const auto edge : facade.GetAdjacentEdgeRange(heapNode.node)) { const auto &data = facade.GetEdgeData(edge); @@ -421,23 +420,22 @@ void retrievePackedPathFromSingleManyToManyHeap( // assumes that heaps are already setup correctly. // ATTENTION: This only works if no additional offset is supplied next to the Phantom Node // Offsets. -// In case additional offsets are supplied, you might have to force a loop first. -// A forced loop might be necessary, if source and target are on the same segment. +// In case additional offsets are supplied, you might have to force a routing step first. +// A forced step might be necessary, if source and target are on the same segment. // If this is the case and the offsets of the respective direction are larger for the source // than the target -// then a force loop is required (e.g. source_phantom.forward_segment_id == +// then a force step is required (e.g. source_phantom.forward_segment_id == // target_phantom.forward_segment_id // && source_phantom.GetForwardWeightPlusOffset() > target_phantom.GetForwardWeightPlusOffset()) // requires -// a force loop, if the heaps have been initialized with positive offsets. +// a force step, if the heaps have been initialized with positive offsets. void search(SearchEngineData &engine_working_data, const DataFacade &facade, SearchEngineData::QueryHeap &forward_heap, SearchEngineData::QueryHeap &reverse_heap, EdgeWeight &weight, std::vector &packed_leg, - const std::vector &force_loop_forward_node, - const std::vector &force_loop_reverse_node, + const std::vector &force_step_nodes, const EdgeWeight duration_upper_bound = INVALID_EDGE_WEIGHT); template @@ -447,8 +445,7 @@ void search(SearchEngineData &engine_working_data, SearchEngineData::QueryHeap &reverse_heap, EdgeWeight &weight, std::vector &packed_leg, - const std::vector &force_loop_forward_node, - const std::vector &force_loop_reverse_node, + const std::vector &force_step_nodes, const PhantomEndpointT & /*endpoints*/, const EdgeWeight duration_upper_bound = INVALID_EDGE_WEIGHT) { @@ -459,14 +456,13 @@ void search(SearchEngineData &engine_working_data, reverse_heap, weight, packed_leg, - force_loop_forward_node, - force_loop_reverse_node, + force_step_nodes, duration_upper_bound); } // Requires the heaps for be empty // If heaps should be adjusted to be initialized outside of this function, -// the addition of force_loop parameters might be required +// the addition of force_step parameters might be required double getNetworkDistance(SearchEngineData &engine_working_data, const DataFacade &facade, SearchEngineData::QueryHeap &forward_heap, diff --git a/include/engine/routing_algorithms/routing_base_mld.hpp b/include/engine/routing_algorithms/routing_base_mld.hpp index c577c3be3..330983626 100644 --- a/include/engine/routing_algorithms/routing_base_mld.hpp +++ b/include/engine/routing_algorithms/routing_base_mld.hpp @@ -389,8 +389,7 @@ void routingStep(const DataFacade &facade, typename SearchEngineData::QueryHeap &reverse_heap, NodeID &middle_node, EdgeWeight &path_upper_bound, - const std::vector &force_loop_forward_nodes, - const std::vector &force_loop_reverse_nodes, + const std::vector &force_step_nodes, const Args &...args) { const auto heapNode = forward_heap.DeleteMinGetHeapNode(); @@ -409,11 +408,8 @@ void routingStep(const DataFacade &facade, auto reverse_weight = reverseHeapNode->weight; auto path_weight = weight + reverse_weight; - // MLD uses loops forcing only to prune single node paths in forward and/or - // backward direction (there is no need to force loops in MLD but in CH) - if (!force_loop(force_loop_forward_nodes, heapNode) && - !force_loop(force_loop_reverse_nodes, heapNode) && (path_weight >= EdgeWeight{0}) && - (path_weight < path_upper_bound)) + if (!shouldForceStep(force_step_nodes, heapNode, reverseHeapNode.get()) && + (path_weight >= EdgeWeight{0}) && (path_weight < path_upper_bound)) { middle_node = heapNode.node; path_upper_bound = path_weight; @@ -438,8 +434,7 @@ UnpackedPath search(SearchEngineData &engine_working_data, const DataFacade &facade, typename SearchEngineData::QueryHeap &forward_heap, typename SearchEngineData::QueryHeap &reverse_heap, - const std::vector &force_loop_forward_nodes, - const std::vector &force_loop_reverse_nodes, + const std::vector &force_step_nodes, EdgeWeight weight_upper_bound, const Args &...args) { @@ -463,27 +458,15 @@ UnpackedPath search(SearchEngineData &engine_working_data, { if (!forward_heap.Empty()) { - routingStep(facade, - forward_heap, - reverse_heap, - middle, - weight, - force_loop_forward_nodes, - force_loop_reverse_nodes, - args...); + routingStep( + facade, forward_heap, reverse_heap, middle, weight, force_step_nodes, args...); if (!forward_heap.Empty()) forward_heap_min = forward_heap.MinKey(); } if (!reverse_heap.Empty()) { - routingStep(facade, - reverse_heap, - forward_heap, - middle, - weight, - force_loop_reverse_nodes, - force_loop_forward_nodes, - args...); + routingStep( + facade, reverse_heap, forward_heap, middle, weight, force_step_nodes, args...); if (!reverse_heap.Empty()) reverse_heap_min = reverse_heap.MinKey(); } @@ -512,9 +495,7 @@ UnpackedPath search(SearchEngineData &engine_working_data, for (auto const &packed_edge : packed_path) { - NodeID source, target; - bool overlay_edge; - std::tie(source, target, overlay_edge) = packed_edge; + auto [source, target, overlay_edge] = packed_edge; if (!overlay_edge) { // a base graph edge unpacked_nodes.push_back(target); @@ -534,21 +515,14 @@ UnpackedPath search(SearchEngineData &engine_working_data, forward_heap.Insert(source, {0}, {source}); reverse_heap.Insert(target, {0}, {target}); - // TODO: when structured bindings will be allowed change to - // auto [subpath_weight, subpath_source, subpath_target, subpath] = ... - EdgeWeight subpath_weight; - std::vector subpath_nodes; - std::vector subpath_edges; - std::tie(subpath_weight, subpath_nodes, subpath_edges) = - search(engine_working_data, - facade, - forward_heap, - reverse_heap, - force_loop_forward_nodes, - force_loop_reverse_nodes, - INVALID_EDGE_WEIGHT, - sublevel, - parent_cell_id); + auto [subpath_weight, subpath_nodes, subpath_edges] = search(engine_working_data, + facade, + forward_heap, + reverse_heap, + force_step_nodes, + INVALID_EDGE_WEIGHT, + sublevel, + parent_cell_id); BOOST_ASSERT(!subpath_edges.empty()); BOOST_ASSERT(subpath_nodes.size() > 1); BOOST_ASSERT(subpath_nodes.front() == source); @@ -570,8 +544,7 @@ inline void search(SearchEngineData &engine_working_data, typename SearchEngineData::QueryHeap &reverse_heap, EdgeWeight &weight, std::vector &unpacked_nodes, - const std::vector &force_loop_forward_node, - const std::vector &force_loop_reverse_node, + const std::vector &force_step_nodes, const PhantomEndpointT &endpoints, const EdgeWeight weight_upper_bound = INVALID_EDGE_WEIGHT) { @@ -580,8 +553,7 @@ inline void search(SearchEngineData &engine_working_data, facade, forward_heap, reverse_heap, - force_loop_forward_node, - force_loop_reverse_node, + force_step_nodes, weight_upper_bound, endpoints); } @@ -633,17 +605,8 @@ double getNetworkDistance(SearchEngineData &engine_working_data, const PhantomEndpoints endpoints{source_phantom, target_phantom}; insertNodesInHeaps(forward_heap, reverse_heap, endpoints); - EdgeWeight weight = INVALID_EDGE_WEIGHT; - std::vector unpacked_nodes; - std::vector unpacked_edges; - std::tie(weight, unpacked_nodes, unpacked_edges) = search(engine_working_data, - facade, - forward_heap, - reverse_heap, - {}, - {}, - weight_upper_bound, - endpoints); + auto [weight, unpacked_nodes, unpacked_edges] = search( + engine_working_data, facade, forward_heap, reverse_heap, {}, weight_upper_bound, endpoints); if (weight == INVALID_EDGE_WEIGHT) { diff --git a/include/engine/routing_algorithms/shortest_path_impl.hpp b/include/engine/routing_algorithms/shortest_path_impl.hpp index c21295236..b9e74fd02 100644 --- a/include/engine/routing_algorithms/shortest_path_impl.hpp +++ b/include/engine/routing_algorithms/shortest_path_impl.hpp @@ -64,7 +64,6 @@ void searchWithUTurn(SearchEngineData &engine_working_data, leg_weight, leg_packed_path, {}, - {}, candidates); } @@ -124,8 +123,7 @@ void search(SearchEngineData &engine_working_data, reverse_heap, new_total_weight_to_forward, leg_packed_path_forward, - getForwardLoopNodes(candidates), - {}, + getForwardForceNodes(candidates), candidates); } @@ -164,8 +162,7 @@ void search(SearchEngineData &engine_working_data, reverse_heap, new_total_weight_to_reverse, leg_packed_path_reverse, - {}, - getBackwardLoopNodes(candidates), + getBackwardForceNodes(candidates), candidates); } } diff --git a/src/engine/routing_algorithms/alternative_path_ch.cpp b/src/engine/routing_algorithms/alternative_path_ch.cpp index 10ccfe28c..4110eb714 100644 --- a/src/engine/routing_algorithms/alternative_path_ch.cpp +++ b/src/engine/routing_algorithms/alternative_path_ch.cpp @@ -187,7 +187,6 @@ void computeWeightAndSharingOfViaPath(SearchEngineData &engine_workin s_v_middle, upper_bound_s_v_path_weight, min_edge_offset, - {}, {}); } // compute path by reusing backward search from node t @@ -202,7 +201,6 @@ void computeWeightAndSharingOfViaPath(SearchEngineData &engine_workin v_t_middle, upper_bound_of_v_t_path_weight, min_edge_offset, - {}, {}); } *real_weight_of_via_path = upper_bound_s_v_path_weight + upper_bound_of_v_t_path_weight; @@ -348,7 +346,6 @@ bool viaNodeCandidatePassesTTest(SearchEngineData &engine_working_dat *s_v_middle, upper_bound_s_v_path_weight, min_edge_offset, - {}, {}); } @@ -369,7 +366,6 @@ bool viaNodeCandidatePassesTTest(SearchEngineData &engine_working_dat *v_t_middle, upper_bound_of_v_t_path_weight, min_edge_offset, - {}, {}); } @@ -538,12 +534,12 @@ bool viaNodeCandidatePassesTTest(SearchEngineData &engine_working_dat if (!forward_heap3.Empty()) { routingStep( - facade, forward_heap3, reverse_heap3, middle, upper_bound, min_edge_offset, {}, {}); + facade, forward_heap3, reverse_heap3, middle, upper_bound, min_edge_offset, {}); } if (!reverse_heap3.Empty()) { routingStep( - facade, reverse_heap3, forward_heap3, middle, upper_bound, min_edge_offset, {}, {}); + facade, reverse_heap3, forward_heap3, middle, upper_bound, min_edge_offset, {}); } } return (upper_bound <= t_test_path_weight); diff --git a/src/engine/routing_algorithms/alternative_path_mld.cpp b/src/engine/routing_algorithms/alternative_path_mld.cpp index 945aa15fe..25d0d94e9 100644 --- a/src/engine/routing_algorithms/alternative_path_mld.cpp +++ b/src/engine/routing_algorithms/alternative_path_mld.cpp @@ -631,7 +631,6 @@ void unpackPackedPaths(InputIt first, forward_heap, reverse_heap, {}, - {}, INVALID_EDGE_WEIGHT, sublevel, parent_cell_id); @@ -720,7 +719,6 @@ makeCandidateVias(SearchEngineData &search_engine_data, overlap_via, overlap_weight, {}, - {}, endpoint_candidates); if (!forward_heap.Empty()) @@ -746,7 +744,6 @@ makeCandidateVias(SearchEngineData &search_engine_data, overlap_via, overlap_weight, {}, - {}, endpoint_candidates); if (!reverse_heap.Empty()) diff --git a/src/engine/routing_algorithms/direct_shortest_path.cpp b/src/engine/routing_algorithms/direct_shortest_path.cpp index 923c6e4f7..65e924cf5 100644 --- a/src/engine/routing_algorithms/direct_shortest_path.cpp +++ b/src/engine/routing_algorithms/direct_shortest_path.cpp @@ -34,7 +34,6 @@ InternalRouteResult directShortestPathSearch(SearchEngineData &en weight, packed_leg, {}, - {}, endpoint_candidates); std::vector unpacked_nodes; @@ -81,7 +80,6 @@ InternalRouteResult directShortestPathSearch(SearchEngineData &e forward_heap, reverse_heap, {}, - {}, INVALID_EDGE_WEIGHT, endpoint_candidates); diff --git a/src/engine/routing_algorithms/routing_base.cpp b/src/engine/routing_algorithms/routing_base.cpp index 29dec6bd4..28fad23e8 100644 --- a/src/engine/routing_algorithms/routing_base.cpp +++ b/src/engine/routing_algorithms/routing_base.cpp @@ -3,21 +3,29 @@ namespace osrm::engine::routing_algorithms { -bool requiresForwardLoop(const PhantomNode &source, const PhantomNode &target) +bool requiresForwardForce(const PhantomNode &source, const PhantomNode &target) { + // Conditions to force a routing step: + // - Valid source and target. + // - Source and target on same segment. + // - Source is "downstream" of target in the direction of the edge. return source.IsValidForwardSource() && target.IsValidForwardTarget() && source.forward_segment_id.id == target.forward_segment_id.id && source.GetForwardWeightPlusOffset() > target.GetForwardWeightPlusOffset(); } -bool requiresBackwardLoop(const PhantomNode &source, const PhantomNode &target) +bool requiresBackwardForce(const PhantomNode &source, const PhantomNode &target) { + // Conditions to force a routing step: + // - Valid source and target. + // - Source and target on same segment. + // - Source is "downstream" of target in the direction of the edge. return source.IsValidReverseSource() && target.IsValidReverseTarget() && source.reverse_segment_id.id == target.reverse_segment_id.id && source.GetReverseWeightPlusOffset() > target.GetReverseWeightPlusOffset(); } -std::vector getForwardLoopNodes(const PhantomEndpointCandidates &endpoint_candidates) +std::vector getForwardForceNodes(const PhantomEndpointCandidates &endpoint_candidates) { std::vector res; for (const auto &source_phantom : endpoint_candidates.source_phantoms) @@ -26,7 +34,7 @@ std::vector getForwardLoopNodes(const PhantomEndpointCandidates &endpoin std::any_of(endpoint_candidates.target_phantoms.begin(), endpoint_candidates.target_phantoms.end(), [&](const auto &target_phantom) - { return requiresForwardLoop(source_phantom, target_phantom); }); + { return requiresForwardForce(source_phantom, target_phantom); }); if (requires_loop) { res.push_back(source_phantom.forward_segment_id.id); @@ -35,12 +43,12 @@ std::vector getForwardLoopNodes(const PhantomEndpointCandidates &endpoin return res; } -std::vector getForwardLoopNodes(const PhantomCandidatesToTarget &endpoint_candidates) +std::vector getForwardForceNodes(const PhantomCandidatesToTarget &endpoint_candidates) { std::vector res; for (const auto &source_phantom : endpoint_candidates.source_phantoms) { - if (requiresForwardLoop(source_phantom, endpoint_candidates.target_phantom)) + if (requiresForwardForce(source_phantom, endpoint_candidates.target_phantom)) { res.push_back(source_phantom.forward_segment_id.id); } @@ -48,7 +56,7 @@ std::vector getForwardLoopNodes(const PhantomCandidatesToTarget &endpoin return res; } -std::vector getBackwardLoopNodes(const PhantomEndpointCandidates &endpoint_candidates) +std::vector getBackwardForceNodes(const PhantomEndpointCandidates &endpoint_candidates) { std::vector res; for (const auto &source_phantom : endpoint_candidates.source_phantoms) @@ -57,7 +65,7 @@ std::vector getBackwardLoopNodes(const PhantomEndpointCandidates &endpoi std::any_of(endpoint_candidates.target_phantoms.begin(), endpoint_candidates.target_phantoms.end(), [&](const auto &target_phantom) - { return requiresBackwardLoop(source_phantom, target_phantom); }); + { return requiresBackwardForce(source_phantom, target_phantom); }); if (requires_loop) { res.push_back(source_phantom.reverse_segment_id.id); @@ -66,12 +74,12 @@ std::vector getBackwardLoopNodes(const PhantomEndpointCandidates &endpoi return res; } -std::vector getBackwardLoopNodes(const PhantomCandidatesToTarget &endpoint_candidates) +std::vector getBackwardForceNodes(const PhantomCandidatesToTarget &endpoint_candidates) { std::vector res; for (const auto &source_phantom : endpoint_candidates.source_phantoms) { - if (requiresBackwardLoop(source_phantom, endpoint_candidates.target_phantom)) + if (requiresBackwardForce(source_phantom, endpoint_candidates.target_phantom)) { res.push_back(source_phantom.reverse_segment_id.id); } diff --git a/src/engine/routing_algorithms/routing_base_ch.cpp b/src/engine/routing_algorithms/routing_base_ch.cpp index 304fce630..7bf006075 100644 --- a/src/engine/routing_algorithms/routing_base_ch.cpp +++ b/src/engine/routing_algorithms/routing_base_ch.cpp @@ -73,23 +73,22 @@ void retrievePackedPathFromSingleManyToManyHeap( // assumes that heaps are already setup correctly. // ATTENTION: This only works if no additional offset is supplied next to the Phantom Node // Offsets. -// In case additional offsets are supplied, you might have to force a loop first. -// A forced loop might be necessary, if source and target are on the same segment. +// In case additional offsets are supplied, you might have to force a routing step first. +// A forced step might be necessary, if source and target are on the same segment. // If this is the case and the offsets of the respective direction are larger for the source // than the target -// then a force loop is required (e.g. source_phantom.forward_segment_id == +// then a force step is required (e.g. source_phantom.forward_segment_id == // target_phantom.forward_segment_id // && source_phantom.GetForwardWeightPlusOffset() > target_phantom.GetForwardWeightPlusOffset()) // requires -// a force loop, if the heaps have been initialized with positive offsets. +// a force step, if the heaps have been initialized with positive offsets. void search(SearchEngineData & /*engine_working_data*/, const DataFacade &facade, SearchEngineData::QueryHeap &forward_heap, SearchEngineData::QueryHeap &reverse_heap, EdgeWeight &weight, std::vector &packed_leg, - const std::vector &force_loop_forward_nodes, - const std::vector &force_loop_reverse_nodes, + const std::vector &force_step_nodes, const EdgeWeight weight_upper_bound) { if (forward_heap.Empty() || reverse_heap.Empty()) @@ -118,8 +117,7 @@ void search(SearchEngineData & /*engine_working_data*/, middle, weight, min_edge_offset, - force_loop_forward_nodes, - force_loop_reverse_nodes); + force_step_nodes); } if (!reverse_heap.Empty()) { @@ -129,8 +127,7 @@ void search(SearchEngineData & /*engine_working_data*/, middle, weight, min_edge_offset, - force_loop_reverse_nodes, - force_loop_forward_nodes); + force_step_nodes); } } @@ -159,7 +156,7 @@ void search(SearchEngineData & /*engine_working_data*/, // Requires the heaps for be empty // If heaps should be adjusted to be initialized outside of this function, -// the addition of force_loop parameters might be required +// the addition of force_step parameters might be required double getNetworkDistance(SearchEngineData &engine_working_data, const DataFacade &facade, SearchEngineData::QueryHeap &forward_heap, @@ -183,7 +180,6 @@ double getNetworkDistance(SearchEngineData &engine_working_data, weight, packed_path, {}, - {}, endpoints, weight_upper_bound); diff --git a/unit_tests/engine/offline_facade.cpp b/unit_tests/engine/offline_facade.cpp index c09d4c0d6..822d02f4f 100644 --- a/unit_tests/engine/offline_facade.cpp +++ b/unit_tests/engine/offline_facade.cpp @@ -334,8 +334,7 @@ inline void search(SearchEngineData &engine_working_data, typename SearchEngineData::QueryHeap &reverse_heap, EdgeWeight &weight, std::vector &packed_leg, - const std::vector &forward_loop_nodes, - const std::vector &reverse_loop_nodes, + const std::vector &loop_nodes, const PhantomT &endpoints, const EdgeWeight weight_upper_bound = INVALID_EDGE_WEIGHT) { @@ -345,8 +344,7 @@ inline void search(SearchEngineData &engine_working_data, reverse_heap, weight, packed_leg, - forward_loop_nodes, - reverse_loop_nodes, + loop_nodes, endpoints, weight_upper_bound); } From 1e2ffee97c4fa875d17fa83e525f03706e8507f4 Mon Sep 17 00:00:00 2001 From: Dennis Luxen Date: Sat, 11 May 2024 09:13:17 +0200 Subject: [PATCH 2/9] Update Makefile: fix typo (#6878) --- test/data/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/data/Makefile b/test/data/Makefile index 87cd75b1d..14c586e93 100755 --- a/test/data/Makefile +++ b/test/data/Makefile @@ -33,7 +33,7 @@ mld/$(DATA_NAME).osrm: $(DATA_NAME).osrm cp $(DATA_NAME).osrm.* mld/ $(DATA_NAME).osrm: $(DATA_NAME).osm.pbf $(DATA_NAME).poly $(PROFILE) $(OSRM_EXTRACT) - @echo "Verifiyng data file integrity..." + @echo "Verifying data file integrity..." $(MD5SUM) -c data.md5sum @echo "Running osrm-extract..." $(TIMER) "osrm-extract\t$@" $(OSRM_EXTRACT) $< -p $(PROFILE) From ee8e0f890add1fb0037ca68fdda5b4c4025e4e71 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sat, 11 May 2024 16:30:43 +0200 Subject: [PATCH 3/9] Optimise path distance calculation in MLD map matching (#6876) --- .github/workflows/osrm-backend.yml | 3 +- CHANGELOG.md | 2 ++ .../routing_algorithms/routing_base_mld.hpp | 33 +++++++++++++++++-- src/benchmarks/match.cpp | 3 ++ 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/.github/workflows/osrm-backend.yml b/.github/workflows/osrm-backend.yml index 421a33ddc..e6dc8fca9 100644 --- a/.github/workflows/osrm-backend.yml +++ b/.github/workflows/osrm-backend.yml @@ -518,7 +518,8 @@ jobs: make --jobs=${JOBS} benchmarks ./src/benchmarks/alias-bench ./src/benchmarks/json-render-bench ../src/benchmarks/portugal_to_korea.json - ./src/benchmarks/match-bench ../test/data/ch/monaco.osrm + ./src/benchmarks/match-bench ../test/data/ch/monaco.osrm ch + ./src/benchmarks/match-bench ../test/data/mld/monaco.osrm mld ./src/benchmarks/packedvector-bench ./src/benchmarks/rtree-bench ../test/data/monaco.osrm.ramIndex ../test/data/monaco.osrm.fileIndex ../test/data/monaco.osrm.nbg_nodes popd diff --git a/CHANGELOG.md b/CHANGELOG.md index 56e2da05e..a736d3040 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ - FIXED: Fix bug when searching for maneuver overrides [#6739](https://github.com/Project-OSRM/osrm-backend/pull/6739) - FIXED: Remove force-loop checks for routes with u-turns [#6858](https://github.com/Project-OSRM/osrm-backend/pull/6858) - FIXED: Correctly check runtime search conditions for forcing routing steps [#6866](https://github.com/Project-OSRM/osrm-backend/pull/6866) + - Map Matching: + - CHANGED: Optimise path distance calculation in MLD map matching. [#6876](https://github.com/Project-OSRM/osrm-backend/pull/6876) - Debug tiles: - FIXED: Ensure speed layer features have unique ids. [#6726](https://github.com/Project-OSRM/osrm-backend/pull/6726) diff --git a/include/engine/routing_algorithms/routing_base_mld.hpp b/include/engine/routing_algorithms/routing_base_mld.hpp index 330983626..b85ce5496 100644 --- a/include/engine/routing_algorithms/routing_base_mld.hpp +++ b/include/engine/routing_algorithms/routing_base_mld.hpp @@ -613,11 +613,38 @@ double getNetworkDistance(SearchEngineData &engine_working_data, return std::numeric_limits::max(); } - std::vector unpacked_path; + BOOST_ASSERT(unpacked_nodes.size() >= 1); - annotatePath(facade, endpoints, unpacked_nodes, unpacked_edges, unpacked_path); + EdgeDistance distance = {0.0}; - return getPathDistance(facade, unpacked_path, source_phantom, target_phantom); + if (source_phantom.forward_segment_id.id == unpacked_nodes.front()) + { + BOOST_ASSERT(source_phantom.forward_segment_id.enabled); + distance = EdgeDistance{0} - source_phantom.GetForwardDistance(); + } + else if (source_phantom.reverse_segment_id.id == unpacked_nodes.front()) + { + BOOST_ASSERT(source_phantom.reverse_segment_id.enabled); + distance = EdgeDistance{0} - source_phantom.GetReverseDistance(); + } + + for (size_t index = 0; index < unpacked_nodes.size() - 1; ++index) + { + distance += facade.GetNodeDistance(unpacked_nodes[index]); + } + + if (target_phantom.forward_segment_id.id == unpacked_nodes.back()) + { + BOOST_ASSERT(target_phantom.forward_segment_id.enabled); + distance += target_phantom.GetForwardDistance(); + } + else if (target_phantom.reverse_segment_id.id == unpacked_nodes.back()) + { + BOOST_ASSERT(target_phantom.reverse_segment_id.enabled); + distance += target_phantom.GetReverseDistance(); + } + + return from_alias(distance); } } // namespace osrm::engine::routing_algorithms::mld diff --git a/src/benchmarks/match.cpp b/src/benchmarks/match.cpp index 932820e88..caed1a00b 100644 --- a/src/benchmarks/match.cpp +++ b/src/benchmarks/match.cpp @@ -1,3 +1,4 @@ +#include "engine/engine_config.hpp" #include "util/timing_util.hpp" #include "osrm/match_parameters.hpp" @@ -32,6 +33,8 @@ try // Configure based on a .osrm base path, and no datasets in shared mem from osrm-datastore EngineConfig config; config.storage_config = {argv[1]}; + config.algorithm = (argc > 2 && std::string{argv[2]} == "mld") ? EngineConfig::Algorithm::MLD + : EngineConfig::Algorithm::CH; config.use_shared_memory = false; // Routing machine with several services (such as Route, Table, Nearest, Trip, Match) From c00c1574791eb76b7510aa7f91e57c41098ee3e7 Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Tue, 14 May 2024 11:49:49 +0200 Subject: [PATCH 4/9] Replace dead link with archive.org link (#6882) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index afe4940c0..671af2478 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ The following services are available via HTTP API, C++ library interface and Nod To quickly try OSRM use our [demo server](http://map.project-osrm.org) which comes with both the backend and a frontend on top. -For a quick introduction about how the road network is represented in OpenStreetMap and how to map specific road network features have a look at [this guide about mapping for navigation](https://www.mapbox.com/mapping/mapping-for-navigation/). +For a quick introduction about how the road network is represented in OpenStreetMap and how to map specific road network features have a look at [the OSM wiki on routing](https://wiki.openstreetmap.org/wiki/Routing) or [this guide about mapping for navigation](https://web.archive.org/web/20221206013651/https://labs.mapbox.com/mapping/mapping-for-navigation/). Related [Project-OSRM](https://github.com/Project-OSRM) repositories: - [osrm-frontend](https://github.com/Project-OSRM/osrm-frontend) - User-facing frontend with map. The demo server runs this on top of the backend From 54e50a67a8ac40e2efd70752399c90ff5898b9ce Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Tue, 14 May 2024 17:14:59 +0200 Subject: [PATCH 5/9] Add benchmarks comparison job (#6880) --- .github/workflows/osrm-backend.yml | 81 ++++++++++++++++++----- scripts/ci/post_benchmark_results.py | 96 ++++++++++++++++++++++++++++ scripts/ci/run_benchmarks.sh | 24 +++++++ src/benchmarks/alias.cpp | 8 +-- src/benchmarks/packed_vector.cpp | 12 ++-- src/benchmarks/static_rtree.cpp | 17 ++--- 6 files changed, 201 insertions(+), 37 deletions(-) create mode 100644 scripts/ci/post_benchmark_results.py create mode 100755 scripts/ci/run_benchmarks.sh diff --git a/.github/workflows/osrm-backend.yml b/.github/workflows/osrm-backend.yml index e6dc8fca9..f1b28b310 100644 --- a/.github/workflows/osrm-backend.yml +++ b/.github/workflows/osrm-backend.yml @@ -253,7 +253,6 @@ jobs: BUILD_TYPE: Release CCOMPILER: gcc-13 CXXCOMPILER: g++-13 - ENABLE_BENCHMARKS: ON CXXFLAGS: '-Wno-array-bounds -Wno-uninitialized' - name: gcc-12-release @@ -264,7 +263,6 @@ jobs: BUILD_TYPE: Release CCOMPILER: gcc-12 CXXCOMPILER: g++-12 - ENABLE_BENCHMARKS: ON CXXFLAGS: '-Wno-array-bounds -Wno-uninitialized' - name: gcc-11-release @@ -275,7 +273,6 @@ jobs: BUILD_TYPE: Release CCOMPILER: gcc-11 CXXCOMPILER: g++-11 - ENABLE_BENCHMARKS: ON - name: conan-linux-release-node build_node_package: true @@ -511,18 +508,6 @@ jobs: fi popd npm test - - name: Run benchmarks - if: ${{ matrix.ENABLE_BENCHMARKS == 'ON' }} - run: | - pushd ${OSRM_BUILD_DIR} - make --jobs=${JOBS} benchmarks - ./src/benchmarks/alias-bench - ./src/benchmarks/json-render-bench ../src/benchmarks/portugal_to_korea.json - ./src/benchmarks/match-bench ../test/data/ch/monaco.osrm ch - ./src/benchmarks/match-bench ../test/data/mld/monaco.osrm mld - ./src/benchmarks/packedvector-bench - ./src/benchmarks/rtree-bench ../test/data/monaco.osrm.ramIndex ../test/data/monaco.osrm.fileIndex ../test/data/monaco.osrm.nbg_nodes - popd - name: Use Node 18 if: ${{ matrix.NODE_PACKAGE_TESTS_ONLY == 'ON' }} @@ -595,8 +580,72 @@ jobs: replacesArtifacts: true token: ${{ secrets.GITHUB_TOKEN }} + + benchmarks: + if: github.event_name == 'pull_request' + needs: [format-taginfo-docs] + runs-on: ubuntu-22.04 + env: + CCOMPILER: clang-13 + CXXCOMPILER: clang++-13 + CC: clang-13 + CXX: clang++-13 + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + GITHUB_REPOSITORY: ${{ github.repository }} + steps: + - name: Enable compiler cache + uses: actions/cache@v3 + with: + path: ~/.ccache + key: v1-ccache-benchmarks-${{ github.sha }} + restore-keys: | + v1-ccache-benchmarks- + - name: Enable Conan cache + uses: actions/cache@v3 + with: + path: ~/.conan + key: v1-conan-benchmarks-${{ github.sha }} + restore-keys: | + v1-conan-benchmarks- + - name: Checkout PR Branch + uses: actions/checkout@v3 + with: + ref: ${{ github.head_ref }} + path: pr + - run: python3 -m pip install "conan<2.0.0" "requests==2.31.0" + - name: Build PR Branch + run: | + mkdir -p pr/build + cd pr/build + cmake -DENABLE_CONAN=ON -DCMAKE_BUILD_TYPE=Release .. + make -j$(nproc) + make -j$(nproc) benchmarks + cd .. + make -C test/data + - name: Checkout Base Branch + uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.base.ref }} + path: base + - name: Build Base Branch + run: | + mkdir base/build + cd base/build + cmake -DENABLE_CONAN=ON -DCMAKE_BUILD_TYPE=Release .. + make -j$(nproc) + make -j$(nproc) benchmarks + cd .. + make -C test/data + - name: Run Benchmarks + run: | + ./pr/scripts/ci/run_benchmarks.sh base pr + - name: Post Benchmark Results + run: | + python3 pr/scripts/ci/post_benchmark_results.py base_results pr_results + ci-complete: runs-on: ubuntu-22.04 - needs: [build-test-publish, docker-image, windows-release-node] + needs: [build-test-publish, docker-image, windows-release-node, benchmarks] steps: - run: echo "CI complete" diff --git a/scripts/ci/post_benchmark_results.py b/scripts/ci/post_benchmark_results.py new file mode 100644 index 000000000..3efd5fcad --- /dev/null +++ b/scripts/ci/post_benchmark_results.py @@ -0,0 +1,96 @@ +import requests +import os +import re +import sys +import json + +GITHUB_TOKEN = os.getenv('GITHUB_TOKEN') +REPO = os.getenv('GITHUB_REPOSITORY') +PR_NUMBER = os.getenv('PR_NUMBER') + +REPO_OWNER, REPO_NAME = REPO.split('/') + +def create_markdown_table(results): + results = sorted(results, key=lambda x: x['name']) + header = "| Benchmark | Base | PR |\n|-----------|------|----|" + rows = [] + for result in results: + name = result['name'] + base = result['base'].replace('\n', '
') + pr = result['pr'].replace('\n', '
') + row = f"| {name} | {base} | {pr} |" + rows.append(row) + return f"{header}\n" + "\n".join(rows) + +def get_pr_details(repo_owner, repo_name, pr_number): + url = f"https://api.github.com/repos/{repo_owner}/{repo_name}/pulls/{pr_number}" + headers = {'Authorization': f'token {GITHUB_TOKEN}'} + response = requests.get(url, headers=headers) + response.raise_for_status() + return response.json() + +def update_pr_description(repo_owner, repo_name, pr_number, body): + url = f"https://api.github.com/repos/{repo_owner}/{repo_name}/pulls/{pr_number}" + headers = {'Authorization': f'token {GITHUB_TOKEN}'} + data = {'body': body} + response = requests.patch(url, headers=headers, json=data) + response.raise_for_status() + return response.json() + + +def collect_benchmark_results(base_folder, pr_folder): + results = [] + results_index = {} + + for file in os.listdir(base_folder): + if not file.endswith('.bench'): continue + with open(f"{base_folder}/{file}") as f: + result = f.read().strip() + results.append({'base': result, 'pr': None, 'name': os.path.splitext(file)[0]}) + results_index[file] = len(results) - 1 + + for file in os.listdir(pr_folder): + if not file.endswith('.bench'): continue + with open(f"{pr_folder}/{file}") as f: + result = f.read().strip() + if file in results_index: + results[results_index[file]]['pr'] = result + else: + results.append({'base': None, 'pr': result, 'name': os.path.splitext(file)[0]}) + + return results + +def main(): + if len(sys.argv) != 3: + print("Usage: python post_benchmark_results.py ") + exit(1) + + base_folder = sys.argv[1] + pr_folder = sys.argv[2] + + benchmark_results = collect_benchmark_results(base_folder, pr_folder) + + pr_details = get_pr_details(REPO_OWNER, REPO_NAME, PR_NUMBER) + pr_body = pr_details.get('body', '') + + + markdown_table = create_markdown_table(benchmark_results) + new_benchmark_section = f"\n## Benchmark Results\n{markdown_table}\n" + + if re.search(r'.*', pr_body, re.DOTALL): + updated_body = re.sub( + r'.*', + new_benchmark_section, + pr_body, + flags=re.DOTALL + ) + else: + updated_body = f"{pr_body}\n\n{new_benchmark_section}" + + update_pr_description(REPO_OWNER, REPO_NAME, PR_NUMBER, updated_body) + print("PR description updated successfully.") + + + +if __name__ == "__main__": + main() diff --git a/scripts/ci/run_benchmarks.sh b/scripts/ci/run_benchmarks.sh new file mode 100755 index 000000000..94cf57a57 --- /dev/null +++ b/scripts/ci/run_benchmarks.sh @@ -0,0 +1,24 @@ +#!/bin/bash +set -eou pipefail + +function run_benchmarks_for_folder { + echo "Running benchmarks for $1" + + FOLDER=$1 + RESULTS_FOLDER=$2 + + mkdir -p $RESULTS_FOLDER + + BENCHMARKS_FOLDER="$FOLDER/build/src/benchmarks" + + ./$BENCHMARKS_FOLDER/match-bench "./$FOLDER/test/data/mld/monaco.osrm" mld > "$RESULTS_FOLDER/match_mld.bench" + ./$BENCHMARKS_FOLDER/match-bench "./$FOLDER/test/data/ch/monaco.osrm" ch > "$RESULTS_FOLDER/match_ch.bench" + ./$BENCHMARKS_FOLDER/alias-bench > "$RESULTS_FOLDER/alias.bench" + ./$BENCHMARKS_FOLDER/json-render-bench "./$FOLDER/src/benchmarks/portugal_to_korea.json" > "$RESULTS_FOLDER/json-render.bench" + ./$BENCHMARKS_FOLDER/packedvector-bench > "$RESULTS_FOLDER/packedvector.bench" + ./$BENCHMARKS_FOLDER/rtree-bench "./$FOLDER/test/data/monaco.osrm.ramIndex" "./$FOLDER/test/data/monaco.osrm.fileIndex" "./$FOLDER/test/data/monaco.osrm.nbg_nodes" > "$RESULTS_FOLDER/rtree.bench" +} + +run_benchmarks_for_folder $1 "${1}_results" +run_benchmarks_for_folder $2 "${2}_results" + diff --git a/src/benchmarks/alias.cpp b/src/benchmarks/alias.cpp index f8ca5dd12..f2eab6af8 100644 --- a/src/benchmarks/alias.cpp +++ b/src/benchmarks/alias.cpp @@ -64,7 +64,7 @@ int main(int, char **) return EXIT_FAILURE; } TIMER_STOP(aliased_u32); - util::Log() << "aliased u32: " << TIMER_MSEC(aliased_u32); + std::cout << "aliased u32: " << TIMER_MSEC(aliased_u32) << std::endl; TIMER_START(plain_u32); for (auto round : util::irange(0, num_rounds)) @@ -83,7 +83,7 @@ int main(int, char **) return EXIT_FAILURE; } TIMER_STOP(plain_u32); - util::Log() << "plain u32: " << TIMER_MSEC(plain_u32); + std::cout << "plain u32: " << TIMER_MSEC(plain_u32) << std::endl; TIMER_START(aliased_double); for (auto round : util::irange(0, num_rounds)) @@ -103,7 +103,7 @@ int main(int, char **) return EXIT_FAILURE; } TIMER_STOP(aliased_double); - util::Log() << "aliased double: " << TIMER_MSEC(aliased_double); + std::cout << "aliased double: " << TIMER_MSEC(aliased_double) << std::endl; TIMER_START(plain_double); for (auto round : util::irange(0, num_rounds)) @@ -123,5 +123,5 @@ int main(int, char **) return EXIT_FAILURE; } TIMER_STOP(plain_double); - util::Log() << "plain double: " << TIMER_MSEC(plain_double); + std::cout << "plain double: " << TIMER_MSEC(plain_double) << std::endl; } diff --git a/src/benchmarks/packed_vector.cpp b/src/benchmarks/packed_vector.cpp index ac51d1a68..62aa1634d 100644 --- a/src/benchmarks/packed_vector.cpp +++ b/src/benchmarks/packed_vector.cpp @@ -72,10 +72,10 @@ int main(int, char **) auto write_slowdown = result_packed.random_write_ms / result_plain.random_write_ms; auto read_slowdown = result_packed.random_read_ms / result_plain.random_read_ms; - util::Log() << "random write: std::vector " << result_plain.random_write_ms - << " ms, util::packed_vector " << result_packed.random_write_ms << " ms. " - << write_slowdown; - util::Log() << "random read: std::vector " << result_plain.random_read_ms - << " ms, util::packed_vector " << result_packed.random_read_ms << " ms. " - << read_slowdown; + std::cout << "random write:\nstd::vector " << result_plain.random_write_ms + << " ms\nutil::packed_vector " << result_packed.random_write_ms << " ms\n" + << "slowdown: " << write_slowdown << std::endl; + std::cout << "random read:\nstd::vector " << result_plain.random_read_ms + << " ms\nutil::packed_vector " << result_packed.random_read_ms << " ms\n" + << "slowdown: " << read_slowdown << std::endl; } diff --git a/src/benchmarks/static_rtree.cpp b/src/benchmarks/static_rtree.cpp index d2dd08fe5..eaa784ea8 100644 --- a/src/benchmarks/static_rtree.cpp +++ b/src/benchmarks/static_rtree.cpp @@ -36,8 +36,6 @@ void benchmarkQuery(const std::vector &queries, const std::string &name, QueryT query) { - std::cout << "Running " << name << " with " << queries.size() << " coordinates: " << std::flush; - TIMER_START(query); for (const auto &q : queries) { @@ -46,11 +44,9 @@ void benchmarkQuery(const std::vector &queries, } TIMER_STOP(query); - std::cout << "Took " << TIMER_SEC(query) << " seconds " - << "(" << TIMER_MSEC(query) << "ms" - << ") -> " << TIMER_MSEC(query) / queries.size() << " ms/query " - << "(" << TIMER_MSEC(query) << "ms" - << ")" << std::endl; + std::cout << name << ":\n" + << TIMER_MSEC(query) << "ms" + << " -> " << TIMER_MSEC(query) / queries.size() << " ms/query" << std::endl; } void benchmark(BenchStaticRTree &rtree, unsigned num_queries) @@ -65,11 +61,10 @@ void benchmark(BenchStaticRTree &rtree, unsigned num_queries) util::FixedLatitude{lat_udist(mt_rand)}); } + benchmarkQuery( + queries, "1 result", [&rtree](const util::Coordinate &q) { return rtree.Nearest(q, 1); }); benchmarkQuery(queries, - "raw RTree queries (1 result)", - [&rtree](const util::Coordinate &q) { return rtree.Nearest(q, 1); }); - benchmarkQuery(queries, - "raw RTree queries (10 results)", + "10 results", [&rtree](const util::Coordinate &q) { return rtree.Nearest(q, 10); }); } } // namespace osrm::benchmarks From 11c7ddc84d2f32180cd5cdd74c07fb077f1c2c81 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Sun, 19 May 2024 19:30:24 +0200 Subject: [PATCH 6/9] Fix failing gcc-13 based CI jobs (#6886) * Attempt to fix failing CI on gcc-13 jobs * Attempt to fix failing CI on gcc-13 jobs * Attempt to fix failing CI on gcc-13 jobs * Attempt to fix failing CI on gcc-13 jobs * Attempt to fix failing CI on gcc-13 jobs --- .github/workflows/osrm-backend.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/osrm-backend.yml b/.github/workflows/osrm-backend.yml index f1b28b310..3b8242153 100644 --- a/.github/workflows/osrm-backend.yml +++ b/.github/workflows/osrm-backend.yml @@ -151,7 +151,7 @@ jobs: - name: gcc-13-debug-cov continue-on-error: false node: 20 - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 BUILD_TOOLS: ON BUILD_TYPE: Debug CCOMPILER: gcc-13 @@ -248,7 +248,7 @@ jobs: - name: gcc-13-release continue-on-error: false node: 20 - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 BUILD_TOOLS: ON BUILD_TYPE: Release CCOMPILER: gcc-13 From 89fce286a7cc93f557971c459c0abaf6077cc697 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 20 May 2024 09:16:53 +0200 Subject: [PATCH 7/9] Fix benchmark script for the case if PR has empty description (#6887) --- scripts/ci/post_benchmark_results.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/ci/post_benchmark_results.py b/scripts/ci/post_benchmark_results.py index 3efd5fcad..a5dc38aa5 100644 --- a/scripts/ci/post_benchmark_results.py +++ b/scripts/ci/post_benchmark_results.py @@ -71,9 +71,9 @@ def main(): benchmark_results = collect_benchmark_results(base_folder, pr_folder) pr_details = get_pr_details(REPO_OWNER, REPO_NAME, PR_NUMBER) - pr_body = pr_details.get('body', '') + # in both cases when there is no PR body or PR body is None fallback to empty string + pr_body = pr_details.get('body', '') or '' - markdown_table = create_markdown_table(benchmark_results) new_benchmark_section = f"\n## Benchmark Results\n{markdown_table}\n" @@ -85,7 +85,7 @@ def main(): flags=re.DOTALL ) else: - updated_body = f"{pr_body}\n\n{new_benchmark_section}" + updated_body = f"{pr_body}\n\n{new_benchmark_section}" if len(pr_body) > 0 else new_benchmark_section update_pr_description(REPO_OWNER, REPO_NAME, PR_NUMBER, updated_body) print("PR description updated successfully.") From 8a82d3929ca18902477f19c625ddefa98873473f Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 20 May 2024 11:15:55 +0200 Subject: [PATCH 8/9] Improve map matching benchmark (#6885) --- src/benchmarks/match.cpp | 61 ++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/src/benchmarks/match.cpp b/src/benchmarks/match.cpp index caed1a00b..83ef7ef1b 100644 --- a/src/benchmarks/match.cpp +++ b/src/benchmarks/match.cpp @@ -11,14 +11,14 @@ #include "osrm/status.hpp" #include - +#include #include #include +#include +#include #include #include -#include - int main(int argc, const char *argv[]) try { @@ -214,24 +214,49 @@ try params.coordinates.push_back( FloatCoordinate{FloatLongitude{7.415342330932617}, FloatLatitude{43.733251335381205}}); - TIMER_START(routes); - auto NUM = 100; - for (int i = 0; i < NUM; ++i) + auto run_benchmark = [&](std::optional radiusInMeters) { - engine::api::ResultT result = json::Object(); - const auto rc = osrm.Match(params, result); - auto &json_result = result.get(); - if (rc != Status::Ok || - json_result.values.at("matchings").get().values.size() != 1) + params.radiuses = {}; + if (radiusInMeters) { - return EXIT_FAILURE; + for (size_t index = 0; index < params.coordinates.size(); ++index) + { + params.radiuses.emplace_back(*radiusInMeters); + } } + + TIMER_START(routes); + auto NUM = 100; + for (int i = 0; i < NUM; ++i) + { + engine::api::ResultT result = json::Object(); + const auto rc = osrm.Match(params, result); + auto &json_result = result.get(); + if (rc != Status::Ok || + json_result.values.at("matchings").get().values.size() != 1) + { + throw std::runtime_error{"Couldn't match"}; + } + } + TIMER_STOP(routes); + if (radiusInMeters) + { + std::cout << "Radius " << *radiusInMeters << "m: " << std::endl; + } + else + { + std::cout << "Default radius: " << std::endl; + } + std::cout << (TIMER_MSEC(routes) / NUM) << "ms/req at " << params.coordinates.size() + << " coordinate" << std::endl; + std::cout << (TIMER_MSEC(routes) / NUM / params.coordinates.size()) << "ms/coordinate" + << std::endl; + }; + + for (auto radius : std::vector>{std::nullopt, 5.0, 10.0, 15.0, 30.0}) + { + run_benchmark(radius); } - TIMER_STOP(routes); - std::cout << (TIMER_MSEC(routes) / NUM) << "ms/req at " << params.coordinates.size() - << " coordinate" << std::endl; - std::cout << (TIMER_MSEC(routes) / NUM / params.coordinates.size()) << "ms/coordinate" - << std::endl; return EXIT_SUCCESS; } @@ -239,4 +264,4 @@ catch (const std::exception &e) { std::cerr << "Error: " << e.what() << std::endl; return EXIT_FAILURE; -} +} \ No newline at end of file From d259848456c8c84df0422f7389ef034e63e6462d Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 20 May 2024 12:32:40 +0200 Subject: [PATCH 9/9] Optimise R-tree queries in the case of map matching (#6881) --- CHANGELOG.md | 1 + .../contiguous_internalmem_datafacade.hpp | 2 +- include/engine/geospatial_query.hpp | 38 +++- include/util/coordinate_calculation.hpp | 7 + include/util/rectangle.hpp | 13 +- include/util/static_rtree.hpp | 175 +++++++++++------- src/benchmarks/match.cpp | 1 + unit_tests/util/static_rtree.cpp | 68 +++++-- 8 files changed, 219 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a736d3040..72cea29b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ - FIXED: Correctly check runtime search conditions for forcing routing steps [#6866](https://github.com/Project-OSRM/osrm-backend/pull/6866) - Map Matching: - CHANGED: Optimise path distance calculation in MLD map matching. [#6876](https://github.com/Project-OSRM/osrm-backend/pull/6876) + - CHANGED: Optimise R-tree queries in the case of map matching. [#6881](https://github.com/Project-OSRM/osrm-backend/pull/6876) - Debug tiles: - FIXED: Ensure speed layer features have unique ids. [#6726](https://github.com/Project-OSRM/osrm-backend/pull/6726) diff --git a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp index 9b35989e1..ac9497eb0 100644 --- a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp +++ b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp @@ -375,7 +375,7 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade BOOST_ASSERT(m_geospatial_query.get()); return m_geospatial_query->NearestPhantomNodes( - input_coordinate, approach, boost::none, max_distance, bearing, use_all_edges); + input_coordinate, approach, max_distance, bearing, use_all_edges); } std::vector diff --git a/include/engine/geospatial_query.hpp b/include/engine/geospatial_query.hpp index b8e43b637..2fce9c0c9 100644 --- a/include/engine/geospatial_query.hpp +++ b/include/engine/geospatial_query.hpp @@ -47,12 +47,42 @@ template class GeospatialQuery return rtree.SearchInBox(bbox); } + std::vector + NearestPhantomNodes(const util::Coordinate input_coordinate, + const Approach approach, + const double max_distance, + const boost::optional bearing_with_range, + const boost::optional use_all_edges) const + { + auto results = rtree.SearchInRange( + input_coordinate, + max_distance, + [this, approach, &input_coordinate, &bearing_with_range, &use_all_edges, max_distance]( + const CandidateSegment &segment) + { + auto invalidDistance = + CheckSegmentDistance(input_coordinate, segment, max_distance); + if (invalidDistance) + { + return std::make_pair(false, false); + } + auto valid = CheckSegmentExclude(segment) && + CheckApproach(input_coordinate, segment, approach) && + (use_all_edges ? HasValidEdge(segment, *use_all_edges) + : HasValidEdge(segment)) && + (bearing_with_range ? CheckSegmentBearing(segment, *bearing_with_range) + : std::make_pair(true, true)); + return valid; + }); + return MakePhantomNodes(input_coordinate, results); + } + // Returns max_results nearest PhantomNodes that are valid within the provided parameters. // Does not filter by small/big component! std::vector NearestPhantomNodes(const util::Coordinate input_coordinate, const Approach approach, - const boost::optional max_results, + const size_t max_results, const boost::optional max_distance, const boost::optional bearing_with_range, const boost::optional use_all_edges) const @@ -70,10 +100,10 @@ template class GeospatialQuery : std::make_pair(true, true)); return valid; }, - [this, &max_distance, &max_results, input_coordinate](const std::size_t num_results, - const CandidateSegment &segment) + [this, &max_distance, max_results, input_coordinate](const std::size_t num_results, + const CandidateSegment &segment) { - return (max_results && num_results >= *max_results) || + return (num_results >= max_results) || (max_distance && max_distance != -1.0 && CheckSegmentDistance(input_coordinate, segment, *max_distance)); }); diff --git a/include/util/coordinate_calculation.hpp b/include/util/coordinate_calculation.hpp index 3ab8db076..6a716ffaf 100644 --- a/include/util/coordinate_calculation.hpp +++ b/include/util/coordinate_calculation.hpp @@ -36,6 +36,13 @@ inline double radToDeg(const double radian) } } // namespace detail +const constexpr static double METERS_PER_DEGREE_LAT = 110567.0; + +inline double metersPerLngDegree(const FixedLatitude lat) +{ + return std::cos(detail::degToRad(static_cast(toFloating(lat)))) * METERS_PER_DEGREE_LAT; +} + //! Takes the squared euclidean distance of the input coordinates. Does not return meters! std::uint64_t squaredEuclideanDistance(const Coordinate lhs, const Coordinate rhs); diff --git a/include/util/rectangle.hpp b/include/util/rectangle.hpp index 65050f200..aca192446 100644 --- a/include/util/rectangle.hpp +++ b/include/util/rectangle.hpp @@ -3,7 +3,6 @@ #include "util/coordinate.hpp" #include "util/coordinate_calculation.hpp" - #include #include @@ -168,6 +167,18 @@ struct RectangleInt2D min_lat != FixedLatitude{std::numeric_limits::max()} && max_lat != FixedLatitude{std::numeric_limits::min()}; } + + static RectangleInt2D ExpandMeters(const Coordinate &coordinate, const double meters) + { + const double lat_offset = meters / coordinate_calculation::METERS_PER_DEGREE_LAT; + const double lon_offset = + meters / coordinate_calculation::metersPerLngDegree(coordinate.lat); + + return RectangleInt2D{coordinate.lon - toFixed(FloatLongitude{lon_offset}), + coordinate.lon + toFixed(FloatLongitude{lon_offset}), + coordinate.lat - toFixed(FloatLatitude{lat_offset}), + coordinate.lat + toFixed(FloatLatitude{lat_offset})}; + } }; } // namespace osrm::util diff --git a/include/util/static_rtree.hpp b/include/util/static_rtree.hpp index d39d8454d..dc88da233 100644 --- a/include/util/static_rtree.hpp +++ b/include/util/static_rtree.hpp @@ -2,7 +2,6 @@ #define STATIC_RTREE_HPP #include "storage/tar_fwd.hpp" - #include "util/bearing.hpp" #include "util/coordinate_calculation.hpp" #include "util/deallocating_vector.hpp" @@ -11,6 +10,7 @@ #include "util/integer_range.hpp" #include "util/mmap_file.hpp" #include "util/rectangle.hpp" +#include "util/timing_util.hpp" #include "util/typedefs.hpp" #include "util/vector_view.hpp" #include "util/web_mercator.hpp" @@ -487,70 +487,9 @@ class StaticRTree Rectangle needs to be projected!*/ std::vector SearchInBox(const Rectangle &search_rectangle) const { - const Rectangle projected_rectangle{ - search_rectangle.min_lon, - search_rectangle.max_lon, - toFixed(FloatLatitude{ - web_mercator::latToY(toFloating(FixedLatitude(search_rectangle.min_lat)))}), - toFixed(FloatLatitude{ - web_mercator::latToY(toFloating(FixedLatitude(search_rectangle.max_lat)))})}; std::vector results; - - std::queue traversal_queue; - traversal_queue.push(TreeIndex{}); - - while (!traversal_queue.empty()) - { - auto const current_tree_index = traversal_queue.front(); - traversal_queue.pop(); - - // If we're at the bottom of the tree, we need to explore the - // element array - if (is_leaf(current_tree_index)) - { - - // Note: irange is [start,finish), so we need to +1 to make sure we visit the - // last - for (const auto current_child_index : child_indexes(current_tree_index)) - { - const auto ¤t_edge = m_objects[current_child_index]; - - // we don't need to project the coordinates here, - // because we use the unprojected rectangle to test against - const Rectangle bbox{std::min(m_coordinate_list[current_edge.u].lon, - m_coordinate_list[current_edge.v].lon), - std::max(m_coordinate_list[current_edge.u].lon, - m_coordinate_list[current_edge.v].lon), - std::min(m_coordinate_list[current_edge.u].lat, - m_coordinate_list[current_edge.v].lat), - std::max(m_coordinate_list[current_edge.u].lat, - m_coordinate_list[current_edge.v].lat)}; - - // use the _unprojected_ input rectangle here - if (bbox.Intersects(search_rectangle)) - { - results.push_back(current_edge); - } - } - } - else - { - BOOST_ASSERT(current_tree_index.level + 1 < m_tree_level_starts.size()); - - for (const auto child_index : child_indexes(current_tree_index)) - { - const auto &child_rectangle = - m_search_tree[child_index].minimum_bounding_rectangle; - - if (child_rectangle.Intersects(projected_rectangle)) - { - traversal_queue.push(TreeIndex( - current_tree_index.level + 1, - child_index - m_tree_level_starts[current_tree_index.level + 1])); - } - } - } - } + SearchInBox(search_rectangle, + [&results](const auto &edge_data) { results.push_back(edge_data); }); return results; } @@ -565,6 +504,45 @@ class StaticRTree { return num_results >= max_results; }); } + // NB 1: results are not guaranteed to be sorted by distance + // NB 2: maxDistanceMeters is not a hard limit, it's just a way to reduce the number of edges + // returned + template + std::vector SearchInRange(const Coordinate input_coordinate, + double maxDistanceMeters, + const FilterT filter) const + { + auto projected_coordinate = web_mercator::fromWGS84(input_coordinate); + Coordinate fixed_projected_coordinate{projected_coordinate}; + + auto bbox = Rectangle::ExpandMeters(input_coordinate, maxDistanceMeters); + std::vector results; + + SearchInBox( + bbox, + [&results, &filter, fixed_projected_coordinate, this](const EdgeDataT ¤t_edge) + { + const auto projected_u = web_mercator::fromWGS84(m_coordinate_list[current_edge.u]); + const auto projected_v = web_mercator::fromWGS84(m_coordinate_list[current_edge.v]); + + auto [_, projected_nearest] = coordinate_calculation::projectPointOnSegment( + projected_u, projected_v, fixed_projected_coordinate); + + CandidateSegment current_candidate{projected_nearest, current_edge}; + auto use_segment = filter(current_candidate); + if (!use_segment.first && !use_segment.second) + { + return; + } + current_candidate.data.forward_segment_id.enabled &= use_segment.first; + current_candidate.data.reverse_segment_id.enabled &= use_segment.second; + + results.push_back(current_candidate); + }); + + return results; + } + // Return edges in distance order with the coordinate of the closest point on the edge. template std::vector Nearest(const Coordinate input_coordinate, @@ -572,8 +550,10 @@ class StaticRTree const TerminationT terminate) const { std::vector results; + auto projected_coordinate = web_mercator::fromWGS84(input_coordinate); Coordinate fixed_projected_coordinate{projected_coordinate}; + // initialize queue with root element std::priority_queue traversal_queue; traversal_queue.push(QueryCandidate{0, TreeIndex{}}); @@ -631,6 +611,73 @@ class StaticRTree } private: + template + void SearchInBox(const Rectangle &search_rectangle, Callback &&callback) const + { + const Rectangle projected_rectangle{ + search_rectangle.min_lon, + search_rectangle.max_lon, + toFixed(FloatLatitude{ + web_mercator::latToY(toFloating(FixedLatitude(search_rectangle.min_lat)))}), + toFixed(FloatLatitude{ + web_mercator::latToY(toFloating(FixedLatitude(search_rectangle.max_lat)))})}; + std::queue traversal_queue; + traversal_queue.push(TreeIndex{}); + + while (!traversal_queue.empty()) + { + auto const current_tree_index = traversal_queue.front(); + traversal_queue.pop(); + + // If we're at the bottom of the tree, we need to explore the + // element array + if (is_leaf(current_tree_index)) + { + + // Note: irange is [start,finish), so we need to +1 to make sure we visit the + // last + for (const auto current_child_index : child_indexes(current_tree_index)) + { + const auto ¤t_edge = m_objects[current_child_index]; + + // we don't need to project the coordinates here, + // because we use the unprojected rectangle to test against + const Rectangle bbox{std::min(m_coordinate_list[current_edge.u].lon, + m_coordinate_list[current_edge.v].lon), + std::max(m_coordinate_list[current_edge.u].lon, + m_coordinate_list[current_edge.v].lon), + std::min(m_coordinate_list[current_edge.u].lat, + m_coordinate_list[current_edge.v].lat), + std::max(m_coordinate_list[current_edge.u].lat, + m_coordinate_list[current_edge.v].lat)}; + + // use the _unprojected_ input rectangle here + if (bbox.Intersects(search_rectangle)) + { + callback(current_edge); + } + } + } + else + { + BOOST_ASSERT(current_tree_index.level + 1 < m_tree_level_starts.size()); + + for (const auto child_index : child_indexes(current_tree_index)) + { + const auto &child_rectangle = + m_search_tree[child_index].minimum_bounding_rectangle; + + if (child_rectangle.Intersects(projected_rectangle)) + { + traversal_queue.push(TreeIndex( + current_tree_index.level + 1, + child_index - m_tree_level_starts[current_tree_index.level + 1])); + } + } + } + } + } + /** * Iterates over all the objects in a leaf node and inserts them into our * search priority queue. The speed of this function is very much governed diff --git a/src/benchmarks/match.cpp b/src/benchmarks/match.cpp index 83ef7ef1b..cece7960b 100644 --- a/src/benchmarks/match.cpp +++ b/src/benchmarks/match.cpp @@ -11,6 +11,7 @@ #include "osrm/status.hpp" #include + #include #include #include diff --git a/unit_tests/util/static_rtree.cpp b/unit_tests/util/static_rtree.cpp index f46aa7387..812202d38 100644 --- a/unit_tests/util/static_rtree.cpp +++ b/unit_tests/util/static_rtree.cpp @@ -348,7 +348,13 @@ BOOST_AUTO_TEST_CASE(radius_regression_test) { auto results = query.NearestPhantomNodes( - input, osrm::engine::Approach::UNRESTRICTED, boost::none, 0.01, boost::none, true); + input, osrm::engine::Approach::UNRESTRICTED, 0.01, boost::none, true); + BOOST_CHECK_EQUAL(results.size(), 0); + } + + { + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 1, 0.01, boost::none, true); BOOST_CHECK_EQUAL(results.size(), 0); } } @@ -374,13 +380,25 @@ BOOST_AUTO_TEST_CASE(permissive_edge_snapping) { auto results = query.NearestPhantomNodes( - input, osrm::engine::Approach::UNRESTRICTED, boost::none, 1000, boost::none, false); + input, osrm::engine::Approach::UNRESTRICTED, 1000, boost::none, false); BOOST_CHECK_EQUAL(results.size(), 1); } { auto results = query.NearestPhantomNodes( - input, osrm::engine::Approach::UNRESTRICTED, boost::none, 1000, boost::none, true); + input, osrm::engine::Approach::UNRESTRICTED, 1000, boost::none, true); + BOOST_CHECK_EQUAL(results.size(), 2); + } + + { + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 10, 1000, boost::none, false); + BOOST_CHECK_EQUAL(results.size(), 1); + } + + { + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 10, 1000, boost::none, true); BOOST_CHECK_EQUAL(results.size(), 2); } } @@ -442,27 +460,45 @@ BOOST_AUTO_TEST_CASE(bearing_tests) { auto results = query.NearestPhantomNodes( - input, osrm::engine::Approach::UNRESTRICTED, boost::none, 11000, boost::none, true); + input, osrm::engine::Approach::UNRESTRICTED, 11000, boost::none, true); BOOST_CHECK_EQUAL(results.size(), 2); } { - auto results = query.NearestPhantomNodes(input, - osrm::engine::Approach::UNRESTRICTED, - boost::none, - 11000, - engine::Bearing{270, 10}, - true); + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 10, 11000, boost::none, true); + BOOST_CHECK_EQUAL(results.size(), 2); + } + + { + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 11000, engine::Bearing{270, 10}, true); BOOST_CHECK_EQUAL(results.size(), 0); } { - auto results = query.NearestPhantomNodes(input, - osrm::engine::Approach::UNRESTRICTED, - boost::none, - 11000, - engine::Bearing{45, 10}, - true); + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 10, 11000, engine::Bearing{270, 10}, true); + BOOST_CHECK_EQUAL(results.size(), 0); + } + + { + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 11000, engine::Bearing{45, 10}, true); + BOOST_CHECK_EQUAL(results.size(), 2); + + BOOST_CHECK(results[0].phantom_node.forward_segment_id.enabled); + BOOST_CHECK(!results[0].phantom_node.reverse_segment_id.enabled); + BOOST_CHECK_EQUAL(results[0].phantom_node.forward_segment_id.id, 1); + + BOOST_CHECK(!results[1].phantom_node.forward_segment_id.enabled); + BOOST_CHECK(results[1].phantom_node.reverse_segment_id.enabled); + BOOST_CHECK_EQUAL(results[1].phantom_node.reverse_segment_id.id, 1); + } + + { + auto results = query.NearestPhantomNodes( + input, osrm::engine::Approach::UNRESTRICTED, 10, 11000, engine::Bearing{45, 10}, true); BOOST_CHECK_EQUAL(results.size(), 2); BOOST_CHECK(results[0].phantom_node.forward_segment_id.enabled);