Open-Harbor/osrm-backend
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Make edge metrics strongly typed (#6421)

Browse Source 
This change takes the existing typedefs for weight, duration and
distance, and makes them proper types, using the existing Alias
functionality.

Primarily this is to prevent bugs where the metrics are switched,
but it also adds additional documentation. For example, it now
makes it clear (despite the naming of variables) that most of the
trip algorithm is running on the duration metric.

I've not made any changes to the casts performed between metrics
and numeric types, they now just more explicit.
This commit is contained in:
Michael Bell
2022-10-28 15:16:12 +01:00
committed by GitHub
parent 16685d0de9
commit 5d468f2897
69 changed files with 922 additions and 686 deletions
Download Patch File Download Diff File Expand all files Collapse all files
include/util/alias.hpp
+35
View File
@@ -28,6 +28,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#ifndef OSRM_UTIL_ALIAS_HPP
#define OSRM_UTIL_ALIAS_HPP
#include <boost/numeric/conversion/cast.hpp>
#include <functional>
#include <iostream>
#include <type_traits>
@@ -125,6 +126,40 @@ template <typename From, typename Tag> struct Alias final
}
};
template <typename ToAlias, typename FromAlias> inline ToAlias alias_cast(const FromAlias &from)
{
static_assert(std::is_arithmetic<typename FromAlias::value_type>::value,
"Alias From needs to be based on an arithmetic type");
static_assert(std::is_arithmetic<typename ToAlias::value_type>::value,
"Alias Other needs to be based on an arithmetic type");
return {static_cast<typename ToAlias::value_type>(
static_cast<const typename FromAlias::value_type>(from))};
}
template <typename ToNumeric, typename FromAlias> inline ToNumeric from_alias(const FromAlias &from)
{
static_assert(std::is_arithmetic<typename FromAlias::value_type>::value,
"Alias From needs to be based on an arithmetic type");
static_assert(std::is_arithmetic<ToNumeric>::value, "Numeric needs to be an arithmetic type");
return {static_cast<ToNumeric>(static_cast<const typename FromAlias::value_type>(from))};
}
template <typename ToAlias,
typename FromNumeric,
typename = std::enable_if_t<!std::is_same<ToAlias, FromNumeric>::value>>
inline ToAlias to_alias(const FromNumeric &from)
{
static_assert(std::is_arithmetic<FromNumeric>::value, "Numeric needs to be an arithmetic type");
static_assert(std::is_arithmetic<typename ToAlias::value_type>::value,
"Alias needs to be based on an arithmetic type");
return {static_cast<typename ToAlias::value_type>(from)};
}
// Sometimes metrics are stored either as bitfields or the alias itself.
// So we'll try to convert to alias without knowing which is the case.
// Therefore, we need this no-op overload, otherwise it will fail on the arithmetic requirement.
template <typename ToAlias> inline ToAlias to_alias(const ToAlias &from) { return from; }
template <typename From, typename Tag>
inline std::ostream &operator<<(std::ostream &stream, const Alias<From, Tag> &inst)
{
include/util/dist_table_wrapper.hpp
+2 -2
View File
@@ -34,7 +34,7 @@ template <typename T> class DistTableWrapper
std::size_t size() const { return table_.size(); }
EdgeWeight operator()(NodeID from, NodeID to) const
T operator()(NodeID from, NodeID to) const
{
BOOST_ASSERT_MSG(from < number_of_nodes_, "from ID is out of bound");
BOOST_ASSERT_MSG(to < number_of_nodes_, "to ID is out of bound");
@@ -46,7 +46,7 @@ template <typename T> class DistTableWrapper
return table_[index];
}
void SetValue(NodeID from, NodeID to, EdgeWeight value)
void SetValue(NodeID from, NodeID to, T value)
{
BOOST_ASSERT_MSG(from < number_of_nodes_, "from ID is out of bound");
BOOST_ASSERT_MSG(to < number_of_nodes_, "to ID is out of bound");
include/util/node_based_graph.hpp
+6 -6
View File
@@ -21,14 +21,14 @@ namespace util
struct NodeBasedEdgeData
{
NodeBasedEdgeData()
: weight(INVALID_EDGE_WEIGHT), duration(INVALID_EDGE_WEIGHT),
: weight(INVALID_EDGE_WEIGHT), duration(INVALID_EDGE_DURATION),
distance(INVALID_EDGE_DISTANCE), geometry_id({0, false}), reversed(false),
annotation_data(-1)
{
}
NodeBasedEdgeData(EdgeWeight weight,
EdgeWeight duration,
EdgeDuration duration,
EdgeDistance distance,
GeometryID geometry_id,
bool reversed,
@@ -40,7 +40,7 @@ struct NodeBasedEdgeData
}
EdgeWeight weight;
EdgeWeight duration;
EdgeDuration duration;
EdgeDistance distance;
GeometryID geometry_id;
bool reversed : 1;
@@ -88,9 +88,9 @@ NodeBasedDynamicGraphFromEdges(NodeID number_of_nodes,
output_edge.data.flags = input_edge.flags;
output_edge.data.annotation_data = input_edge.annotation_data;
BOOST_ASSERT(output_edge.data.weight > 0);
BOOST_ASSERT(output_edge.data.duration > 0);
BOOST_ASSERT(output_edge.data.distance >= 0);
BOOST_ASSERT(output_edge.data.weight > EdgeWeight{0});
BOOST_ASSERT(output_edge.data.duration > EdgeDuration{0});
BOOST_ASSERT(output_edge.data.distance >= EdgeDistance{0});
});
tbb::parallel_sort(edges_list.begin(), edges_list.end());
include/util/packed_vector.hpp
+34 -2
View File
@@ -83,19 +83,45 @@ inline T get_upper_half_value(WordT word,
}
template <typename WordT, typename T>
inline WordT set_lower_value(WordT word, WordT mask, std::uint8_t offset, T value)
inline WordT set_lower_value(WordT word,
WordT mask,
std::uint8_t offset,
T value,
typename std::enable_if_t<std::is_integral<T>::value> * = nullptr)
{
static_assert(std::is_unsigned<WordT>::value, "Only unsigned word types supported for now.");
return (word & ~mask) | ((static_cast<WordT>(value) << offset) & mask);
}
template <typename WordT, typename T>
inline WordT set_upper_value(WordT word, WordT mask, std::uint8_t offset, T value)
inline WordT set_upper_value(WordT word,
WordT mask,
std::uint8_t offset,
T value,
typename std::enable_if_t<std::is_integral<T>::value> * = nullptr)
{
static_assert(std::is_unsigned<WordT>::value, "Only unsigned word types supported for now.");
return (word & ~mask) | ((static_cast<WordT>(value) >> offset) & mask);
}
template <typename WordT, typename T>
inline WordT set_lower_value(
WordT word, WordT mask, std::uint8_t offset, T value, typename T::value_type * = nullptr)
{
static_assert(std::is_unsigned<WordT>::value, "Only unsigned word types supported for now.");
return (word & ~mask) |
((static_cast<WordT>(static_cast<typename T::value_type>(value)) << offset) & mask);
}
template <typename WordT, typename T>
inline WordT set_upper_value(
WordT word, WordT mask, std::uint8_t offset, T value, typename T::value_type * = nullptr)
{
static_assert(std::is_unsigned<WordT>::value, "Only unsigned word types supported for now.");
return (word & ~mask) |
((static_cast<WordT>(static_cast<typename T::value_type>(value)) >> offset) & mask);
}
inline bool compare_and_swap(uint64_t *ptr, uint64_t old_value, uint64_t new_value)
{
#if defined(_MSC_VER)
@@ -287,6 +313,12 @@ template <typename T, std::size_t Bits, storage::Ownership Ownership> class Pack
return &container == &other.container && internal_index == other.internal_index;
}
// FIXME: This is needed for tests on Boost ranges to correctly compare Alias values.
template <typename F, typename U> bool operator!=(const osrm::Alias<F, U> value) const
{
return container.get_value(internal_index) != value;
}
friend std::ostream &operator<<(std::ostream &os, const internal_reference &rhs)
{
return os << static_cast<T>(rhs);
include/util/typedefs.hpp
+50 -16
View File
@@ -48,7 +48,26 @@ struct osm_way_id
struct duplicated_node
{
};
struct edge_weight
{
};
struct edge_duration
{
};
struct edge_distance
{
};
struct segment_weight
{
};
struct segment_duration
{
};
struct turn_penalty
{
};
} // namespace tag
using OSMNodeID = osrm::Alias<std::uint64_t, tag::osm_node_id>;
// clang-tidy fires `bugprone-throw-keyword-missing` here for unknown reason
// NOLINTNEXTLINE(bugprone-throw-keyword-missing)
@@ -77,12 +96,13 @@ using EdgeID = std::uint32_t;
using NameID = std::uint32_t;
using AnnotationID = std::uint32_t;
using PackedGeometryID = std::uint32_t;
using EdgeWeight = std::int32_t;
using EdgeDuration = std::int32_t;
using EdgeDistance = float;
using SegmentWeight = std::uint32_t;
using SegmentDuration = std::uint32_t;
using TurnPenalty = std::int16_t; // turn penalty in 100ms units
using EdgeWeight = osrm::Alias<std::int32_t, tag::edge_weight>;
using EdgeDuration = osrm::Alias<std::int32_t, tag::edge_duration>;
using EdgeDistance = osrm::Alias<float, tag::edge_distance>;
using SegmentWeight = osrm::Alias<std::uint32_t, tag::segment_weight>;
using SegmentDuration = osrm::Alias<std::uint32_t, tag::segment_duration>;
using TurnPenalty = osrm::Alias<std::int16_t, tag::turn_penalty>; // turn penalty in 100ms units
static const std::size_t INVALID_INDEX = std::numeric_limits<std::size_t>::max();
@@ -109,16 +129,30 @@ static const NameID EMPTY_NAMEID = 0;
static const unsigned INVALID_COMPONENTID = 0;
static const std::size_t SEGMENT_WEIGHT_BITS = 22;
static const std::size_t SEGMENT_DURATION_BITS = 22;
static const SegmentWeight INVALID_SEGMENT_WEIGHT = (1u << SEGMENT_WEIGHT_BITS) - 1;
static const SegmentDuration INVALID_SEGMENT_DURATION = (1u << SEGMENT_DURATION_BITS) - 1;
static const SegmentWeight MAX_SEGMENT_WEIGHT = INVALID_SEGMENT_WEIGHT - 1;
static const SegmentDuration MAX_SEGMENT_DURATION = INVALID_SEGMENT_DURATION - 1;
static const EdgeWeight INVALID_EDGE_WEIGHT = std::numeric_limits<EdgeWeight>::max();
static const EdgeDuration MAXIMAL_EDGE_DURATION = std::numeric_limits<EdgeDuration>::max();
static const EdgeDistance MAXIMAL_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max();
static const TurnPenalty INVALID_TURN_PENALTY = std::numeric_limits<TurnPenalty>::max();
static const EdgeDistance INVALID_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max();
static const EdgeDistance INVALID_FALLBACK_SPEED = std::numeric_limits<EdgeDistance>::max();
static const SegmentWeight INVALID_SEGMENT_WEIGHT = SegmentWeight{(1u << SEGMENT_WEIGHT_BITS) - 1};
static const SegmentDuration INVALID_SEGMENT_DURATION =
SegmentDuration{(1u << SEGMENT_DURATION_BITS) - 1};
static const SegmentWeight MAX_SEGMENT_WEIGHT = INVALID_SEGMENT_WEIGHT - SegmentWeight{1};
static const SegmentDuration MAX_SEGMENT_DURATION = INVALID_SEGMENT_DURATION - SegmentDuration{1};
static const EdgeWeight INVALID_EDGE_WEIGHT =
EdgeWeight{std::numeric_limits<EdgeWeight::value_type>::max()};
static const EdgeDuration INVALID_EDGE_DURATION =
EdgeDuration{std::numeric_limits<EdgeDuration::value_type>::max()};
static const EdgeDistance INVALID_EDGE_DISTANCE =
EdgeDistance{std::numeric_limits<EdgeDistance::value_type>::max()};
static const TurnPenalty INVALID_TURN_PENALTY =
TurnPenalty{std::numeric_limits<TurnPenalty::value_type>::max()};
static const EdgeDistance INVALID_FALLBACK_SPEED =
EdgeDistance{std::numeric_limits<EdgeDistance::value_type>::max()};
// TODO: These are the same as the invalid values. Do we need both?
static const EdgeWeight MAXIMAL_EDGE_WEIGHT =
EdgeWeight{std::numeric_limits<EdgeWeight::value_type>::max()};
static const EdgeDuration MAXIMAL_EDGE_DURATION =
EdgeDuration{std::numeric_limits<EdgeDuration::value_type>::max()};
static const EdgeDistance MAXIMAL_EDGE_DISTANCE =
EdgeDistance{std::numeric_limits<EdgeDistance::value_type>::max()};
static const TurnPenalty MAXIMAL_TURN_PENALTY =
TurnPenalty{std::numeric_limits<TurnPenalty::value_type>::max()};
using DatasourceID = std::uint8_t;