From 5df4d96ed7e4f6983fb6dee972de5dc8cc2adf19 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Fri, 23 Feb 2024 22:09:58 +0100 Subject: [PATCH 01/10] tests still failing --- proto/CMakeLists.txt | 3 +- proto/api.proto | 9 ++- proto/options.proto | 2 +- src/proto_conversions.cc | 10 +++ src/thor/bidirectional_astar.cc | 28 ++++---- src/thor/costmatrix.cc | 25 ++++---- src/thor/dijkstras.cc | 5 +- src/thor/expansion_action.cc | 109 +++++++++++++++----------------- src/tyr/CMakeLists.txt | 3 +- src/tyr/serializers.cc | 3 + valhalla/proto_conversions.h | 1 + valhalla/thor/dijkstras.h | 2 +- valhalla/thor/matrixalgorithm.h | 2 +- valhalla/thor/pathalgorithm.h | 2 +- valhalla/tyr/serializers.h | 6 ++ 15 files changed, 118 insertions(+), 92 deletions(-) diff --git a/proto/CMakeLists.txt b/proto/CMakeLists.txt index f58a7c76a9..f7cfb1c898 100644 --- a/proto/CMakeLists.txt +++ b/proto/CMakeLists.txt @@ -12,7 +12,8 @@ set(protobuf_descriptors incidents.proto status.proto matrix.proto - isochrone.proto) + isochrone.proto + expansion.proto) if(ENABLE_DATA_TOOLS) # Only mjolnir needs the OSM PBF descriptors diff --git a/proto/api.proto b/proto/api.proto index 448713c309..19b1b72e94 100644 --- a/proto/api.proto +++ b/proto/api.proto @@ -9,6 +9,7 @@ import public "info.proto"; // statistics about the request, filled out by import public "status.proto"; // info for status endpoint import public "matrix.proto"; // the matrix results import public "isochrone.proto"; // the isochrone results +import public "expansion.proto"; // the expansion results message Api { // this is the request to the api @@ -20,11 +21,9 @@ message Api { Status status = 4; // status Matrix matrix = 5; // sources_to_targets Isochrone isochrone = 6; // isochrone - //TODO: isochrone - //TODO: matrix - //TODO: locate - //TODO: height - //TODO: expansion + Expansion expansion = 7; // expansion + //TODO: locate; + //TODO: height; // here we store a bit of info about what happened during request processing (stats/errors/warnings) Info info = 20; diff --git a/proto/options.proto b/proto/options.proto index f6c4ca3194..1c24898cfb 100644 --- a/proto/options.proto +++ b/proto/options.proto @@ -53,10 +53,10 @@ message PbfFieldSelector { bool status = 4; // /status bool matrix = 5; // sources_to_targets bool isochrone = 6; + bool expansion = 9; // TODO: enable these once we have objects for them // bool locate = 7; // bool height = 8; - // bool expansion = 9; } message AvoidEdge { diff --git a/src/proto_conversions.cc b/src/proto_conversions.cc index bcab5517f3..8f3fbd0252 100644 --- a/src/proto_conversions.cc +++ b/src/proto_conversions.cc @@ -440,4 +440,14 @@ travel_mode_type(const valhalla::DirectionsLeg_Maneuver& maneuver) { throw std::logic_error("Unknown travel mode"); } } + +const std::string& Expansion_EdgeStatus_Enum_Name(const Expansion_EdgeStatus status) { + static const std::unordered_map statuses{ + {Expansion_EdgeStatus_reached, "r"}, + {Expansion_EdgeStatus_settled, "s"}, + {Expansion_EdgeStatus_connected, "c"}, + }; + auto i = statuses.find(status); + return i == statuses.cend() ? empty_str : i->second; +} } // namespace valhalla diff --git a/src/thor/bidirectional_astar.cc b/src/thor/bidirectional_astar.cc index 61de6281f9..c71639f0fa 100644 --- a/src/thor/bidirectional_astar.cc +++ b/src/thor/bidirectional_astar.cc @@ -357,7 +357,7 @@ inline bool BidirectionalAStar::ExpandInner(baldr::GraphReader& graphreader, ? GraphId{} : (FORWARD ? edgelabels_forward_ : edgelabels_reverse_)[pred.predecessor()].edgeid(); expansion_callback_(graphreader, FORWARD ? meta.edge_id : opp_edge_id, prev_pred, - "bidirectional_astar", "r", newcost.secs, + "bidirectional_astar", Expansion_EdgeStatus_reached, newcost.secs, pred.path_distance() + meta.edge->length(), newcost.cost); } @@ -703,8 +703,9 @@ BidirectionalAStar::GetBestPath(valhalla::Location& origin, const auto prev_pred = fwd_pred.predecessor() == kInvalidLabel ? GraphId{} : edgelabels_forward_[fwd_pred.predecessor()].edgeid(); - expansion_callback_(graphreader, fwd_pred.edgeid(), prev_pred, "bidirectional_astar", "s", - fwd_pred.cost().secs, fwd_pred.path_distance(), fwd_pred.cost().cost); + expansion_callback_(graphreader, fwd_pred.edgeid(), prev_pred, "bidirectional_astar", + Expansion_EdgeStatus_settled, fwd_pred.cost().secs, + fwd_pred.path_distance(), fwd_pred.cost().cost); } // Prune path if predecessor is not a through edge or if the maximum @@ -750,8 +751,9 @@ BidirectionalAStar::GetBestPath(valhalla::Location& origin, const auto prev_pred = rev_pred.predecessor() == kInvalidLabel ? GraphId{} : edgelabels_reverse_[rev_pred.predecessor()].edgeid(); - expansion_callback_(graphreader, rev_pred.opp_edgeid(), prev_pred, "bidirectional_astar", "s", - rev_pred.cost().secs, rev_pred.path_distance(), rev_pred.cost().cost); + expansion_callback_(graphreader, rev_pred.opp_edgeid(), prev_pred, "bidirectional_astar", + Expansion_EdgeStatus_settled, rev_pred.cost().secs, + rev_pred.path_distance(), rev_pred.cost().cost); } // Prune path if predecessor is not a through edge @@ -865,8 +867,9 @@ bool BidirectionalAStar::SetForwardConnection(GraphReader& graphreader, const BD const auto prev_pred = pred.predecessor() == kInvalidLabel ? GraphId{} : edgelabels_forward_[pred.predecessor()].edgeid(); - expansion_callback_(graphreader, pred.edgeid(), prev_pred, "bidirectional_astar", "c", - pred.cost().secs, pred.path_distance(), pred.cost().cost); + expansion_callback_(graphreader, pred.edgeid(), prev_pred, "bidirectional_astar", + Expansion_EdgeStatus_connected, pred.cost().secs, pred.path_distance(), + pred.cost().cost); } return true; @@ -937,8 +940,9 @@ bool BidirectionalAStar::SetReverseConnection(GraphReader& graphreader, const BD const auto prev_pred = fwd_pred.predecessor() == kInvalidLabel ? GraphId{} : edgelabels_forward_[fwd_pred.predecessor()].edgeid(); - expansion_callback_(graphreader, fwd_edge_id, prev_pred, "bidirectional_astar", "c", - fwd_pred.cost().secs, fwd_pred.path_distance(), fwd_pred.cost().cost); + expansion_callback_(graphreader, fwd_edge_id, prev_pred, "bidirectional_astar", + Expansion_EdgeStatus_connected, fwd_pred.cost().secs, + fwd_pred.path_distance(), fwd_pred.cost().cost); } return true; @@ -1020,7 +1024,8 @@ void BidirectionalAStar::SetOrigin(GraphReader& graphreader, // setting this edge as reached if (expansion_callback_) { - expansion_callback_(graphreader, edgeid, GraphId{}, "bidirectional_astar", "r", cost.secs, + expansion_callback_(graphreader, edgeid, GraphId{}, "bidirectional_astar", + Expansion_EdgeStatus_reached, cost.secs, static_cast(edge.distance() + 0.5), cost.cost); } @@ -1116,7 +1121,8 @@ void BidirectionalAStar::SetDestination(GraphReader& graphreader, // setting this edge as reached, sending the opposing because this is the reverse tree if (expansion_callback_) { - expansion_callback_(graphreader, edgeid, GraphId{}, "bidirectional_astar", "r", cost.secs, + expansion_callback_(graphreader, edgeid, GraphId{}, "bidirectional_astar", + Expansion_EdgeStatus_reached, cost.secs, static_cast(edge.distance() + 0.5), cost.cost); } diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index 2386aa1229..920ab51d49 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -55,9 +55,9 @@ CostMatrix::CostMatrix(const boost::property_tree::ptree& config) max_reserved_locations_count_( config.get("max_reserved_locations_costmatrix", kMaxLocationReservation)), check_reverse_connections_(config.get("costmatrix_check_reverse_connection", false)), - access_mode_(kAutoAccess), - mode_(travel_mode_t::kDrive), locs_count_{0, 0}, locs_remaining_{0, 0}, - current_cost_threshold_(0), targets_{new ReachedMap}, sources_{new ReachedMap} { + access_mode_(kAutoAccess), mode_(travel_mode_t::kDrive), locs_count_{0, 0}, + locs_remaining_{0, 0}, current_cost_threshold_(0), targets_{new ReachedMap}, + sources_{new ReachedMap} { } CostMatrix::~CostMatrix() { @@ -546,8 +546,8 @@ bool CostMatrix::ExpandInner(baldr::GraphReader& graphreader, // setting this edge as reached if (expansion_callback_) { - expansion_callback_(graphreader, meta.edge_id, pred.edgeid(), "costmatrix", "r", newcost.secs, - pred_dist, newcost.cost); + expansion_callback_(graphreader, meta.edge_id, pred.edgeid(), "costmatrix", + Expansion_EdgeStatus_reached, newcost.secs, pred_dist, newcost.cost); } return !(pred.not_thru_pruning() && meta.edge->not_thru()); @@ -590,8 +590,9 @@ bool CostMatrix::Expand(const uint32_t index, if (expansion_callback_) { auto prev_pred = pred.predecessor() == kInvalidLabel ? GraphId{} : edgelabels[pred.predecessor()].edgeid(); - expansion_callback_(graphreader, pred.edgeid(), prev_pred, "costmatrix", "s", pred.cost().secs, - pred.path_distance(), pred.cost().cost); + expansion_callback_(graphreader, pred.edgeid(), prev_pred, "costmatrix", + Expansion_EdgeStatus_settled, pred.cost().secs, pred.path_distance(), + pred.cost().cost); } if (FORWARD) { @@ -818,8 +819,9 @@ void CostMatrix::CheckForwardConnections(const uint32_t source, auto prev_pred = pred.predecessor() == kInvalidLabel ? GraphId{} : edgelabel_[MATRIX_FORW][source][pred.predecessor()].edgeid(); - expansion_callback_(graphreader, pred.edgeid(), prev_pred, "costmatrix", "c", pred.cost().secs, - pred.path_distance(), pred.cost().cost); + expansion_callback_(graphreader, pred.edgeid(), prev_pred, "costmatrix", + Expansion_EdgeStatus_connected, pred.cost().secs, pred.path_distance(), + pred.cost().cost); } } @@ -916,8 +918,9 @@ void CostMatrix::CheckReverseConnections(const uint32_t target, auto prev_pred = rev_pred.predecessor() == kInvalidLabel ? GraphId{} : edgelabel_[MATRIX_REV][source][rev_pred.predecessor()].edgeid(); - expansion_callback_(graphreader, rev_pred.edgeid(), prev_pred, "costmatrix", "c", - rev_pred.cost().secs, rev_pred.path_distance(), rev_pred.cost().cost); + expansion_callback_(graphreader, rev_pred.edgeid(), prev_pred, "costmatrix", + Expansion_EdgeStatus_connected, rev_pred.cost().secs, + rev_pred.path_distance(), rev_pred.cost().cost); } } } diff --git a/src/thor/dijkstras.cc b/src/thor/dijkstras.cc index dae5ac87b1..4c1e7b7bb8 100644 --- a/src/thor/dijkstras.cc +++ b/src/thor/dijkstras.cc @@ -384,8 +384,9 @@ void Dijkstras::Compute(google::protobuf::RepeatedPtrField& const auto prev_pred = pred.predecessor() == kInvalidLabel ? GraphId{} : bdedgelabels_[pred.predecessor()].edgeid(); - expansion_callback_(graphreader, pred.edgeid(), prev_pred, "dijkstras", "s", pred.cost().secs, - pred.path_distance(), pred.cost().cost); + expansion_callback_(graphreader, pred.edgeid(), prev_pred, "dijkstras", + Expansion_EdgeStatus_settled, pred.cost().secs, pred.path_distance(), + pred.cost().cost); } } } diff --git a/src/thor/expansion_action.cc b/src/thor/expansion_action.cc index 476e4cc760..ecbd16c6b2 100644 --- a/src/thor/expansion_action.cc +++ b/src/thor/expansion_action.cc @@ -6,9 +6,53 @@ #include "midgard/logging.h" #include "midgard/polyline2.h" #include "midgard/util.h" +#include "tyr/serializers.h" using namespace rapidjson; using namespace valhalla::midgard; +using namespace valhalla::tyr; + +namespace { + +using namespace valhalla; + +void writeExpansionProgress(Expansion* expansion, + const baldr::GraphId& edgeid, + const baldr::GraphId& prev_edgeid, + const std::vector& shape, + const std::unordered_set exp_props, + const Expansion_EdgeStatus& status, + const float& duration, + const uint32_t& distance, + const float& cost) { + + auto* geom = expansion->add_geometries(); + // make the geom + for (const auto& p : shape) { + geom->add_coords(round(p.lng() * 1e6)); + geom->add_coords(round(p.lat() * 1e6)); + } + + // no properties asked for, don't collect any + if (!exp_props.size()) { + return; + } + + // make the properties + if (exp_props.count(Options_ExpansionProperties_duration)) + expansion->add_durations(static_cast(duration)); + if (exp_props.count(Options_ExpansionProperties_distance)) + expansion->add_distances(static_cast(distance)); + if (exp_props.count(Options_ExpansionProperties_cost)) + expansion->add_costs(static_cast(cost)); + if (exp_props.count(Options_ExpansionProperties_edge_status)) + expansion->add_edge_status(status); + if (exp_props.count(Options_ExpansionProperties_edge_id)) + expansion->add_edge_id(static_cast(duration)); + if (exp_props.count(Options_ExpansionProperties_pred_edge_id)) + expansion->add_pred_edge_id(static_cast(duration)); +} +} // namespace namespace valhalla { namespace thor { @@ -36,15 +80,16 @@ std::string thor_worker_t::expansion(Api& request) { writer("type", "FeatureCollection"); writer.start_array("features"); writer.set_precision(6); - + auto* expansion = request.mutable_expansion(); // a lambda that the path algorithm can call to add stuff to the dom // route and isochrone produce different GeoJSON properties std::string algo = ""; - auto track_expansion = [&writer, &opp_edges, &gen_factor, &skip_opps, &exp_props, + auto track_expansion = [&expansion, &opp_edges, &gen_factor, &skip_opps, &exp_props, &algo](baldr::GraphReader& reader, baldr::GraphId edgeid, baldr::GraphId prev_edgeid, const char* algorithm = nullptr, - std::string status = nullptr, const float duration = 0.f, - const uint32_t distance = 0, const float cost = 0.f) { + const Expansion_EdgeStatus status = Expansion_EdgeStatus_reached, + const float duration = 0.f, const uint32_t distance = 0, + const float cost = 0.f) { algo = algorithm; auto tile = reader.GetGraphTile(edgeid); @@ -73,51 +118,8 @@ std::string thor_worker_t::expansion(Api& request) { std::reverse(shape.begin(), shape.end()); Polyline2::Generalize(shape, gen_factor, {}, false); - writer.start_object(); // feature object - writer("type", "Feature"); - - writer.start_object("geometry"); - writer("type", "LineString"); - writer.start_array("coordinates"); - - // make the geom - for (const auto& p : shape) { - writer.start_array(); - writer(p.lng()); - writer(p.lat()); - writer.end_array(); - } - - writer.end_array(); // coordinates - writer.end_object(); // geometry - - writer.start_object("properties"); - // no properties asked for, don't collect any - if (!exp_props.size()) { - writer.end_object(); // properties - writer.end_object(); // feature - return; - } - - // make the properties - if (exp_props.count(Options_ExpansionProperties_duration)) { - writer("duration", static_cast(duration)); - } - if (exp_props.count(Options_ExpansionProperties_distance)) { - writer("distance", static_cast(distance)); - } - if (exp_props.count(Options_ExpansionProperties_cost)) { - writer("cost", static_cast(cost)); - } - if (exp_props.count(Options_ExpansionProperties_edge_status)) - writer("edge_status", status); - if (exp_props.count(Options_ExpansionProperties_edge_id)) - writer("edge_id", static_cast(edgeid)); - if (exp_props.count(Options_ExpansionProperties_pred_edge_id)) - writer("pred_edge_id", static_cast(prev_edgeid)); - - writer.end_object(); // properties - writer.end_object(); // feature + writeExpansionProgress(expansion, edgeid, prev_edgeid, shape, exp_props, status, duration, + distance, cost); }; // tell all the algorithms how to track expansion @@ -150,13 +152,6 @@ std::string thor_worker_t::expansion(Api& request) { // anyway } - // close the GeoJSON - writer.end_array(); // features - writer.start_object("properties"); - writer("algorithm", algo); - writer.end_object(); - writer.end_object(); // object - // tell all the algorithms to stop tracking the expansion for (auto* alg : std::vector{&multi_modal_astar, &timedep_forward, &timedep_reverse, &bidir_astar, &bss_astar}) { @@ -166,7 +161,7 @@ std::string thor_worker_t::expansion(Api& request) { isochrone_gen.SetInnerExpansionCallback(nullptr); // serialize it - return writer.get_buffer(); + return tyr::serializeExpansion(request, algo); } } // namespace thor diff --git a/src/tyr/CMakeLists.txt b/src/tyr/CMakeLists.txt index 197e14b53b..735f88e17b 100644 --- a/src/tyr/CMakeLists.txt +++ b/src/tyr/CMakeLists.txt @@ -8,7 +8,8 @@ set(sources route_serializer_osrm.cc route_summary_cache.cc serializers.cc - transit_available_serializer.cc) + transit_available_serializer.cc + expansion_serializer.cc) set(sources_with_warnings locate_serializer.cc diff --git a/src/tyr/serializers.cc b/src/tyr/serializers.cc index d70d0a08ec..5f5f7cb758 100644 --- a/src/tyr/serializers.cc +++ b/src/tyr/serializers.cc @@ -216,6 +216,9 @@ std::string serializePbf(Api& request) { case Options::isochrone: selection.set_isochrone(true); break; + case Options::expansion: + selection.set_expansion(true); + break; // should never get here, actions which dont have pbf yet return json default: throw std::logic_error("Requested action is not yet serializable as pbf"); diff --git a/valhalla/proto_conversions.h b/valhalla/proto_conversions.h index 7dda741eb9..768bdc5d14 100644 --- a/valhalla/proto_conversions.h +++ b/valhalla/proto_conversions.h @@ -546,6 +546,7 @@ const std::string& Location_Type_Enum_Name(const Location::Type t); const std::string& Location_SideOfStreet_Enum_Name(const Location::SideOfStreet s); bool Options_ExpansionProperties_Enum_Parse(const std::string& prop, Options::ExpansionProperties* a); bool Options_ExpansionAction_Enum_Parse(const std::string& action, Options::Action* a); +const std::string& Expansion_EdgeStatus_Enum_Name(const Expansion_EdgeStatus status); std::pair travel_mode_type(const valhalla::DirectionsLeg_Maneuver& maneuver); diff --git a/valhalla/thor/dijkstras.h b/valhalla/thor/dijkstras.h index 4fe12eea49..dd8cdbb0e0 100644 --- a/valhalla/thor/dijkstras.h +++ b/valhalla/thor/dijkstras.h @@ -74,7 +74,7 @@ class Dijkstras { const baldr::GraphId, const baldr::GraphId, const char*, - const char*, + const Expansion::EdgeStatus, float, uint32_t, float)>; diff --git a/valhalla/thor/matrixalgorithm.h b/valhalla/thor/matrixalgorithm.h index a697dcb7cf..f2f62b7ace 100644 --- a/valhalla/thor/matrixalgorithm.h +++ b/valhalla/thor/matrixalgorithm.h @@ -124,7 +124,7 @@ class MatrixAlgorithm { const baldr::GraphId, const baldr::GraphId, const char*, - const char*, + const Expansion_EdgeStatus, float, uint32_t, float)>; diff --git a/valhalla/thor/pathalgorithm.h b/valhalla/thor/pathalgorithm.h index 0c18bcf914..5b7d12bb4c 100644 --- a/valhalla/thor/pathalgorithm.h +++ b/valhalla/thor/pathalgorithm.h @@ -130,7 +130,7 @@ class PathAlgorithm { const baldr::GraphId, const baldr::GraphId, const char*, - const char*, + const Expansion_EdgeStatus, float, uint32_t, float)>; diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index ecb9fdb319..5bfc20ca42 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -17,6 +17,7 @@ #include #include #include +#include #include namespace valhalla { @@ -44,6 +45,11 @@ std::string serializeIsochrones(Api& request, bool polygons = true, bool show_locations = false); +/** + * Write GeoJSON from expansion pbf + */ +std::string serializeExpansion(Api& request, const std::string& algo); + /** * Turn heights and ranges into a height response * From 72cee8136d9464e09d90b5ae820adbb2f204e405 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Fri, 23 Feb 2024 22:13:28 +0100 Subject: [PATCH 02/10] serializer missing --- proto/expansion.proto | 26 ++++++++++ src/tyr/expansion_serializer.cc | 91 +++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 proto/expansion.proto create mode 100644 src/tyr/expansion_serializer.cc diff --git a/proto/expansion.proto b/proto/expansion.proto new file mode 100644 index 0000000000..00fce9023f --- /dev/null +++ b/proto/expansion.proto @@ -0,0 +1,26 @@ +syntax = "proto3"; + +option optimize_for = LITE_RUNTIME; +package valhalla; + +message Expansion { + + message Geometry { + repeated sint32 coords = 1; + } + + enum EdgeStatus { + connected = 0; + settled = 1; + reached = 2; + } + + repeated uint32 costs = 1; + repeated uint32 durations = 2; + repeated uint32 distances = 3; + repeated EdgeStatus edge_status = 4; + repeated uint32 edge_id = 5; + repeated uint32 pred_edge_id = 6; + + repeated Geometry geometries = 7; +} \ No newline at end of file diff --git a/src/tyr/expansion_serializer.cc b/src/tyr/expansion_serializer.cc new file mode 100644 index 0000000000..19cbb3d03d --- /dev/null +++ b/src/tyr/expansion_serializer.cc @@ -0,0 +1,91 @@ + +#include "baldr/rapidjson_utils.h" +#include "tyr/serializers.h" + +using namespace valhalla; +using namespace rapidjson; + +namespace valhalla { +namespace tyr { +std::string serializeExpansion(Api& request, const std::string& algo) { + if (request.options().format()) + return serializePbf(request); + + // form GeoJSON + writer_wrapper_t writer(1024 * 1024); + writer.start_object(); + writer("type", "FeatureCollection"); + writer.start_array("features"); + writer.set_precision(6); + + std::unordered_set exp_props; + for (const auto& prop : request.options().expansion_properties()) { + exp_props.insert(static_cast(prop)); + } + auto expansion = request.expansion(); + for (int i = 0; i < expansion.geometries().size(); ++i) { + // create features + writer.start_object(); // feature object + writer("type", "Feature"); + + writer.start_object("geometry"); + writer("type", "LineString"); + writer.start_array("coordinates"); + + // make the geom + auto geom = expansion.geometries(i); + for (int j = 0; j < geom.coords().size() - 1; j += 2) { + writer.start_array(); + writer(static_cast((geom.coords(j) / 1e6))); + writer(static_cast((geom.coords(j + 1) / 1e6))); + writer.end_array(); + } + + writer.end_array(); // coordinates + writer.end_object(); // geometry + + writer.start_object("properties"); + // no properties asked for, don't collect any + if (!exp_props.size()) { + writer.end_object(); // properties + writer.end_object(); // feature + writer.end_array(); // features + writer.start_object("properties"); + writer("algorithm", algo); + writer.end_object(); + writer.end_object(); // object + return writer.get_buffer(); + } + + // make the properties + if (exp_props.count(Options_ExpansionProperties_duration)) { + writer("duration", static_cast(expansion.durations(i))); + } + if (exp_props.count(Options_ExpansionProperties_distance)) { + writer("distance", static_cast(expansion.distances(i))); + } + if (exp_props.count(Options_ExpansionProperties_cost)) { + writer("cost", static_cast(expansion.costs(i))); + } + if (exp_props.count(Options_ExpansionProperties_edge_status)) + writer("edge_status", Expansion_EdgeStatus_Enum_Name(expansion.edge_status(i))); + if (exp_props.count(Options_ExpansionProperties_edge_id)) + writer("edge_id", static_cast(expansion.edge_id(i))); + if (exp_props.count(Options_ExpansionProperties_pred_edge_id)) + writer("pred_edge_id", static_cast(expansion.pred_edge_id(i))); + + writer.end_object(); // properties + writer.end_object(); // feature + } + + // close the GeoJSON + writer.end_array(); // features + writer.start_object("properties"); + writer("algorithm", algo); + writer.end_object(); + writer.end_object(); // object + + return writer.get_buffer(); +} +} // namespace tyr +} // namespace valhalla \ No newline at end of file From 756f5314c3579ba86ebda42ee9167423b7aee5fc Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 27 Feb 2024 16:19:16 +0100 Subject: [PATCH 03/10] wasnt serializing properly when no props passed --- src/thor/expansion_action.cc | 6 ------ src/tyr/expansion_serializer.cc | 9 ++------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/thor/expansion_action.cc b/src/thor/expansion_action.cc index ecbd16c6b2..f00563117c 100644 --- a/src/thor/expansion_action.cc +++ b/src/thor/expansion_action.cc @@ -74,12 +74,6 @@ std::string thor_worker_t::expansion(Api& request) { exp_props.insert(static_cast(prop)); } - // default the expansion geojson so its easy to add to as we go - writer_wrapper_t writer(1024 * 1024); - writer.start_object(); - writer("type", "FeatureCollection"); - writer.start_array("features"); - writer.set_precision(6); auto* expansion = request.mutable_expansion(); // a lambda that the path algorithm can call to add stuff to the dom // route and isochrone produce different GeoJSON properties diff --git a/src/tyr/expansion_serializer.cc b/src/tyr/expansion_serializer.cc index 19cbb3d03d..31609fb7cf 100644 --- a/src/tyr/expansion_serializer.cc +++ b/src/tyr/expansion_serializer.cc @@ -8,7 +8,7 @@ using namespace rapidjson; namespace valhalla { namespace tyr { std::string serializeExpansion(Api& request, const std::string& algo) { - if (request.options().format()) + if (request.options().format() == Options::Format::Options_Format_pbf) return serializePbf(request); // form GeoJSON @@ -49,12 +49,7 @@ std::string serializeExpansion(Api& request, const std::string& algo) { if (!exp_props.size()) { writer.end_object(); // properties writer.end_object(); // feature - writer.end_array(); // features - writer.start_object("properties"); - writer("algorithm", algo); - writer.end_object(); - writer.end_object(); // object - return writer.get_buffer(); + continue; } // make the properties From 7514a624f91205e52c91f56a5ba2c653a1069539 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 27 Feb 2024 16:23:53 +0100 Subject: [PATCH 04/10] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b65d254608..5a9da16a78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,7 +94,7 @@ * ADDED: Improved instructions for blind users [#3694](https://github.com/valhalla/valhalla/pull/3694) * FIXED: Fixed roundoff issue in Tiles Row and Col methods [#4585](https://github.com/valhalla/valhalla/pull/4585) * ADDED: isochrone proper polygon support & pbf output for isochrone [#4575](https://github.com/valhalla/valhalla/pull/4575) - + * ADDED: PBF support for expansion [#4614](https://github.com/valhalla/valhalla/pull/4614/) ## Release Date: 2023-05-11 Valhalla 3.4.0 * **Removed** * REMOVED: Docker image pushes to Dockerhub [#4033](https://github.com/valhalla/valhalla/pull/4033) From 56812fac39a7652c1ef7923e56d814d6f9caeb4c Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Tue, 27 Feb 2024 17:51:12 +0100 Subject: [PATCH 05/10] format --- src/thor/costmatrix.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index 920ab51d49..7fc94c959d 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -55,9 +55,9 @@ CostMatrix::CostMatrix(const boost::property_tree::ptree& config) max_reserved_locations_count_( config.get("max_reserved_locations_costmatrix", kMaxLocationReservation)), check_reverse_connections_(config.get("costmatrix_check_reverse_connection", false)), - access_mode_(kAutoAccess), mode_(travel_mode_t::kDrive), locs_count_{0, 0}, - locs_remaining_{0, 0}, current_cost_threshold_(0), targets_{new ReachedMap}, - sources_{new ReachedMap} { + access_mode_(kAutoAccess), + mode_(travel_mode_t::kDrive), locs_count_{0, 0}, locs_remaining_{0, 0}, + current_cost_threshold_(0), targets_{new ReachedMap}, sources_{new ReachedMap} { } CostMatrix::~CostMatrix() { From 76297848490b24b60bbd43f86589390614773bac Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 4 Apr 2024 16:27:56 +0200 Subject: [PATCH 06/10] proper e2e testing --- src/thor/costmatrix.cc | 14 ++--- src/tyr/matrix_serializer.cc | 5 ++ src/tyr/serializers.cc | 2 + src/worker.cc | 3 +- test/gurka/test_expansion.cc | 105 ++++++++++++++++++++++++++++------- 5 files changed, 102 insertions(+), 27 deletions(-) diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index 7771ef9a49..9dac9b6584 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -55,9 +55,9 @@ CostMatrix::CostMatrix(const boost::property_tree::ptree& config) max_reserved_locations_count_( config.get("max_reserved_locations_costmatrix", kMaxLocationReservation)), check_reverse_connections_(config.get("costmatrix_check_reverse_connection", false)), - access_mode_(kAutoAccess), - mode_(travel_mode_t::kDrive), locs_count_{0, 0}, locs_remaining_{0, 0}, - current_cost_threshold_(0), targets_{new ReachedMap}, sources_{new ReachedMap} { + access_mode_(kAutoAccess), mode_(travel_mode_t::kDrive), locs_count_{0, 0}, + locs_remaining_{0, 0}, current_cost_threshold_(0), targets_{new ReachedMap}, + sources_{new ReachedMap} { } CostMatrix::~CostMatrix() { @@ -828,10 +828,10 @@ void CostMatrix::CheckForwardConnections(const uint32_t source, if (expansion_callback_) { auto prev_pred = fwd_pred.predecessor() == kInvalidLabel ? GraphId{} - : edgelabel_[MATRIX_FORW][source][pred.predecessor()].edgeid(); - expansion_callback_(graphreader, pred.edgeid(), prev_pred, "costmatrix", - Expansion_EdgeStatus_connected, fwd_pred.cost().secs, fwd_pred.path_distance(), - fwd_pred.cost().cost); + : edgelabel_[MATRIX_FORW][source][fwd_pred.predecessor()].edgeid(); + expansion_callback_(graphreader, fwd_pred.edgeid(), prev_pred, "costmatrix", + Expansion_EdgeStatus_connected, fwd_pred.cost().secs, + fwd_pred.path_distance(), fwd_pred.cost().cost); } } diff --git a/src/tyr/matrix_serializer.cc b/src/tyr/matrix_serializer.cc index 14a7348c10..e520b1d928 100644 --- a/src/tyr/matrix_serializer.cc +++ b/src/tyr/matrix_serializer.cc @@ -242,6 +242,11 @@ namespace tyr { std::string serializeMatrix(Api& request) { double distance_scale = (request.options().units() == Options::miles) ? kMilePerMeter : kKmPerMeter; + + // dont bother serializing in case of /expansion request + if (request.options().action() == Options_Action_expansion) + return ""; + switch (request.options().format()) { case Options_Format_osrm: return osrm_serializers::serialize(request); diff --git a/src/tyr/serializers.cc b/src/tyr/serializers.cc index 5f5f7cb758..f9fa77931d 100644 --- a/src/tyr/serializers.cc +++ b/src/tyr/serializers.cc @@ -246,6 +246,8 @@ std::string serializePbf(Api& request) { request.clear_matrix(); if (!selection.isochrone()) request.clear_isochrone(); + if (!selection.expansion()) + request.clear_expansion(); // serialize the bytes auto bytes = request.SerializeAsString(); diff --git a/src/worker.cc b/src/worker.cc index fed19befb2..b914dfa1c0 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -688,7 +688,8 @@ void from_json(rapidjson::Document& doc, Options::Action action, Api& api) { Options::trace_attributes, Options::status, Options::sources_to_targets, - Options::isochrone}; + Options::isochrone, + Options::expansion}; // if its not a pbf supported action we reset to json if (pbf_actions.count(options.action()) == 0) { options.set_format(Options::json); diff --git a/test/gurka/test_expansion.cc b/test/gurka/test_expansion.cc index 5a42ea993a..864f4490ac 100644 --- a/test/gurka/test_expansion.cc +++ b/test/gurka/test_expansion.cc @@ -31,15 +31,16 @@ class ExpansionTest : public ::testing::TestWithParam> expansion_map = gurka::buildtiles(layout, ways, {}, {}, "test/data/expansion"); } - void check_result(const std::string& action, - const std::vector& waypoints, - bool skip_opps, - unsigned exp_feats, - const std::vector& props = {}) { - + valhalla::Api do_expansion_action(std::string* res, + bool skip_opps, + std::string action, + const std::vector& props, + const std::vector& waypoints, + const bool pbf) { std::unordered_map options = {{"/skip_opposites", skip_opps ? "1" : "0"}, - {"/action", action}}; + {"/action", action}, + {"/format", pbf ? "pbf" : "json"}}; for (uint8_t i = 0; i < props.size(); i++) { options.insert({{"/expansion_properties/" + std::to_string(i), props[i]}}); } @@ -48,16 +49,82 @@ class ExpansionTest : public ::testing::TestWithParam> } // get the response - std::string res; valhalla::Api api; if (action == "sources_to_targets") { api = gurka::do_action(Options::expansion, expansion_map, {waypoints[0]}, {waypoints[1]}, - "auto", options, {}, &res); + "auto", options, {}, res); } else { - api = gurka::do_action(Options::expansion, expansion_map, waypoints, "auto", options, {}, &res); + api = gurka::do_action(Options::expansion, expansion_map, waypoints, "auto", options, {}, res); + } + + return api; + } + void check_results(const std::string& action, + const std::vector& waypoints, + bool skip_opps, + unsigned exp_feats, + const std::vector& props = {}) { + check_result_json(action, waypoints, skip_opps, exp_feats, props); + check_result_pbf(action, waypoints, skip_opps, exp_feats, props); + } + void check_result_pbf(const std::string& action, + const std::vector& waypoints, + bool skip_opps, + unsigned exp_feats, + const std::vector& props) { + std::string res; + auto api = do_expansion_action(&res, skip_opps, action, props, waypoints, true); + + Api parsed_api; + parsed_api.ParseFromString(res); + + ASSERT_EQ(parsed_api.expansion().geometries_size(), exp_feats); + + bool edge_status = false; + bool distance = false; + bool duration = false; + bool pred_edge_id = false; + bool edge_id = false; + bool cost = false; + + const std::unordered_map prop_count; + for (const auto& prop : props) { + if (prop == "edge_status") { + edge_status = true; + } + if (prop == "distance") { + distance = true; + } + if (prop == "duration") { + duration = true; + } + if (prop == "pred_edge_id") { + pred_edge_id = true; + } + if (prop == "edge_id") { + edge_id = true; + } + if (prop == "cost") { + cost = true; + } } + ASSERT_EQ(parsed_api.expansion().geometries_size(), exp_feats); + ASSERT_EQ(parsed_api.expansion().edge_status_size(), edge_status ? exp_feats : 0); + ASSERT_EQ(parsed_api.expansion().distances_size(), distance ? exp_feats : 0); + ASSERT_EQ(parsed_api.expansion().durations_size(), duration ? exp_feats : 0); + ASSERT_EQ(parsed_api.expansion().pred_edge_id_size(), pred_edge_id ? exp_feats : 0); + ASSERT_EQ(parsed_api.expansion().edge_id_size(), edge_id ? exp_feats : 0); + ASSERT_EQ(parsed_api.expansion().costs_size(), cost ? exp_feats : 0); + } + void check_result_json(const std::string& action, + const std::vector& waypoints, + bool skip_opps, + unsigned exp_feats, + const std::vector& props) { + std::string res; + auto api = do_expansion_action(&res, skip_opps, action, props, waypoints, false); // get the MultiLineString feature rapidjson::Document res_doc; res_doc.Parse(res.c_str()); @@ -80,35 +147,35 @@ gurka::map ExpansionTest::expansion_map = {}; TEST_P(ExpansionTest, Isochrone) { // test Dijkstra expansion // 11 because there's a one-way - check_result("isochrone", {"A"}, false, 11, GetParam()); + check_results("isochrone", {"A"}, false, 11, GetParam()); } TEST_P(ExpansionTest, IsochroneNoOpposites) { // test Dijkstra expansion and skip collecting more expensive opposite edges - check_result("isochrone", {"A"}, true, 6, GetParam()); + check_results("isochrone", {"A"}, true, 6, GetParam()); } TEST_P(ExpansionTest, Routing) { // test AStar expansion - check_result("route", {"E", "H"}, false, 23, GetParam()); + check_results("route", {"E", "H"}, false, 23, GetParam()); } TEST_P(ExpansionTest, RoutingNoOpposites) { // test AStar expansion and no opposite edges - check_result("route", {"E", "H"}, true, 16, GetParam()); + check_results("route", {"E", "H"}, true, 16, GetParam()); } TEST_P(ExpansionTest, Matrix) { - check_result("sources_to_targets", {"E", "H"}, false, 48, GetParam()); + check_results("sources_to_targets", {"E", "H"}, false, 48, GetParam()); } TEST_P(ExpansionTest, MatrixNoOpposites) { - check_result("sources_to_targets", {"E", "H"}, true, 23, GetParam()); + check_results("sources_to_targets", {"E", "H"}, true, 23, GetParam()); } TEST_F(ExpansionTest, UnsupportedAction) { try { - check_result("status", {"E", "H"}, true, 16); + check_results("status", {"E", "H"}, true, 16); } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 144); } catch (...) { FAIL() << "Expected valhalla_exception_t."; }; @@ -116,7 +183,7 @@ TEST_F(ExpansionTest, UnsupportedAction) { TEST_F(ExpansionTest, UnsupportedPropType) { try { - check_result("route", {"E", "H"}, true, 16, {"foo", "bar"}); + check_results("route", {"E", "H"}, true, 16, {"foo", "bar"}); } catch (const valhalla_exception_t& err) { EXPECT_EQ(err.code, 168); } catch (...) { FAIL() << "Expected valhalla_exception_t."; }; @@ -141,4 +208,4 @@ INSTANTIATE_TEST_SUITE_P(ExpandPropsTest, std::vector{"distance", "duration", "pred_edge_id"}, std::vector{"edge_id", "cost"}, - std::vector{})); + std::vector{})); \ No newline at end of file From 7c78aff0aca24d6c09b7d68371d3cd13e52844a4 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Thu, 4 Apr 2024 16:37:00 +0200 Subject: [PATCH 07/10] format --- src/thor/costmatrix.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index 9dac9b6584..ae3691143c 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -55,9 +55,9 @@ CostMatrix::CostMatrix(const boost::property_tree::ptree& config) max_reserved_locations_count_( config.get("max_reserved_locations_costmatrix", kMaxLocationReservation)), check_reverse_connections_(config.get("costmatrix_check_reverse_connection", false)), - access_mode_(kAutoAccess), mode_(travel_mode_t::kDrive), locs_count_{0, 0}, - locs_remaining_{0, 0}, current_cost_threshold_(0), targets_{new ReachedMap}, - sources_{new ReachedMap} { + access_mode_(kAutoAccess), + mode_(travel_mode_t::kDrive), locs_count_{0, 0}, locs_remaining_{0, 0}, + current_cost_threshold_(0), targets_{new ReachedMap}, sources_{new ReachedMap} { } CostMatrix::~CostMatrix() { From 723925a33d4f239570d999378dfd65f4ab05def0 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Mon, 8 Apr 2024 08:44:46 +0200 Subject: [PATCH 08/10] fix small oversight --- src/thor/expansion_action.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/thor/expansion_action.cc b/src/thor/expansion_action.cc index f00563117c..6d399fb7f0 100644 --- a/src/thor/expansion_action.cc +++ b/src/thor/expansion_action.cc @@ -20,7 +20,7 @@ void writeExpansionProgress(Expansion* expansion, const baldr::GraphId& edgeid, const baldr::GraphId& prev_edgeid, const std::vector& shape, - const std::unordered_set exp_props, + const std::unordered_set& exp_props, const Expansion_EdgeStatus& status, const float& duration, const uint32_t& distance, @@ -48,9 +48,9 @@ void writeExpansionProgress(Expansion* expansion, if (exp_props.count(Options_ExpansionProperties_edge_status)) expansion->add_edge_status(status); if (exp_props.count(Options_ExpansionProperties_edge_id)) - expansion->add_edge_id(static_cast(duration)); + expansion->add_edge_id(static_cast(edgeid)); if (exp_props.count(Options_ExpansionProperties_pred_edge_id)) - expansion->add_pred_edge_id(static_cast(duration)); + expansion->add_pred_edge_id(static_cast(prev_edgeid)); } } // namespace From 46ca18e3e356440a779e11dd2b5e054e8fcdb3a6 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Mon, 8 Apr 2024 09:00:15 +0200 Subject: [PATCH 09/10] tidy --- src/tyr/expansion_serializer.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tyr/expansion_serializer.cc b/src/tyr/expansion_serializer.cc index 31609fb7cf..0401bbc522 100644 --- a/src/tyr/expansion_serializer.cc +++ b/src/tyr/expansion_serializer.cc @@ -33,7 +33,7 @@ std::string serializeExpansion(Api& request, const std::string& algo) { writer.start_array("coordinates"); // make the geom - auto geom = expansion.geometries(i); + const auto& geom = expansion.geometries(i); for (int j = 0; j < geom.coords().size() - 1; j += 2) { writer.start_array(); writer(static_cast((geom.coords(j) / 1e6))); From 15add0bcdd10036adf296fd345927c11e2680b68 Mon Sep 17 00:00:00 2001 From: Christian Beiwinkel Date: Mon, 8 Apr 2024 14:25:23 +0200 Subject: [PATCH 10/10] make packed=true explicit for repeated varint fields --- proto/expansion.proto | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/proto/expansion.proto b/proto/expansion.proto index 00fce9023f..e193e70bf0 100644 --- a/proto/expansion.proto +++ b/proto/expansion.proto @@ -6,7 +6,7 @@ package valhalla; message Expansion { message Geometry { - repeated sint32 coords = 1; + repeated sint32 coords = 1 [packed=true]; } enum EdgeStatus { @@ -15,12 +15,12 @@ message Expansion { reached = 2; } - repeated uint32 costs = 1; - repeated uint32 durations = 2; - repeated uint32 distances = 3; + repeated uint32 costs = 1 [packed=true]; + repeated uint32 durations = 2 [packed=true]; + repeated uint32 distances = 3 [packed=true]; repeated EdgeStatus edge_status = 4; - repeated uint32 edge_id = 5; - repeated uint32 pred_edge_id = 6; + repeated uint32 edge_id = 5 [packed=true]; + repeated uint32 pred_edge_id = 6 [packed=true]; repeated Geometry geometries = 7; } \ No newline at end of file