From 823d7e45e79fcb3bbbcb74de2032a49e0565ede7 Mon Sep 17 00:00:00 2001 From: Daniel Patterson Date: Sun, 10 Feb 2019 00:42:10 -0800 Subject: [PATCH] Revert "change phantom node to store an appx speed regardless of direction. use this speed to estimate an accel/decel penalty and scale the penalty based on the distance traveled up to a cutoff. only used in the table plugin at the moment. TODO still lives in the guidance assemble_leg area" This reverts commit ca55521c8794b36b57fa6a136bffed07d882f8cb. --- include/engine/api/base_parameters.hpp | 6 +-- include/engine/geospatial_query.hpp | 53 +++++++++++++------ include/engine/guidance/assemble_leg.hpp | 8 ++- include/engine/hint.hpp | 2 +- include/engine/phantom_node.hpp | 34 ++++++------ .../routing_algorithms/routing_base.hpp | 8 +-- .../server/api/base_parameters_grammar.hpp | 8 +-- include/util/typedefs.hpp | 3 +- src/engine/plugins/table.cpp | 33 ++---------- 9 files changed, 78 insertions(+), 77 deletions(-) diff --git a/include/engine/api/base_parameters.hpp b/include/engine/api/base_parameters.hpp index 0ada3ac9a..16df39429 100644 --- a/include/engine/api/base_parameters.hpp +++ b/include/engine/api/base_parameters.hpp @@ -123,8 +123,8 @@ struct BaseParameters }); } }; -} // namespace api -} // namespace engine -} // namespace osrm +} +} +} #endif // ROUTE_PARAMETERS_HPP diff --git a/include/engine/geospatial_query.hpp b/include/engine/geospatial_query.hpp index 04871ec52..a059e445b 100644 --- a/include/engine/geospatial_query.hpp +++ b/include/engine/geospatial_query.hpp @@ -466,7 +466,6 @@ template class GeospatialQuery return distance_and_phantoms; } - // TODO: remove stoppage penalty as its not needed here anymore PhantomNodeWithDistance MakePhantomNode(const util::Coordinate input_coordinate, const EdgeData &data, const double min_stoppage_penalty = 0, @@ -556,11 +555,36 @@ template class GeospatialQuery point_on_segment, datafacade.GetCoordinateOfNode(forward_geometry(data.fwd_segment_position + 1))); + // where does this speed lie with respect to the min/max penalizable speeds + auto penalty_range = min_stoppage_penalty != INVALID_MINIMUM_STOPPAGE_PENALTY && + max_stoppage_penalty != INVALID_MAXIMUM_STOPPAGE_PENALTY + ? max_stoppage_penalty - min_stoppage_penalty + : 0; + auto stoppage_penalty = + [penalty_range, min_stoppage_penalty, max_stoppage_penalty](double speed) -> double { + // You're so slow already you don't get a penalty + if (speed < MINIMAL_ACCEL_DECEL_PENALIZABLE_SPEED) + return 0; + + // Find where it is on the scale + constexpr auto max = + MAXIMAL_ACCEL_DECEL_PENALIZABLE_SPEED - MINIMAL_ACCEL_DECEL_PENALIZABLE_SPEED; + auto ratio = (speed - MINIMAL_ACCEL_DECEL_PENALIZABLE_SPEED) / max; + + // You're faster than the max so you get the max + if (ratio >= 1) + return max_stoppage_penalty; + + // You're in between so you get a linear combination + return min_stoppage_penalty + ratio * penalty_range; + }; + // We may end up adding a stoppage penalty - double forward_speed = 0; - double reverse_speed = 0; - auto total_distance = 0; - auto penalty_range = max_stoppage_penalty - min_stoppage_penalty; + EdgeDuration forward_stoppage_penalty = 0; + EdgeDuration reverse_stoppage_penalty = 0; + auto total_distance = penalty_range > 0 ? appx_distance(forward_geometry.begin(), + std::prev(forward_geometry.end())) + : 0.0; ratio = std::min(1.0, std::max(0.0, ratio)); if (data.forward_segment_id.id != SPECIAL_SEGMENTID) { @@ -569,12 +593,11 @@ template class GeospatialQuery // Stoppage penalty based on speed if (data.forward_segment_id.enabled && penalty_range > 0) { - total_distance = - appx_distance(forward_geometry.begin(), std::prev(forward_geometry.end())); const auto total_duration = std::accumulate( forward_durations.begin(), forward_durations.end(), EdgeDuration{0}); - forward_speed = total_distance / (total_duration * 0.1); - reverse_speed = forward_speed; + const auto speed = total_distance / (total_duration * 0.1); + forward_stoppage_penalty = + static_cast((stoppage_penalty(speed) * 10) + .5); } } if (data.reverse_segment_id.id != SPECIAL_SEGMENTID) @@ -584,14 +607,11 @@ template class GeospatialQuery // Stoppage penalty based on speed if (data.reverse_segment_id.enabled && penalty_range > 0) { - if (total_distance == 0) - total_distance = - appx_distance(forward_geometry.begin(), std::prev(forward_geometry.end())); const auto total_duration = std::accumulate( reverse_durations.begin(), reverse_durations.end(), EdgeDuration{0}); - reverse_speed = total_distance / (total_duration * 0.1); - if (forward_speed == 0) - forward_speed = reverse_speed; + const auto speed = total_distance / (total_duration * 0.1); + reverse_stoppage_penalty = + static_cast((stoppage_penalty(speed) * 10) + .5); } } @@ -623,7 +643,8 @@ template class GeospatialQuery reverse_duration, forward_duration_offset, reverse_duration_offset, - static_cast((forward_speed + reverse_speed) / 2.0), + forward_stoppage_penalty, + reverse_stoppage_penalty, is_forward_valid_source, is_forward_valid_target, is_reverse_valid_source, diff --git a/include/engine/guidance/assemble_leg.hpp b/include/engine/guidance/assemble_leg.hpp index 025a4e968..cd10f70a6 100644 --- a/include/engine/guidance/assemble_leg.hpp +++ b/include/engine/guidance/assemble_leg.hpp @@ -182,8 +182,12 @@ inline RouteLeg assembleLeg(const datafacade::BaseDataFacade &facade, duration = std::max(0, duration); } - // TODO: Add start and stop penalties to the duration to simulate accel/deceleration - // + // Add start and stop penalties + if (distance > 0) + duration += + (target_traversed_in_reverse + ? source_node.reverse_duration_penalty + target_node.reverse_duration_penalty + : source_node.forward_duration_penalty + target_node.forward_duration_penalty); std::string summary; if (needs_summary) diff --git a/include/engine/hint.hpp b/include/engine/hint.hpp index 097e8feb7..179a716a4 100644 --- a/include/engine/hint.hpp +++ b/include/engine/hint.hpp @@ -64,7 +64,7 @@ struct Hint }; static_assert(sizeof(Hint) == sizeof(PhantomNode) + 4, "Hint is bigger than expected"); -constexpr std::size_t ENCODED_HINT_SIZE = 120; +constexpr std::size_t ENCODED_HINT_SIZE = 124; static_assert(ENCODED_HINT_SIZE / 4 * 3 >= sizeof(Hint), "ENCODED_HINT_SIZE does not match size of Hint"); } // namespace engine diff --git a/include/engine/phantom_node.hpp b/include/engine/phantom_node.hpp index 5360da7ed..8c948d16a 100644 --- a/include/engine/phantom_node.hpp +++ b/include/engine/phantom_node.hpp @@ -44,16 +44,16 @@ namespace engine struct PhantomNode { PhantomNode() - : forward_segment_id{SPECIAL_SEGMENTID, false}, reverse_segment_id{SPECIAL_SEGMENTID, - false}, - forward_weight(INVALID_EDGE_WEIGHT), reverse_weight(INVALID_EDGE_WEIGHT), - forward_weight_offset(0), reverse_weight_offset(0), + : forward_segment_id{SPECIAL_SEGMENTID, false}, + reverse_segment_id{SPECIAL_SEGMENTID, false}, forward_weight(INVALID_EDGE_WEIGHT), + reverse_weight(INVALID_EDGE_WEIGHT), forward_weight_offset(0), reverse_weight_offset(0), forward_distance(INVALID_EDGE_DISTANCE), reverse_distance(INVALID_EDGE_DISTANCE), forward_distance_offset(0), reverse_distance_offset(0), forward_duration(MAXIMAL_EDGE_DURATION), reverse_duration(MAXIMAL_EDGE_DURATION), - forward_duration_offset(0), reverse_duration_offset(0), speed_approximation(0), - fwd_segment_position(0), is_valid_forward_source{false}, is_valid_forward_target{false}, - is_valid_reverse_source{false}, is_valid_reverse_target{false}, bearing(0) + forward_duration_offset(0), reverse_duration_offset(0), forward_duration_penalty(0), + reverse_duration_penalty(0), fwd_segment_position(0), is_valid_forward_source{false}, + is_valid_forward_target{false}, is_valid_reverse_source{false}, + is_valid_reverse_target{false}, bearing(0) { } @@ -110,9 +110,8 @@ struct PhantomNode bool IsValid(const unsigned number_of_nodes) const { - return location.IsValid() && - ((forward_segment_id.id < number_of_nodes) || - (reverse_segment_id.id < number_of_nodes)) && + return location.IsValid() && ((forward_segment_id.id < number_of_nodes) || + (reverse_segment_id.id < number_of_nodes)) && ((forward_weight != INVALID_EDGE_WEIGHT) || (reverse_weight != INVALID_EDGE_WEIGHT)) && ((forward_duration != MAXIMAL_EDGE_DURATION) || @@ -169,7 +168,8 @@ struct PhantomNode EdgeDuration reverse_duration, EdgeDuration forward_duration_offset, EdgeDuration reverse_duration_offset, - EdgeDistance speed_approximation, + EdgeDuration forward_duration_penalty, + EdgeDuration reverse_duration_penalty, bool is_valid_forward_source, bool is_valid_forward_target, bool is_valid_reverse_source, @@ -185,9 +185,10 @@ struct PhantomNode reverse_distance_offset{reverse_distance_offset}, forward_duration{forward_duration}, reverse_duration{reverse_duration}, forward_duration_offset{forward_duration_offset}, reverse_duration_offset{reverse_duration_offset}, - speed_approximation{speed_approximation}, component{component.id, component.is_tiny}, - location{location}, input_location{input_location}, - fwd_segment_position{other.fwd_segment_position}, + forward_duration_penalty{forward_duration_penalty}, + reverse_duration_penalty{reverse_duration_penalty}, + component{component.id, component.is_tiny}, location{location}, + input_location{input_location}, fwd_segment_position{other.fwd_segment_position}, is_valid_forward_source{is_valid_forward_source}, is_valid_forward_target{is_valid_forward_target}, is_valid_reverse_source{is_valid_reverse_source}, @@ -209,7 +210,8 @@ struct PhantomNode EdgeDuration reverse_duration; EdgeDuration forward_duration_offset; // TODO: try to remove -> requires path unpacking changes EdgeDuration reverse_duration_offset; // TODO: try to remove -> requires path unpacking changes - EdgeDistance speed_approximation; // m/s + EdgeDuration forward_duration_penalty; + EdgeDuration reverse_duration_penalty; ComponentID component; @@ -225,7 +227,7 @@ struct PhantomNode unsigned short bearing : 12; }; -static_assert(sizeof(PhantomNode) == 84, "PhantomNode has more padding then expected"); +static_assert(sizeof(PhantomNode) == 88, "PhantomNode has more padding then expected"); using PhantomNodePair = std::pair; diff --git a/include/engine/routing_algorithms/routing_base.hpp b/include/engine/routing_algorithms/routing_base.hpp index 4afaae71f..f340c934a 100644 --- a/include/engine/routing_algorithms/routing_base.hpp +++ b/include/engine/routing_algorithms/routing_base.hpp @@ -86,7 +86,7 @@ void insertSourceInHeap(ManyToManyQueryHeap &heap, const PhantomNode &phantom_no heap.Insert(phantom_node.forward_segment_id.id, -phantom_node.GetForwardWeightPlusOffset(), {phantom_node.forward_segment_id.id, - -phantom_node.GetForwardDuration(), + -phantom_node.GetForwardDuration() + phantom_node.forward_duration_penalty, -phantom_node.GetForwardDistance()}); } if (phantom_node.IsValidReverseSource()) @@ -94,7 +94,7 @@ void insertSourceInHeap(ManyToManyQueryHeap &heap, const PhantomNode &phantom_no heap.Insert(phantom_node.reverse_segment_id.id, -phantom_node.GetReverseWeightPlusOffset(), {phantom_node.reverse_segment_id.id, - -phantom_node.GetReverseDuration(), + -phantom_node.GetReverseDuration() + phantom_node.reverse_duration_penalty, -phantom_node.GetReverseDistance()}); } } @@ -107,7 +107,7 @@ void insertTargetInHeap(ManyToManyQueryHeap &heap, const PhantomNode &phantom_no heap.Insert(phantom_node.forward_segment_id.id, phantom_node.GetForwardWeightPlusOffset(), {phantom_node.forward_segment_id.id, - phantom_node.GetForwardDuration(), + phantom_node.GetForwardDuration() + phantom_node.forward_duration_penalty, phantom_node.GetForwardDistance()}); } if (phantom_node.IsValidReverseTarget()) @@ -115,7 +115,7 @@ void insertTargetInHeap(ManyToManyQueryHeap &heap, const PhantomNode &phantom_no heap.Insert(phantom_node.reverse_segment_id.id, phantom_node.GetReverseWeightPlusOffset(), {phantom_node.reverse_segment_id.id, - phantom_node.GetReverseDuration(), + phantom_node.GetReverseDuration() + phantom_node.reverse_duration_penalty, phantom_node.GetReverseDistance()}); } } diff --git a/include/server/api/base_parameters_grammar.hpp b/include/server/api/base_parameters_grammar.hpp index adc4a704f..10484b26d 100644 --- a/include/server/api/base_parameters_grammar.hpp +++ b/include/server/api/base_parameters_grammar.hpp @@ -25,7 +25,7 @@ namespace { namespace ph = boost::phoenix; namespace qi = boost::spirit::qi; -} // namespace +} template struct no_trailing_dot_policy : qi::real_policies { @@ -222,8 +222,8 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar qi::symbols approach_type; qi::symbols snapping_type; }; -} // namespace api -} // namespace server -} // namespace osrm +} +} +} #endif diff --git a/include/util/typedefs.hpp b/include/util/typedefs.hpp index 5a3c7ba65..64af1c4cc 100644 --- a/include/util/typedefs.hpp +++ b/include/util/typedefs.hpp @@ -119,7 +119,8 @@ static const EdgeDistance INVALID_EDGE_DISTANCE = std::numeric_limits::max(); constexpr EdgeDuration INVALID_MINIMUM_STOPPAGE_PENALTY = std::numeric_limits::max(); constexpr EdgeDuration INVALID_MAXIMUM_STOPPAGE_PENALTY = std::numeric_limits::max(); -constexpr float MAXIMAL_ACCEL_DECEL_PENALIZABLE_SPEED_INVERSE = 40.f; // seconds/metre +constexpr double MINIMAL_ACCEL_DECEL_PENALIZABLE_SPEED = 10; // metres/second +constexpr double MAXIMAL_ACCEL_DECEL_PENALIZABLE_SPEED = 40; // metres/sec // FIXME the bitfields we use require a reduced maximal duration, this should be kept consistent // within the code base. For now we have to ensure that we don't case 30 bit to -1 and break any diff --git a/src/engine/plugins/table.cpp b/src/engine/plugins/table.cpp index 23490e007..be8b1a606 100644 --- a/src/engine/plugins/table.cpp +++ b/src/engine/plugins/table.cpp @@ -97,22 +97,6 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, std::vector estimated_pairs; - auto penalize = [¶ms](float speed, EdgeDuration duration) -> EdgeDuration { - // this is the penalty before adjusting for the length of the route - auto speed_ratio = speed * MAXIMAL_ACCEL_DECEL_PENALIZABLE_SPEED_INVERSE; - auto penalty = speed_ratio * (params.max_stoppage_penalty - params.min_stoppage_penalty) + - params.min_stoppage_penalty; - // how much distance would we cover if we took half of it at this speed - // scale the penalty based on this distance, if its small we dont get much - // of the penalty, buf if its large we get up to the whole penalty - // .05 is to scale the duration to second and cut it in half - // .005 is like / 200m, so the estimated distance as a ratio of 200m - // so you get the full penalty if your route is 200m or more - auto distance_ratio = (speed * duration * .05) * 0.008; - return static_cast( - (distance_ratio < 1.f ? distance_ratio * penalty : penalty) * 10.); - }; - // Scan table for null results - if any exist, replace with distance estimates if (params.fallback_speed != INVALID_FALLBACK_SPEED || params.scale_factor != 1 || (params.min_stoppage_penalty != INVALID_MINIMUM_STOPPAGE_PENALTY && @@ -124,20 +108,9 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, { const auto &table_index = row * num_destinations + column; BOOST_ASSERT(table_index < result_tables_pair.first.size()); - // apply an accel/deceleration penalty to the duration - if (result_tables_pair.first[table_index] != MAXIMAL_EDGE_DURATION && row != column) - { - const auto &source = - snapped_phantoms[params.sources.empty() ? row : params.sources[row]]; - const auto &destination = - snapped_phantoms[params.destinations.empty() ? column - : params.destinations[column]]; - result_tables_pair.first[table_index] += - penalize(source.speed_approximation, - result_tables_pair.first[table_index]) + - penalize(destination.speed_approximation, - result_tables_pair.first[table_index]); - } + // Zero out the diagonal + if (result_tables_pair.first[table_index] != MAXIMAL_EDGE_DURATION && row == column) + result_tables_pair.first[table_index] = 0; // Estimate null results based on fallback_speed (if valid) and distance if (params.fallback_speed != INVALID_FALLBACK_SPEED && params.fallback_speed > 0 && result_tables_pair.first[table_index] == MAXIMAL_EDGE_DURATION)