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.
This commit is contained in:
Michael Bell
2024-05-10 22:00:24 +01:00
committed by GitHub
parent 70969186f6
commit ffc39b8ad2
12 changed files with 180 additions and 126 deletions
@@ -187,7 +187,6 @@ void computeWeightAndSharingOfViaPath(SearchEngineData<Algorithm> &engine_workin
s_v_middle,
upper_bound_s_v_path_weight,
min_edge_offset,
{},
{});
}
// compute path <v,..,t> by reusing backward search from node t
@@ -202,7 +201,6 @@ void computeWeightAndSharingOfViaPath(SearchEngineData<Algorithm> &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<Algorithm> &engine_working_dat
*s_v_middle,
upper_bound_s_v_path_weight,
min_edge_offset,
{},
{});
}
@@ -369,7 +366,6 @@ bool viaNodeCandidatePassesTTest(SearchEngineData<Algorithm> &engine_working_dat
*v_t_middle,
upper_bound_of_v_t_path_weight,
min_edge_offset,
{},
{});
}
@@ -538,12 +534,12 @@ bool viaNodeCandidatePassesTTest(SearchEngineData<Algorithm> &engine_working_dat
if (!forward_heap3.Empty())
{
routingStep<FORWARD_DIRECTION>(
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<REVERSE_DIRECTION>(
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);
@@ -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<Algorithm> &search_engine_data,
overlap_via,
overlap_weight,
{},
{},
endpoint_candidates);
if (!forward_heap.Empty())
@@ -746,7 +744,6 @@ makeCandidateVias(SearchEngineData<Algorithm> &search_engine_data,
overlap_via,
overlap_weight,
{},
{},
endpoint_candidates);
if (!reverse_heap.Empty())
@@ -34,7 +34,6 @@ InternalRouteResult directShortestPathSearch(SearchEngineData<ch::Algorithm> &en
weight,
packed_leg,
{},
{},
endpoint_candidates);
std::vector<NodeID> unpacked_nodes;
@@ -81,7 +80,6 @@ InternalRouteResult directShortestPathSearch(SearchEngineData<mld::Algorithm> &e
forward_heap,
reverse_heap,
{},
{},
INVALID_EDGE_WEIGHT,
endpoint_candidates);
+18 -10
View File
@@ -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<NodeID> getForwardLoopNodes(const PhantomEndpointCandidates &endpoint_candidates)
std::vector<NodeID> getForwardForceNodes(const PhantomEndpointCandidates &endpoint_candidates)
{
std::vector<NodeID> res;
for (const auto &source_phantom : endpoint_candidates.source_phantoms)
@@ -26,7 +34,7 @@ std::vector<NodeID> 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<NodeID> getForwardLoopNodes(const PhantomEndpointCandidates &endpoin
return res;
}
std::vector<NodeID> getForwardLoopNodes(const PhantomCandidatesToTarget &endpoint_candidates)
std::vector<NodeID> getForwardForceNodes(const PhantomCandidatesToTarget &endpoint_candidates)
{
std::vector<NodeID> 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<NodeID> getForwardLoopNodes(const PhantomCandidatesToTarget &endpoin
return res;
}
std::vector<NodeID> getBackwardLoopNodes(const PhantomEndpointCandidates &endpoint_candidates)
std::vector<NodeID> getBackwardForceNodes(const PhantomEndpointCandidates &endpoint_candidates)
{
std::vector<NodeID> res;
for (const auto &source_phantom : endpoint_candidates.source_phantoms)
@@ -57,7 +65,7 @@ std::vector<NodeID> 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<NodeID> getBackwardLoopNodes(const PhantomEndpointCandidates &endpoi
return res;
}
std::vector<NodeID> getBackwardLoopNodes(const PhantomCandidatesToTarget &endpoint_candidates)
std::vector<NodeID> getBackwardForceNodes(const PhantomCandidatesToTarget &endpoint_candidates)
{
std::vector<NodeID> 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);
}
@@ -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<Algorithm> & /*engine_working_data*/,
const DataFacade<Algorithm> &facade,
SearchEngineData<Algorithm>::QueryHeap &forward_heap,
SearchEngineData<Algorithm>::QueryHeap &reverse_heap,
EdgeWeight &weight,
std::vector<NodeID> &packed_leg,
const std::vector<NodeID> &force_loop_forward_nodes,
const std::vector<NodeID> &force_loop_reverse_nodes,
const std::vector<NodeID> &force_step_nodes,
const EdgeWeight weight_upper_bound)
{
if (forward_heap.Empty() || reverse_heap.Empty())
@@ -118,8 +117,7 @@ void search(SearchEngineData<Algorithm> & /*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<Algorithm> & /*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<Algorithm> & /*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<Algorithm> &engine_working_data,
const DataFacade<Algorithm> &facade,
SearchEngineData<Algorithm>::QueryHeap &forward_heap,
@@ -183,7 +180,6 @@ double getNetworkDistance(SearchEngineData<Algorithm> &engine_working_data,
weight,
packed_path,
{},
{},
endpoints,
weight_upper_bound);