From ffcd93a1c08303dc1f2008c56548b810f4cfc0f0 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Sat, 20 May 2023 16:06:51 +0200 Subject: [PATCH 01/14] start with matrix pbf output; only specified the proto schema so far and replace MatrixType enum with protobuf definition --- proto/CMakeLists.txt | 3 +- proto/matrix.proto | 24 ++++++++++++++++ src/proto_conversions.cc | 3 ++ src/thor/matrix_action.cc | 26 +++++++++-------- src/thor/optimized_route_action.cc | 2 +- src/tyr/matrix_serializer.cc | 21 +++++++------- test/gurka/test_matrix_time_dependent.cc | 36 ++++++++++++------------ valhalla/proto_conversions.h | 3 ++ valhalla/thor/matrix_common.h | 11 ++++---- valhalla/tyr/serializers.h | 2 +- 10 files changed, 81 insertions(+), 50 deletions(-) create mode 100644 proto/matrix.proto diff --git a/proto/CMakeLists.txt b/proto/CMakeLists.txt index 14bbaee932..26d659e144 100644 --- a/proto/CMakeLists.txt +++ b/proto/CMakeLists.txt @@ -10,7 +10,8 @@ set(protobuf_descriptors transit.proto transit_fetch.proto incidents.proto - status.proto) + status.proto + matrix.proto) if(ENABLE_DATA_TOOLS) # Only mjolnir needs the OSM PBF descriptors diff --git a/proto/matrix.proto b/proto/matrix.proto new file mode 100644 index 0000000000..2d03322678 --- /dev/null +++ b/proto/matrix.proto @@ -0,0 +1,24 @@ +syntax = "proto3"; +option optimize_for = LITE_RUNTIME; +package valhalla; +import public "common.proto"; + +message Matrix { + enum Algorithm { + TimeDistanceMatrix = 0; + CostMatrix = 1; + } + + message TimeDistance { + double distance = 1; + double time = 2; + uint32 from_index = 3; + uint32 to_index = 4; + string date_time = 5; + } + + repeated Location sources = 1; + repeated Location targets = 2; + repeated TimeDistance time_distances = 3; + Algorithm algorithm = 4; +} diff --git a/src/proto_conversions.cc b/src/proto_conversions.cc index b16473cf16..5005c8bc79 100644 --- a/src/proto_conversions.cc +++ b/src/proto_conversions.cc @@ -4,6 +4,9 @@ using namespace valhalla; namespace valhalla { +std::string MatrixAlgoToString(const valhalla::Matrix::Algorithm algo) { + return algo == valhalla::Matrix::CostMatrix ? "costmatrix" : "timedistancematrix"; +}; std::string incidentTypeToString(const valhalla::IncidentsTile::Metadata::Type& incident_type) { switch (incident_type) { diff --git a/src/thor/matrix_action.cc b/src/thor/matrix_action.cc index 0fb0b1b219..10caea3a7e 100644 --- a/src/thor/matrix_action.cc +++ b/src/thor/matrix_action.cc @@ -50,10 +50,10 @@ std::string thor_worker_t::matrix(Api& request) { mode_costing, mode, max_matrix_distance.find(costing)->second, options.matrix_locations()); - return tyr::serializeMatrix(request, time_distances, distance_scale, MatrixType::TimeDist); + return tyr::serializeMatrix(request, time_distances, distance_scale, Matrix::TimeDistanceMatrix); } - MatrixType matrix_type = MatrixType::Cost; + Matrix::Algorithm matrix_algo = Matrix::CostMatrix; switch (source_to_target_algorithm) { case SELECT_OPTIMAL: // TODO - Do further performance testing to pick the best algorithm for the job @@ -64,11 +64,11 @@ std::string thor_worker_t::matrix(Api& request) { // exceeds some threshold if (options.sources().size() <= kCostMatrixThreshold || options.targets().size() <= kCostMatrixThreshold) { - matrix_type = MatrixType::TimeDist; + matrix_algo = Matrix::TimeDistanceMatrix; } break; case travel_mode_t::kPublicTransit: - matrix_type = MatrixType::TimeDist; + matrix_algo = Matrix::TimeDistanceMatrix; break; default: break; @@ -77,31 +77,33 @@ std::string thor_worker_t::matrix(Api& request) { case COST_MATRIX: break; case TIME_DISTANCE_MATRIX: - matrix_type = MatrixType::TimeDist; + matrix_algo = Matrix::TimeDistanceMatrix; break; } // similar to routing: prefer the exact unidirectional algo if not requested otherwise // don't use matrix_type, we only need it to set the right warnings for what will be used bool has_time = - check_matrix_time(request, - options.prioritize_bidirectional() ? MatrixType::Cost : MatrixType::TimeDist); + check_matrix_time(request, options.prioritize_bidirectional() ? Matrix::CostMatrix + : Matrix::TimeDistanceMatrix); if (has_time && !options.prioritize_bidirectional() && source_to_target_algorithm != COST_MATRIX) { - return tyr::serializeMatrix(request, timedistancematrix(), distance_scale, MatrixType::TimeDist); + return tyr::serializeMatrix(request, timedistancematrix(), distance_scale, + Matrix::TimeDistanceMatrix); } else if (has_time && options.prioritize_bidirectional() && source_to_target_algorithm != TIME_DISTANCE_MATRIX) { - return tyr::serializeMatrix(request, costmatrix(has_time), distance_scale, MatrixType::Cost); - } else if (matrix_type == MatrixType::Cost) { + return tyr::serializeMatrix(request, costmatrix(has_time), distance_scale, Matrix::CostMatrix); + } else if (matrix_algo == Matrix::CostMatrix) { // if this happens, the server config only allows for timedist matrix if (has_time && !options.prioritize_bidirectional()) { add_warning(request, 301); } - return tyr::serializeMatrix(request, costmatrix(has_time), distance_scale, MatrixType::Cost); + return tyr::serializeMatrix(request, costmatrix(has_time), distance_scale, Matrix::CostMatrix); } else { if (has_time && options.prioritize_bidirectional()) { add_warning(request, 300); } - return tyr::serializeMatrix(request, timedistancematrix(), distance_scale, MatrixType::TimeDist); + return tyr::serializeMatrix(request, timedistancematrix(), distance_scale, + Matrix::TimeDistanceMatrix); } } } // namespace thor diff --git a/src/thor/optimized_route_action.cc b/src/thor/optimized_route_action.cc index 2525a24c39..24cedb9412 100644 --- a/src/thor/optimized_route_action.cc +++ b/src/thor/optimized_route_action.cc @@ -31,7 +31,7 @@ void thor_worker_t::optimized_route(Api& request) { std::vector td = costmatrix.SourceToTarget(*options.mutable_sources(), *options.mutable_targets(), *reader, mode_costing, mode, max_matrix_distance.find(costing)->second, - check_matrix_time(request, MatrixType::Cost), + check_matrix_time(request, Matrix::CostMatrix), options.date_time_type() == Options::invariant); // Return an error if any locations are totally unreachable diff --git a/src/tyr/matrix_serializer.cc b/src/tyr/matrix_serializer.cc index 53abe98c0f..ddfaf24a9e 100644 --- a/src/tyr/matrix_serializer.cc +++ b/src/tyr/matrix_serializer.cc @@ -51,7 +51,7 @@ namespace osrm_serializers { json::MapPtr serialize(const Api& request, const std::vector& time_distances, double distance_scale, - MatrixType matrix_type) { + const std::string& matrix_algo) { auto json = json::map({}); auto time = json::array({}); auto distance = json::array({}); @@ -72,8 +72,7 @@ json::MapPtr serialize(const Api& request, } json->emplace("durations", time); json->emplace("distances", distance); - json->emplace("algorithm", - std::string(matrix_type == MatrixType::Cost ? "costmatrix" : "timedistancematrix")); + json->emplace("algorithm", matrix_algo); return json; } } // namespace osrm_serializers @@ -127,7 +126,7 @@ json::ArrayPtr serialize_row(const std::vector& tds, json::MapPtr serialize(const Api& request, const std::vector& time_distances, double distance_scale, - MatrixType matrix_type) { + const std::string& matrix_algo) { auto json = json::map({}); const auto& options = request.options(); @@ -162,8 +161,7 @@ json::MapPtr serialize(const Api& request, } json->emplace("units", Options_Units_Enum_Name(options.units())); - json->emplace("algorithm", - std::string(matrix_type == MatrixType::Cost ? "costmatrix" : "timedistancematrix")); + json->emplace("algorithm", matrix_algo); if (options.has_id_case()) { json->emplace("id", options.id()); @@ -184,12 +182,13 @@ namespace tyr { std::string serializeMatrix(const Api& request, const std::vector& time_distances, double distance_scale, - MatrixType matrix_type) { + const Matrix::Algorithm& matrix_algo) { - auto json = - request.options().format() == Options::osrm - ? osrm_serializers::serialize(request, time_distances, distance_scale, matrix_type) - : valhalla_serializers::serialize(request, time_distances, distance_scale, matrix_type); + auto json = request.options().format() == Options::osrm + ? osrm_serializers::serialize(request, time_distances, distance_scale, + MatrixAlgoToString(matrix_algo)) + : valhalla_serializers::serialize(request, time_distances, distance_scale, + MatrixAlgoToString(matrix_algo)); std::stringstream ss; ss << *json; diff --git a/test/gurka/test_matrix_time_dependent.cc b/test/gurka/test_matrix_time_dependent.cc index e4b933f7dd..16de58fbff 100644 --- a/test/gurka/test_matrix_time_dependent.cc +++ b/test/gurka/test_matrix_time_dependent.cc @@ -30,7 +30,7 @@ void update_traffic_on_edges(baldr::GraphReader& reader, void check_matrix(const rapidjson::Document& result, const std::vector& exp_dists, bool valid_traffic, - MatrixType matrix_type) { + Matrix::Algorithm matrix_type) { size_t i = 0; for (const auto& origin_row : result["sources_to_targets"].GetArray()) { auto origin_td = origin_row.GetArray(); @@ -46,7 +46,7 @@ void check_matrix(const rapidjson::Document& result, } } const std::string algo = result["algorithm"].GetString(); - const std::string exp_algo = matrix_type == MatrixType::Cost ? "costmatrix" : "timedistancematrix"; + const std::string exp_algo = MatrixAlgoToString(matrix_type); EXPECT_EQ(algo, exp_algo); } } // namespace @@ -124,7 +124,7 @@ TEST_F(MatrixTest, MatrixNoTraffic) { res_doc.Parse(res.c_str()); // we expect to take the motorways, residential path is 2.8f - check_matrix(res_doc, {0.0f, 3.2f, 3.2f, 0.0f}, false, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 3.2f, 3.2f, 0.0f}, false, Matrix::CostMatrix); } TEST_F(MatrixTest, TDMatrixWithLiveTraffic) { @@ -138,7 +138,7 @@ TEST_F(MatrixTest, TDMatrixWithLiveTraffic) { options, nullptr, &res); rapidjson::Document res_doc; res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 2.8f, 2.8f, 0.0f}, true, MatrixType::TimeDist); + check_matrix(res_doc, {0.0f, 2.8f, 2.8f, 0.0f}, true, Matrix::TimeDistanceMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // forward tree, date_time on the locations, 2nd location has pointless date_time @@ -150,7 +150,7 @@ TEST_F(MatrixTest, TDMatrixWithLiveTraffic) { nullptr, &res); res_doc.Parse(res.c_str()); // the second origin can't respect time (no historical data) - check_matrix(res_doc, {0.0f, 2.8f, 3.2f, 0.0f}, false, MatrixType::TimeDist); + check_matrix(res_doc, {0.0f, 2.8f, 3.2f, 0.0f}, false, Matrix::TimeDistanceMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // TODO: there's still a bug in CostMatrix which chooses the wrong correlated edges: @@ -160,7 +160,7 @@ TEST_F(MatrixTest, TDMatrixWithLiveTraffic) { result = gurka::do_action(Options::sources_to_targets, map, {"E", "L"}, {"L"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {2.8f, 0.0f}, false, MatrixType::Cost); + check_matrix(res_doc, {2.8f, 0.0f}, false, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 1); ASSERT_EQ(result.info().warnings(0).code(), 201); @@ -170,7 +170,7 @@ TEST_F(MatrixTest, TDMatrixWithLiveTraffic) { result = gurka::do_action(Options::sources_to_targets, map, {"1"}, {"1", "2"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 0.2f}, true, MatrixType::TimeDist); + check_matrix(res_doc, {0.0f, 0.2f}, true, Matrix::TimeDistanceMatrix); ASSERT_EQ(result.info().warnings().size(), 0); } @@ -186,7 +186,7 @@ TEST_F(MatrixTest, CostMatrixWithLiveTraffic) { options, nullptr, &res); rapidjson::Document res_doc; res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 2.8f, 2.8f, 0.0f}, true, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 2.8f, 2.8f, 0.0f}, true, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // forward tree, date_time on the locations, 2nd location has pointless date_time @@ -199,7 +199,7 @@ TEST_F(MatrixTest, CostMatrixWithLiveTraffic) { nullptr, &res); res_doc.Parse(res.c_str()); // the second origin can't respect time (no historical data) - check_matrix(res_doc, {0.0f, 2.8f, 3.2f, 0.0f}, false, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 2.8f, 3.2f, 0.0f}, false, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // forward tree, source & target within a single edge @@ -210,7 +210,7 @@ TEST_F(MatrixTest, CostMatrixWithLiveTraffic) { result = gurka::do_action(Options::sources_to_targets, map, {"1"}, {"1", "2"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 0.2f}, true, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 0.2f}, true, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // bidir matrix allows less targets than sources and date_time on the sources @@ -222,7 +222,7 @@ TEST_F(MatrixTest, CostMatrixWithLiveTraffic) { result = gurka::do_action(Options::sources_to_targets, map, {"E", "L"}, {"E"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 2.8f}, true, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 2.8f}, true, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // we don't support date_time on the targets @@ -236,7 +236,7 @@ TEST_F(MatrixTest, CostMatrixWithLiveTraffic) { // TODO: there's still a bug in CostMatrix which chooses the wrong correlated edges: // https://github.com/valhalla/valhalla/issues/3803 // this should really take the longer route since it's not using traffic here - check_matrix(res_doc, {0.0f, 2.8f}, false, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 2.8f}, false, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 1); ASSERT_EQ(result.info().warnings(0).code(), 206); } @@ -268,7 +268,7 @@ TEST_F(MatrixTest, TDSources) { auto result = gurka::do_action(Options::sources_to_targets, map, {"E", "L"}, {"E"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 3.2f}, true, MatrixType::TimeDist); + check_matrix(res_doc, {0.0f, 3.2f}, true, Matrix::TimeDistanceMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // more targets than sources with date_time.type = 2 are disallowed @@ -277,7 +277,7 @@ TEST_F(MatrixTest, TDSources) { result = gurka::do_action(Options::sources_to_targets, map, {"E"}, {"E", "L"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 3.2f}, false, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 3.2f}, false, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().Get(0).code(), 202); ASSERT_EQ(result.info().warnings().size(), 1); @@ -289,7 +289,7 @@ TEST_F(MatrixTest, TDSources) { result = gurka::do_action(Options::sources_to_targets, map, {"E", "L"}, {"E"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 3.2f}, false, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 3.2f}, false, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().Get(0).code(), 201); ASSERT_EQ(result.info().warnings().size(), 1); } @@ -303,7 +303,7 @@ TEST_F(MatrixTest, TDTargets) { auto result = gurka::do_action(Options::sources_to_targets, map, {"E"}, {"E", "L"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 2.8f}, true, MatrixType::TimeDist); + check_matrix(res_doc, {0.0f, 2.8f}, true, Matrix::TimeDistanceMatrix); ASSERT_EQ(result.info().warnings().size(), 0); // more sources than targets with date_time.type = 1 are disallowed @@ -312,7 +312,7 @@ TEST_F(MatrixTest, TDTargets) { result = gurka::do_action(Options::sources_to_targets, map, {"E", "L"}, {"E"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 3.2f}, false, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 3.2f}, false, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 1); ASSERT_EQ(result.info().warnings().Get(0).code(), 201); @@ -322,7 +322,7 @@ TEST_F(MatrixTest, TDTargets) { result = gurka::do_action(Options::sources_to_targets, map, {"E"}, {"E", "L"}, "auto", options, nullptr, &res); res_doc.Parse(res.c_str()); - check_matrix(res_doc, {0.0f, 3.2f}, false, MatrixType::Cost); + check_matrix(res_doc, {0.0f, 3.2f}, false, Matrix::CostMatrix); ASSERT_EQ(result.info().warnings().size(), 1); ASSERT_EQ(result.info().warnings().Get(0).code(), 202); } diff --git a/valhalla/proto_conversions.h b/valhalla/proto_conversions.h index 65cecb4d01..4ab9f071b1 100644 --- a/valhalla/proto_conversions.h +++ b/valhalla/proto_conversions.h @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace valhalla { @@ -246,6 +247,8 @@ inline TripLeg_Use GetTripLegUse(const baldr::Use use) { } } +// matrix algo to string +std::string MatrixAlgoToString(const valhalla::Matrix::Algorithm algo); // Get the string representing the incident-type std::string incidentTypeToString(const valhalla::IncidentsTile::Metadata::Type& incident_type); // Get the string representing the incident-Impact diff --git a/valhalla/thor/matrix_common.h b/valhalla/thor/matrix_common.h index e7ecf499c5..efd7b4d5ec 100644 --- a/valhalla/thor/matrix_common.h +++ b/valhalla/thor/matrix_common.h @@ -8,6 +8,7 @@ #include #include +#include "proto/matrix.pb.h" #include #include #include @@ -22,8 +23,6 @@ namespace valhalla { namespace thor { -enum class MatrixType : bool { TimeDist, Cost }; - // Default for time distance matrix is to find all locations constexpr uint32_t kAllLocations = std::numeric_limits::max(); constexpr float kMaxCost = 99999999.9999f; @@ -106,13 +105,13 @@ inline std::string get_date_time(const std::string& origin_dt, // return true if any location had a valid time set // return false if it doesn't make sense computationally and add warnings accordingly -inline bool check_matrix_time(Api& request, const MatrixType type) { +inline bool check_matrix_time(Api& request, const Matrix::Algorithm algo) { const auto& options = request.options(); bool less_sources = options.sources().size() <= options.targets().size(); for (const auto& source : options.sources()) { if (!source.date_time().empty()) { - if (!less_sources && type == MatrixType::TimeDist) { + if (!less_sources && algo == Matrix::TimeDistanceMatrix) { add_warning(request, 201); return false; } @@ -121,10 +120,10 @@ inline bool check_matrix_time(Api& request, const MatrixType type) { } for (const auto& target : options.targets()) { if (!target.date_time().empty()) { - if (less_sources && type == MatrixType::TimeDist) { + if (less_sources && algo == Matrix::TimeDistanceMatrix) { add_warning(request, 202); return false; - } else if (type == MatrixType::Cost) { + } else if (algo == Matrix::CostMatrix) { add_warning(request, 206); return false; } diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index 30cd1b4f4f..1e188fb987 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -33,7 +33,7 @@ std::string serializeDirections(Api& request); std::string serializeMatrix(const Api& request, const std::vector& time_distances, double distance_scale, - thor::MatrixType matrix_type); + const Matrix::Algorithm& matrix_algo); /** * Turn grid data contours into geojson From 6331656d8e7a179af9e429b869a45716ee062b94 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Sat, 20 May 2023 17:39:37 +0200 Subject: [PATCH 02/14] only change the matrix functions signatures to accept an Api object rather than sources/targets --- src/thor/costmatrix.cc | 19 ++++----- src/thor/matrix_action.cc | 14 +++---- src/thor/optimized_route_action.cc | 4 +- src/thor/timedistancebssmatrix.cc | 46 ++++++++++----------- src/thor/timedistancematrix.cc | 58 +++++++++++++++------------ valhalla/thor/costmatrix.h | 16 ++++---- valhalla/thor/timedistancebssmatrix.h | 37 ++++++++--------- valhalla/thor/timedistancematrix.h | 46 ++++++++++----------- 8 files changed, 119 insertions(+), 121 deletions(-) diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index edd794aa45..e5dac1f53c 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -119,15 +119,13 @@ void CostMatrix::clear() { // Form a time distance matrix from the set of source locations // to the set of target locations. -std::vector CostMatrix::SourceToTarget( - google::protobuf::RepeatedPtrField& source_location_list, - google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t mode, - const float max_matrix_distance, - const bool has_time, - const bool invariant) { +std::vector CostMatrix::SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t mode, + const float max_matrix_distance, + const bool has_time, + const bool invariant) { LOG_INFO("matrix::CostMatrix"); @@ -136,6 +134,9 @@ std::vector CostMatrix::SourceToTarget( costing_ = mode_costing[static_cast(mode_)]; access_mode_ = costing_->access_mode(); + auto& source_location_list = *request.mutable_options()->mutable_sources(); + auto& target_location_list = *request.mutable_options()->mutable_targets(); + current_cost_threshold_ = GetCostThreshold(max_matrix_distance); auto time_infos = SetOriginTimes(source_location_list, graphreader); diff --git a/src/thor/matrix_action.cc b/src/thor/matrix_action.cc index 10caea3a7e..bf5e4d4afa 100644 --- a/src/thor/matrix_action.cc +++ b/src/thor/matrix_action.cc @@ -32,22 +32,20 @@ std::string thor_worker_t::matrix(Api& request) { // lambdas to do the real work auto costmatrix = [&](const bool has_time) { - return costmatrix_.SourceToTarget(*options.mutable_sources(), *options.mutable_targets(), *reader, - mode_costing, mode, max_matrix_distance.find(costing)->second, - has_time, options.date_time_type() == Options::invariant); + return costmatrix_.SourceToTarget(request, *reader, mode_costing, mode, + max_matrix_distance.find(costing)->second, has_time, + options.date_time_type() == Options::invariant); }; auto timedistancematrix = [&]() { - return time_distance_matrix_.SourceToTarget(*options.mutable_sources(), - *options.mutable_targets(), *reader, mode_costing, - mode, max_matrix_distance.find(costing)->second, + return time_distance_matrix_.SourceToTarget(request, *reader, mode_costing, mode, + max_matrix_distance.find(costing)->second, options.matrix_locations(), options.date_time_type() == Options::invariant); }; if (costing == "bikeshare") { const auto& time_distances = - time_distance_bss_matrix_.SourceToTarget(options.sources(), options.targets(), *reader, - mode_costing, mode, + time_distance_bss_matrix_.SourceToTarget(request, *reader, mode_costing, mode, max_matrix_distance.find(costing)->second, options.matrix_locations()); return tyr::serializeMatrix(request, time_distances, distance_scale, Matrix::TimeDistanceMatrix); diff --git a/src/thor/optimized_route_action.cc b/src/thor/optimized_route_action.cc index 24cedb9412..ae0e9e4a61 100644 --- a/src/thor/optimized_route_action.cc +++ b/src/thor/optimized_route_action.cc @@ -29,8 +29,8 @@ void thor_worker_t::optimized_route(Api& request) { // Use CostMatrix to find costs from each location to every other location CostMatrix costmatrix; std::vector td = - costmatrix.SourceToTarget(*options.mutable_sources(), *options.mutable_targets(), *reader, - mode_costing, mode, max_matrix_distance.find(costing)->second, + costmatrix.SourceToTarget(request, *reader, mode_costing, mode, + max_matrix_distance.find(costing)->second, check_matrix_time(request, Matrix::CostMatrix), options.date_time_type() == Options::invariant); diff --git a/src/thor/timedistancebssmatrix.cc b/src/thor/timedistancebssmatrix.cc index 84b5ffb7a5..179e2062fa 100644 --- a/src/thor/timedistancebssmatrix.cc +++ b/src/thor/timedistancebssmatrix.cc @@ -182,15 +182,16 @@ void TimeDistanceBSSMatrix::Expand(GraphReader& graphreader, // Calculate time and distance from one origin location to many destination // locations. template -std::vector TimeDistanceBSSMatrix::ComputeMatrix( - const google::protobuf::RepeatedPtrField& source_location_list, - const google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations) { +std::vector TimeDistanceBSSMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations) { // Run a series of one to many calls and concatenate the results. - const auto& origins = FORWARD ? source_location_list : target_location_list; - const auto& destinations = FORWARD ? target_location_list : source_location_list; + + auto& origins = FORWARD ? *request.mutable_options()->mutable_sources() + : *request.mutable_options()->mutable_targets(); + auto& destinations = FORWARD ? *request.mutable_options()->mutable_targets() + : *request.mutable_options()->mutable_sources(); // Construct adjacency list, edge status, and done set. Set bucket size and // cost range based on DynamicCost. @@ -221,7 +222,7 @@ std::vector TimeDistanceBSSMatrix::ComputeMatrix( uint32_t predindex = adjacencylist_.pop(); if (predindex == kInvalidLabel) { // Can not expand any further... - one_to_many = FormTimeDistanceMatrix(); + one_to_many = FormTimeDistanceMatrix(request); break; } @@ -248,14 +249,14 @@ std::vector TimeDistanceBSSMatrix::ComputeMatrix( const DirectedEdge* edge = tile->directededge(pred.edgeid()); if (UpdateDestinations(origin, destinations, destedge->second, edge, tile, pred, matrix_locations)) { - one_to_many = FormTimeDistanceMatrix(); + one_to_many = FormTimeDistanceMatrix(request); break; } } // Terminate when we are beyond the cost threshold if (pred.cost().cost > current_cost_threshold_) { - one_to_many = FormTimeDistanceMatrix(); + one_to_many = FormTimeDistanceMatrix(request); break; } @@ -281,19 +282,16 @@ std::vector TimeDistanceBSSMatrix::ComputeMatrix( return many_to_many; } -template std::vector TimeDistanceBSSMatrix::ComputeMatrix( - const google::protobuf::RepeatedPtrField& source_location_list, - const google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations); template std::vector -TimeDistanceBSSMatrix::ComputeMatrix( - const google::protobuf::RepeatedPtrField& source_location_list, - const google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations); +TimeDistanceBSSMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations); +template std::vector +TimeDistanceBSSMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations); // Add edges at the origin to the adjacency list template @@ -538,7 +536,7 @@ bool TimeDistanceBSSMatrix::UpdateDestinations( } // Form the time, distance matrix from the destinations list -std::vector TimeDistanceBSSMatrix::FormTimeDistanceMatrix() { +std::vector TimeDistanceBSSMatrix::FormTimeDistanceMatrix(Api& request) { std::vector td; for (auto& dest : destinations_) { td.emplace_back(dest.best_cost.secs, dest.distance); diff --git a/src/thor/timedistancematrix.cc b/src/thor/timedistancematrix.cc index f3deb25e52..d65885385b 100644 --- a/src/thor/timedistancematrix.cc +++ b/src/thor/timedistancematrix.cc @@ -188,14 +188,18 @@ void TimeDistanceMatrix::Expand(GraphReader& graphreader, } template -std::vector TimeDistanceMatrix::ComputeMatrix( - google::protobuf::RepeatedPtrField& origins, - google::protobuf::RepeatedPtrField& destinations, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations, - const bool invariant) { +std::vector TimeDistanceMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations, + const bool invariant) { uint32_t bucketsize = costing_->UnitSize(); + + auto& origins = FORWARD ? *request.mutable_options()->mutable_sources() + : *request.mutable_options()->mutable_targets(); + auto& destinations = FORWARD ? *request.mutable_options()->mutable_targets() + : *request.mutable_options()->mutable_sources(); + auto time_infos = SetTime(origins, graphreader); // Initialize destinations once for all origins @@ -227,7 +231,7 @@ std::vector TimeDistanceMatrix::ComputeMatrix( uint32_t predindex = adjacencylist_.pop(); if (predindex == kInvalidLabel) { // Can not expand any further... - one_to_many = FormTimeDistanceMatrix(graphreader, origin.date_time(), + one_to_many = FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, GraphId{}); break; } @@ -252,7 +256,7 @@ std::vector TimeDistanceMatrix::ComputeMatrix( const DirectedEdge* edge = tile->directededge(pred.edgeid()); if (UpdateDestinations(origin, destinations, destedge->second, edge, tile, pred, time_info, matrix_locations)) { - one_to_many = FormTimeDistanceMatrix(graphreader, origin.date_time(), + one_to_many = FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, pred.edgeid()); break; } @@ -260,7 +264,7 @@ std::vector TimeDistanceMatrix::ComputeMatrix( // Terminate when we are beyond the cost threshold if (pred.cost().cost > current_cost_threshold_) { - one_to_many = FormTimeDistanceMatrix(graphreader, origin.date_time(), + one_to_many = FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, pred.edgeid()); break; } @@ -288,20 +292,18 @@ std::vector TimeDistanceMatrix::ComputeMatrix( return many_to_many; } -template std::vector TimeDistanceMatrix::ComputeMatrix( - google::protobuf::RepeatedPtrField& origins, - google::protobuf::RepeatedPtrField& destinations, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations, - const bool invariant); -template std::vector TimeDistanceMatrix::ComputeMatrix( - google::protobuf::RepeatedPtrField& origins, - google::protobuf::RepeatedPtrField& destinations, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations, - const bool invariant); +template std::vector +TimeDistanceMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations, + const bool invariant); +template std::vector +TimeDistanceMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations, + const bool invariant); // Add edges at the origin to the adjacency list template @@ -556,15 +558,21 @@ bool TimeDistanceMatrix::UpdateDestinations( } // Form the time, distance matrix from the destinations list -std::vector TimeDistanceMatrix::FormTimeDistanceMatrix(GraphReader& reader, +std::vector TimeDistanceMatrix::FormTimeDistanceMatrix(Api& request, + GraphReader& reader, const std::string& origin_dt, const uint64_t& origin_tz, const GraphId& pred_id) { std::vector td; + uint32_t idx = 0; + Matrix matrix; for (auto& dest : destinations_) { + const uint32_t from_index = idx / request.options().targets().size(); + const uint32_t to_index = idx % request.options().targets().size(); auto date_time = get_date_time(origin_dt, origin_tz, pred_id, reader, static_cast(dest.best_cost.secs + .5f)); td.emplace_back(dest.best_cost.secs, dest.distance, date_time); + idx++; } return td; } diff --git a/valhalla/thor/costmatrix.h b/valhalla/thor/costmatrix.h index 4d1667f5e4..199f94f3b6 100644 --- a/valhalla/thor/costmatrix.h +++ b/valhalla/thor/costmatrix.h @@ -95,15 +95,13 @@ class CostMatrix { * @param max_matrix_distance Maximum arc-length distance for current mode. * @return time/distance from origin index to all other locations */ - std::vector - SourceToTarget(google::protobuf::RepeatedPtrField& source_location_list, - google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t mode, - const float max_matrix_distance, - const bool has_time = false, - const bool invariant = false); + std::vector SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t mode, + const float max_matrix_distance, + const bool has_time = false, + const bool invariant = false); /** * Clear the temporary information generated during time+distance diff --git a/valhalla/thor/timedistancebssmatrix.h b/valhalla/thor/timedistancebssmatrix.h index 7add0830d2..b3e42dc048 100644 --- a/valhalla/thor/timedistancebssmatrix.h +++ b/valhalla/thor/timedistancebssmatrix.h @@ -46,14 +46,12 @@ class TimeDistanceBSSMatrix { * to the closest 20 out of 50 locations). * @return time/distance from origin index to all other locations */ - inline std::vector - SourceToTarget(const google::protobuf::RepeatedPtrField& source_location_list, - const google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t /*mode*/, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations) { + inline std::vector SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t /*mode*/, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations) { LOG_INFO("matrix::TimeDistanceBSSMatrix"); @@ -61,14 +59,13 @@ class TimeDistanceBSSMatrix { pedestrian_costing_ = mode_costing[static_cast(sif::travel_mode_t::kPedestrian)]; bicycle_costing_ = mode_costing[static_cast(sif::travel_mode_t::kBicycle)]; - const bool forward_search = source_location_list.size() <= target_location_list.size(); + const bool forward_search = + request.options().sources().size() <= request.options().targets().size(); if (forward_search) { - return ComputeMatrix(source_location_list, target_location_list, - graphreader, max_matrix_distance, + return ComputeMatrix(request, graphreader, max_matrix_distance, matrix_locations); } else { - return ComputeMatrix(source_location_list, target_location_list, - graphreader, max_matrix_distance, + return ComputeMatrix(request, graphreader, max_matrix_distance, matrix_locations); } }; @@ -153,12 +150,10 @@ class TimeDistanceBSSMatrix { */ template - std::vector - ComputeMatrix(const google::protobuf::RepeatedPtrField& source_location_list, - const google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations); + std::vector ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations); /** * Expand from the node along the forward search path. Immediately expands @@ -248,9 +243,11 @@ class TimeDistanceBSSMatrix { /** * Form a time/distance matrix from the results. + * + * @param request the request PBF * @return Returns a time distance matrix among locations. */ - std::vector FormTimeDistanceMatrix(); + std::vector FormTimeDistanceMatrix(Api& request); }; } // namespace thor diff --git a/valhalla/thor/timedistancematrix.h b/valhalla/thor/timedistancematrix.h index 63341cce46..80846b6101 100644 --- a/valhalla/thor/timedistancematrix.h +++ b/valhalla/thor/timedistancematrix.h @@ -46,15 +46,13 @@ class TimeDistanceMatrix { * * @return time/distance from all sources to all targets */ - inline std::vector - SourceToTarget(google::protobuf::RepeatedPtrField& source_location_list, - google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t mode, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations, - const bool invariant = false) { + inline std::vector SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t mode, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations, + const bool invariant = false) { LOG_INFO("matrix::TimeDistanceMatrix"); @@ -62,15 +60,14 @@ class TimeDistanceMatrix { mode_ = mode; costing_ = mode_costing[static_cast(mode_)]; - const bool forward_search = source_location_list.size() <= target_location_list.size(); + const bool forward_search = + request.options().sources().size() <= request.options().targets().size(); if (forward_search) { - return ComputeMatrix(source_location_list, target_location_list, - graphreader, max_matrix_distance, matrix_locations, - invariant); + return ComputeMatrix(request, graphreader, max_matrix_distance, + matrix_locations, invariant); } else { - return ComputeMatrix(target_location_list, source_location_list, - graphreader, max_matrix_distance, matrix_locations, - invariant); + return ComputeMatrix(request, graphreader, max_matrix_distance, + matrix_locations, invariant); } }; @@ -149,13 +146,11 @@ class TimeDistanceMatrix { */ template - std::vector - ComputeMatrix(google::protobuf::RepeatedPtrField& source_location_list, - google::protobuf::RepeatedPtrField& target_location_list, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations, - const bool invariant = false); + std::vector ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations, + const bool invariant = false); /** * Expand from the node along the forward search path. Immediately expands @@ -266,6 +261,8 @@ class TimeDistanceMatrix { /** * Form a time/distance matrix from the results. + * + * @param request The full request object * @param reader GraphReader instance * @param origin_dt The origin's date_time string * @param origin_tz The origin's timezone index @@ -273,7 +270,8 @@ class TimeDistanceMatrix { * * @return Returns a time distance matrix among locations. */ - std::vector FormTimeDistanceMatrix(baldr::GraphReader& reader, + std::vector FormTimeDistanceMatrix(Api& request, + baldr::GraphReader& reader, const std::string& origin_dt, const uint64_t& origin_tz, const baldr::GraphId& pred_id); From 8cec20c3b03478432a92ab63ffbdba74c1f19fec Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Sun, 21 May 2023 21:07:14 +0200 Subject: [PATCH 03/14] finish the implementation; adapt all matrix algos to keep track of things in pbf and add the scaffolding to be able to return pbf --- proto/api.proto | 2 + proto/matrix.proto | 2 +- proto/options.proto | 2 +- src/thor/costmatrix.cc | 34 ++++---- src/thor/matrix_action.cc | 26 +++--- src/thor/optimized_route_action.cc | 16 ++-- src/thor/timedistancebssmatrix.cc | 31 ++++--- src/thor/timedistancematrix.cc | 63 ++++++-------- src/tyr/actor.cc | 8 +- src/tyr/matrix_serializer.cc | 119 ++++++++++++++------------ src/tyr/serializers.cc | 5 ++ src/worker.cc | 11 ++- test/gurka/gurka.cc | 2 + test/gurka/test_pbf_api.cc | 45 ++++++---- valhalla/thor/costmatrix.h | 21 ++--- valhalla/thor/timedistancebssmatrix.h | 23 +++-- valhalla/thor/timedistancematrix.h | 36 ++++---- valhalla/tyr/serializers.h | 5 +- 18 files changed, 231 insertions(+), 220 deletions(-) diff --git a/proto/api.proto b/proto/api.proto index 9154c6d33a..c0dead1a16 100644 --- a/proto/api.proto +++ b/proto/api.proto @@ -7,6 +7,7 @@ import public "trip.proto"; // the paths, filled out by thor import public "directions.proto"; // the directions, filled out by odin import public "info.proto"; // statistics about the request, filled out by loki/thor/odin import public "status.proto"; // info for status endpoint +import public "matrix.proto"; // the matrix results message Api { // this is the request to the api @@ -16,6 +17,7 @@ message Api { Trip trip = 2; // trace_attributes Directions directions = 3; // route, optimized_route, trace_route, centroid Status status = 4; // status + Matrix matrix = 5; // sources_to_targets //TODO: isochrone //TODO: matrix //TODO: locate diff --git a/proto/matrix.proto b/proto/matrix.proto index 2d03322678..328f4c90a6 100644 --- a/proto/matrix.proto +++ b/proto/matrix.proto @@ -10,7 +10,7 @@ message Matrix { } message TimeDistance { - double distance = 1; + uint32 distance = 1; double time = 2; uint32 from_index = 3; uint32 to_index = 4; diff --git a/proto/options.proto b/proto/options.proto index 9dd308aa92..1119a9489f 100644 --- a/proto/options.proto +++ b/proto/options.proto @@ -50,9 +50,9 @@ message PbfFieldSelector { bool trip = 2; // /trace_attributes bool directions = 3; // /route /trace_route /optimized_route /centroid bool status = 4; // /status + bool matrix = 5; // sources_to_targets // TODO: enable these once we have objects for them // bool isochrone = 5; - // bool matrix = 6; // bool locate = 7; // bool height = 8; // bool expansion = 9; diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index e5dac1f53c..1dc90e9ead 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -119,13 +119,13 @@ void CostMatrix::clear() { // Form a time distance matrix from the set of source locations // to the set of target locations. -std::vector CostMatrix::SourceToTarget(Api& request, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t mode, - const float max_matrix_distance, - const bool has_time, - const bool invariant) { +void CostMatrix::SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t mode, + const float max_matrix_distance, + const bool has_time, + const bool invariant) { LOG_INFO("matrix::CostMatrix"); @@ -235,20 +235,20 @@ std::vector CostMatrix::SourceToTarget(Api& request, std::vector td; uint32_t count = 0; for (const auto& connection : best_connection_) { + Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); uint32_t target_idx = count % target_location_list.size(); uint32_t origin_idx = count / target_location_list.size(); - if (has_time) { - auto date_time = get_date_time(source_location_list[origin_idx].date_time(), - time_infos[origin_idx].timezone_index, - target_edgelabel_[target_idx].front().edgeid(), graphreader, - static_cast(connection.cost.secs + .5f)); - td.emplace_back(std::round(connection.cost.secs), std::round(connection.distance), date_time); - } else { - td.emplace_back(std::round(connection.cost.secs), std::round(connection.distance)); - } + auto date_time = get_date_time(source_location_list[origin_idx].date_time(), + time_infos[origin_idx].timezone_index, + target_edgelabel_[target_idx].front().edgeid(), graphreader, + static_cast(connection.cost.secs + .5f)); + td.set_from_index(origin_idx); + td.set_to_index(target_idx); + td.set_distance(connection.distance); + td.set_time(connection.cost.secs); + td.set_date_time(date_time); count++; } - return td; } // Initialize all time distance to "not found". Any locations that diff --git a/src/thor/matrix_action.cc b/src/thor/matrix_action.cc index bf5e4d4afa..8da7adc73d 100644 --- a/src/thor/matrix_action.cc +++ b/src/thor/matrix_action.cc @@ -27,16 +27,15 @@ std::string thor_worker_t::matrix(Api& request) { adjust_scores(options); auto costing = parse_costing(request); - // Distance scaling (miles or km) - double distance_scale = (options.units() == Options::miles) ? kMilePerMeter : kKmPerMeter; - // lambdas to do the real work auto costmatrix = [&](const bool has_time) { + request.mutable_matrix()->set_algorithm(Matrix::CostMatrix); return costmatrix_.SourceToTarget(request, *reader, mode_costing, mode, max_matrix_distance.find(costing)->second, has_time, options.date_time_type() == Options::invariant); }; auto timedistancematrix = [&]() { + request.mutable_matrix()->set_algorithm(Matrix::TimeDistanceMatrix); return time_distance_matrix_.SourceToTarget(request, *reader, mode_costing, mode, max_matrix_distance.find(costing)->second, options.matrix_locations(), @@ -44,11 +43,11 @@ std::string thor_worker_t::matrix(Api& request) { }; if (costing == "bikeshare") { - const auto& time_distances = - time_distance_bss_matrix_.SourceToTarget(request, *reader, mode_costing, mode, - max_matrix_distance.find(costing)->second, - options.matrix_locations()); - return tyr::serializeMatrix(request, time_distances, distance_scale, Matrix::TimeDistanceMatrix); + request.mutable_matrix()->set_algorithm(Matrix::TimeDistanceMatrix); + time_distance_bss_matrix_.SourceToTarget(request, *reader, mode_costing, mode, + max_matrix_distance.find(costing)->second, + options.matrix_locations()); + return tyr::serializeMatrix(request); } Matrix::Algorithm matrix_algo = Matrix::CostMatrix; @@ -85,24 +84,23 @@ std::string thor_worker_t::matrix(Api& request) { check_matrix_time(request, options.prioritize_bidirectional() ? Matrix::CostMatrix : Matrix::TimeDistanceMatrix); if (has_time && !options.prioritize_bidirectional() && source_to_target_algorithm != COST_MATRIX) { - return tyr::serializeMatrix(request, timedistancematrix(), distance_scale, - Matrix::TimeDistanceMatrix); + timedistancematrix(); } else if (has_time && options.prioritize_bidirectional() && source_to_target_algorithm != TIME_DISTANCE_MATRIX) { - return tyr::serializeMatrix(request, costmatrix(has_time), distance_scale, Matrix::CostMatrix); + costmatrix(has_time); } else if (matrix_algo == Matrix::CostMatrix) { // if this happens, the server config only allows for timedist matrix if (has_time && !options.prioritize_bidirectional()) { add_warning(request, 301); } - return tyr::serializeMatrix(request, costmatrix(has_time), distance_scale, Matrix::CostMatrix); + costmatrix(has_time); } else { if (has_time && options.prioritize_bidirectional()) { add_warning(request, 300); } - return tyr::serializeMatrix(request, timedistancematrix(), distance_scale, - Matrix::TimeDistanceMatrix); + timedistancematrix(); } + return tyr::serializeMatrix(request); } } // namespace thor } // namespace valhalla diff --git a/src/thor/optimized_route_action.cc b/src/thor/optimized_route_action.cc index ae0e9e4a61..4715b42992 100644 --- a/src/thor/optimized_route_action.cc +++ b/src/thor/optimized_route_action.cc @@ -28,11 +28,10 @@ void thor_worker_t::optimized_route(Api& request) { // Use CostMatrix to find costs from each location to every other location CostMatrix costmatrix; - std::vector td = - costmatrix.SourceToTarget(request, *reader, mode_costing, mode, - max_matrix_distance.find(costing)->second, - check_matrix_time(request, Matrix::CostMatrix), - options.date_time_type() == Options::invariant); + costmatrix.SourceToTarget(request, *reader, mode_costing, mode, + max_matrix_distance.find(costing)->second, + check_matrix_time(request, Matrix::CostMatrix), + options.date_time_type() == Options::invariant); // Return an error if any locations are totally unreachable const auto& correlated = @@ -41,7 +40,8 @@ void thor_worker_t::optimized_route(Api& request) { // Set time costs to send to Optimizer. std::vector time_costs; bool reachable = true; - for (size_t i = 0; i < td.size(); ++i) { + const auto tds = request.matrix().time_distances(); + for (size_t i = 0; i < tds.size(); ++i) { // If any location is completely unreachable then we cant have a connected path if (i % correlated.size() == 0) { if (!reachable) { @@ -49,9 +49,9 @@ void thor_worker_t::optimized_route(Api& request) { }; reachable = false; } - reachable = reachable || td[i].time != kMaxCost; + reachable = reachable || tds[i].time() != kMaxCost; // Keep the times for the reordering - time_costs.emplace_back(static_cast(td[i].time)); + time_costs.emplace_back(static_cast(tds[i].time())); } Optimizer optimizer; diff --git a/src/thor/timedistancebssmatrix.cc b/src/thor/timedistancebssmatrix.cc index 179e2062fa..18ab04f45b 100644 --- a/src/thor/timedistancebssmatrix.cc +++ b/src/thor/timedistancebssmatrix.cc @@ -182,10 +182,10 @@ void TimeDistanceBSSMatrix::Expand(GraphReader& graphreader, // Calculate time and distance from one origin location to many destination // locations. template -std::vector TimeDistanceBSSMatrix::ComputeMatrix(Api& request, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations) { +void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations) { // Run a series of one to many calls and concatenate the results. auto& origins = FORWARD ? *request.mutable_options()->mutable_sources() @@ -222,7 +222,7 @@ std::vector TimeDistanceBSSMatrix::ComputeMatrix(Api& request, uint32_t predindex = adjacencylist_.pop(); if (predindex == kInvalidLabel) { // Can not expand any further... - one_to_many = FormTimeDistanceMatrix(request); + FormTimeDistanceMatrix(request); break; } @@ -249,14 +249,14 @@ std::vector TimeDistanceBSSMatrix::ComputeMatrix(Api& request, const DirectedEdge* edge = tile->directededge(pred.edgeid()); if (UpdateDestinations(origin, destinations, destedge->second, edge, tile, pred, matrix_locations)) { - one_to_many = FormTimeDistanceMatrix(request); + FormTimeDistanceMatrix(request); break; } } // Terminate when we are beyond the cost threshold if (pred.cost().cost > current_cost_threshold_) { - one_to_many = FormTimeDistanceMatrix(request); + FormTimeDistanceMatrix(request); break; } @@ -279,15 +279,14 @@ std::vector TimeDistanceBSSMatrix::ComputeMatrix(Api& request, } reset(); } - return many_to_many; } -template std::vector +template void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, baldr::GraphReader& graphreader, const float max_matrix_distance, const uint32_t matrix_locations); -template std::vector +template void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, baldr::GraphReader& graphreader, const float max_matrix_distance, @@ -536,12 +535,16 @@ bool TimeDistanceBSSMatrix::UpdateDestinations( } // Form the time, distance matrix from the destinations list -std::vector TimeDistanceBSSMatrix::FormTimeDistanceMatrix(Api& request) { - std::vector td; +void TimeDistanceBSSMatrix::FormTimeDistanceMatrix(Api& request) { + uint32_t idx = 0; for (auto& dest : destinations_) { - td.emplace_back(dest.best_cost.secs, dest.distance); + Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); + td.set_from_index(idx / static_cast(request.options().targets().size())); + td.set_to_index(idx % static_cast(request.options().targets().size())); + td.set_distance(dest.distance); + td.set_time(dest.best_cost.secs); + idx++; } - return td; } } // namespace thor diff --git a/src/thor/timedistancematrix.cc b/src/thor/timedistancematrix.cc index d65885385b..764f647cf0 100644 --- a/src/thor/timedistancematrix.cc +++ b/src/thor/timedistancematrix.cc @@ -188,11 +188,11 @@ void TimeDistanceMatrix::Expand(GraphReader& graphreader, } template -std::vector TimeDistanceMatrix::ComputeMatrix(Api& request, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations, - const bool invariant) { +void TimeDistanceMatrix::ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations, + const bool invariant) { uint32_t bucketsize = costing_->UnitSize(); auto& origins = FORWARD ? *request.mutable_options()->mutable_sources() @@ -231,8 +231,8 @@ std::vector TimeDistanceMatrix::ComputeMatrix(Api& request, uint32_t predindex = adjacencylist_.pop(); if (predindex == kInvalidLabel) { // Can not expand any further... - one_to_many = FormTimeDistanceMatrix(request, graphreader, origin.date_time(), - time_info.timezone_index, GraphId{}); + FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, + GraphId{}); break; } @@ -256,16 +256,16 @@ std::vector TimeDistanceMatrix::ComputeMatrix(Api& request, const DirectedEdge* edge = tile->directededge(pred.edgeid()); if (UpdateDestinations(origin, destinations, destedge->second, edge, tile, pred, time_info, matrix_locations)) { - one_to_many = FormTimeDistanceMatrix(request, graphreader, origin.date_time(), - time_info.timezone_index, pred.edgeid()); + FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, + pred.edgeid()); break; } } // Terminate when we are beyond the cost threshold if (pred.cost().cost > current_cost_threshold_) { - one_to_many = FormTimeDistanceMatrix(request, graphreader, origin.date_time(), - time_info.timezone_index, pred.edgeid()); + FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, + pred.edgeid()); break; } @@ -274,31 +274,17 @@ std::vector TimeDistanceMatrix::ComputeMatrix(Api& request, invariant); } - // Insert one-to-many into many-to-many - if (FORWARD) { - for (size_t target_index = 0; target_index < destinations.size(); target_index++) { - size_t index = origin_index * origins.size() + target_index; - many_to_many[index] = one_to_many[target_index]; - } - } else { - for (size_t source_index = 0; source_index < destinations.size(); source_index++) { - size_t index = source_index * origins.size() + origin_index; - many_to_many[index] = one_to_many[source_index]; - } - } reset(); } - - return many_to_many; } -template std::vector +template void TimeDistanceMatrix::ComputeMatrix(Api& request, baldr::GraphReader& graphreader, const float max_matrix_distance, const uint32_t matrix_locations, const bool invariant); -template std::vector +template void TimeDistanceMatrix::ComputeMatrix(Api& request, baldr::GraphReader& graphreader, const float max_matrix_distance, @@ -558,23 +544,24 @@ bool TimeDistanceMatrix::UpdateDestinations( } // Form the time, distance matrix from the destinations list -std::vector TimeDistanceMatrix::FormTimeDistanceMatrix(Api& request, - GraphReader& reader, - const std::string& origin_dt, - const uint64_t& origin_tz, - const GraphId& pred_id) { - std::vector td; +void TimeDistanceMatrix::FormTimeDistanceMatrix(Api& request, + GraphReader& reader, + const std::string& origin_dt, + const uint64_t& origin_tz, + const GraphId& pred_id) { uint32_t idx = 0; - Matrix matrix; for (auto& dest : destinations_) { - const uint32_t from_index = idx / request.options().targets().size(); - const uint32_t to_index = idx % request.options().targets().size(); + Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); + td.set_from_index(idx / static_cast(request.options().targets().size())); + td.set_to_index(idx % static_cast(request.options().targets().size())); + td.set_distance(dest.distance); + td.set_time(dest.best_cost.secs); + auto date_time = get_date_time(origin_dt, origin_tz, pred_id, reader, static_cast(dest.best_cost.secs + .5f)); - td.emplace_back(dest.best_cost.secs, dest.distance, date_time); + td.set_date_time(date_time); idx++; } - return td; } } // namespace thor diff --git a/src/tyr/actor.cc b/src/tyr/actor.cc index e02ffe39aa..0f332a1409 100644 --- a/src/tyr/actor.cc +++ b/src/tyr/actor.cc @@ -143,12 +143,16 @@ actor_t::matrix(const std::string& request_str, const std::function* int // check the request and locate the locations in the graph pimpl->loki_worker.matrix(*api); // compute the matrix - auto json = pimpl->thor_worker.matrix(*api); + auto format = api->options().format(); + if (format) { + LOG_ERROR("flaj"); + } + auto bytes = pimpl->thor_worker.matrix(*api); // if they want you do to do the cleanup automatically if (auto_cleanup) { cleanup(); } - return json; + return bytes; } std::string actor_t::optimized_route(const std::string& request_str, diff --git a/src/tyr/matrix_serializer.cc b/src/tyr/matrix_serializer.cc index ddfaf24a9e..52ff3634ef 100644 --- a/src/tyr/matrix_serializer.cc +++ b/src/tyr/matrix_serializer.cc @@ -13,12 +13,14 @@ using namespace valhalla::thor; namespace { json::ArrayPtr -serialize_duration(const std::vector& tds, size_t start_td, const size_t td_count) { +serialize_duration(const google::protobuf::RepeatedPtrField& tds, + size_t start_td, + const size_t td_count) { auto time = json::array({}); for (size_t i = start_td; i < start_td + td_count; ++i) { // check to make sure a route was found; if not, return null for time in matrix result - if (tds[i].time != kMaxCost) { - time->emplace_back(static_cast(tds[i].time)); + if (tds[i].time() != kMaxCost) { + time->emplace_back(static_cast(tds[i].time())); } else { time->emplace_back(static_cast(nullptr)); } @@ -26,17 +28,18 @@ serialize_duration(const std::vector& tds, size_t start_td, const return time; } -json::ArrayPtr serialize_distance(const std::vector& tds, - size_t start_td, - const size_t td_count, - const size_t /* source_index */, - const size_t /* target_index */, - double distance_scale) { +json::ArrayPtr +serialize_distance(const google::protobuf::RepeatedPtrField& tds, + size_t start_td, + const size_t td_count, + const size_t /* source_index */, + const size_t /* target_index */, + double distance_scale) { auto distance = json::array({}); for (size_t i = start_td; i < start_td + td_count; ++i) { // check to make sure a route was found; if not, return null for distance in matrix result - if (tds[i].time != kMaxCost) { - distance->emplace_back(json::fixed_t{tds[i].dist * distance_scale, 3}); + if (tds[i].time() != kMaxCost) { + distance->emplace_back(json::fixed_t{tds[i].distance() * distance_scale, 3}); } else { distance->emplace_back(static_cast(nullptr)); } @@ -48,10 +51,7 @@ json::ArrayPtr serialize_distance(const std::vector& tds, namespace osrm_serializers { // Serialize route response in OSRM compatible format. -json::MapPtr serialize(const Api& request, - const std::vector& time_distances, - double distance_scale, - const std::string& matrix_algo) { +std::string serialize(const Api& request, double distance_scale) { auto json = json::map({}); auto time = json::array({}); auto distance = json::array({}); @@ -64,16 +64,20 @@ json::MapPtr serialize(const Api& request, json->emplace("destinations", osrm::waypoints(options.targets())); for (size_t source_index = 0; source_index < options.sources_size(); ++source_index) { - time->emplace_back(serialize_duration(time_distances, source_index * options.targets_size(), + time->emplace_back(serialize_duration(request.matrix().time_distances(), + source_index * options.targets_size(), options.targets_size())); - distance->emplace_back(serialize_distance(time_distances, source_index * options.targets_size(), - options.targets_size(), source_index, 0, - distance_scale)); + distance->emplace_back( + serialize_distance(request.matrix().time_distances(), source_index * options.targets_size(), + options.targets_size(), source_index, 0, distance_scale)); } json->emplace("durations", time); json->emplace("distances", distance); - json->emplace("algorithm", matrix_algo); - return json; + json->emplace("algorithm", MatrixAlgoToString(request.matrix().algorithm())); + + std::stringstream ss; + ss << *json; + return ss.str(); } } // namespace osrm_serializers @@ -93,24 +97,25 @@ json::ArrayPtr locations(const google::protobuf::RepeatedPtrField& tds, - size_t start_td, - const size_t td_count, - const size_t source_index, - const size_t target_index, - double distance_scale) { +json::ArrayPtr +serialize_row(const google::protobuf::RepeatedPtrField& tds, + size_t start_td, + const size_t td_count, + const size_t source_index, + const size_t target_index, + double distance_scale) { auto row = json::array({}); for (size_t i = start_td; i < start_td + td_count; ++i) { // check to make sure a route was found; if not, return null for distance & time in matrix // result json::MapPtr map; - if (tds[i].time != kMaxCost) { + if (tds[i].time() != kMaxCost) { map = json::map({{"from_index", static_cast(source_index)}, {"to_index", static_cast(target_index + (i - start_td))}, - {"time", static_cast(tds[i].time)}, - {"distance", json::fixed_t{tds[i].dist * distance_scale, 3}}}); - if (!tds[i].date_time.empty()) { - map->emplace("date_time", tds[i].date_time); + {"time", static_cast(tds[i].time())}, + {"distance", json::fixed_t{tds[i].distance() * distance_scale, 3}}}); + if (!tds[i].date_time().empty()) { + map->emplace("date_time", tds[i].date_time()); } } else { map = json::map({{"from_index", static_cast(source_index)}, @@ -123,17 +128,15 @@ json::ArrayPtr serialize_row(const std::vector& tds, return row; } -json::MapPtr serialize(const Api& request, - const std::vector& time_distances, - double distance_scale, - const std::string& matrix_algo) { +std::string serialize(const Api& request, double distance_scale) { auto json = json::map({}); const auto& options = request.options(); if (options.verbose()) { json::ArrayPtr matrix = json::array({}); for (size_t source_index = 0; source_index < options.sources_size(); ++source_index) { - matrix->emplace_back(serialize_row(time_distances, source_index * options.targets_size(), + matrix->emplace_back(serialize_row(request.matrix().time_distances(), + source_index * options.targets_size(), options.targets_size(), source_index, 0, distance_scale)); } @@ -148,11 +151,12 @@ json::MapPtr serialize(const Api& request, auto distance = json::array({}); for (size_t source_index = 0; source_index < options.sources_size(); ++source_index) { - time->emplace_back(serialize_duration(time_distances, source_index * options.targets_size(), + time->emplace_back(serialize_duration(request.matrix().time_distances(), + source_index * options.targets_size(), options.targets_size())); - distance->emplace_back(serialize_distance(time_distances, source_index * options.targets_size(), - options.targets_size(), source_index, 0, - distance_scale)); + distance->emplace_back( + serialize_distance(request.matrix().time_distances(), source_index * options.targets_size(), + options.targets_size(), source_index, 0, distance_scale)); } matrix->emplace("distances", distance); matrix->emplace("durations", time); @@ -161,7 +165,7 @@ json::MapPtr serialize(const Api& request, } json->emplace("units", Options_Units_Enum_Name(options.units())); - json->emplace("algorithm", matrix_algo); + json->emplace("algorithm", MatrixAlgoToString(request.matrix().algorithm())); if (options.has_id_case()) { json->emplace("id", options.id()); @@ -172,27 +176,28 @@ json::MapPtr serialize(const Api& request, json->emplace("warnings", valhalla::tyr::serializeWarnings(request)); } - return json; + std::stringstream ss; + ss << *json; + return ss.str(); } } // namespace valhalla_serializers namespace valhalla { namespace tyr { -std::string serializeMatrix(const Api& request, - const std::vector& time_distances, - double distance_scale, - const Matrix::Algorithm& matrix_algo) { - - auto json = request.options().format() == Options::osrm - ? osrm_serializers::serialize(request, time_distances, distance_scale, - MatrixAlgoToString(matrix_algo)) - : valhalla_serializers::serialize(request, time_distances, distance_scale, - MatrixAlgoToString(matrix_algo)); - - std::stringstream ss; - ss << *json; - return ss.str(); +std::string serializeMatrix(Api& request) { + auto format = request.options().format(); + double distance_scale = (request.options().units() == Options::miles) ? kMilePerMeter : kKmPerMeter; + switch (request.options().format()) { + case Options_Format_osrm: + return osrm_serializers::serialize(request, distance_scale); + case Options_Format_json: + return valhalla_serializers::serialize(request, distance_scale); + case Options_Format_pbf: + return serializePbf(request); + default: + throw; + } } } // namespace tyr diff --git a/src/tyr/serializers.cc b/src/tyr/serializers.cc index 0f038c87be..77bf1156e0 100644 --- a/src/tyr/serializers.cc +++ b/src/tyr/serializers.cc @@ -210,6 +210,9 @@ std::string serializePbf(Api& request) { case Options::status: selection.set_status(true); break; + case Options::sources_to_targets: + selection.set_matrix(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"); @@ -233,6 +236,8 @@ std::string serializePbf(Api& request) { request.clear_status(); if (!selection.options()) request.clear_options(); + if (!selection.matrix()) + request.clear_matrix(); // serialize the bytes auto bytes = request.SerializeAsString(); diff --git a/src/worker.cc b/src/worker.cc index da922e2ef9..b6f245d8bc 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -637,10 +637,13 @@ void from_json(rapidjson::Document& doc, Options::Action action, Api& api) { // so that we serialize correctly at the end we fix up any request discrepancies if (options.format() == Options::pbf) { - const std::unordered_set pbf_actions{ - Options::route, Options::optimized_route, Options::trace_route, - Options::centroid, Options::trace_attributes, Options::status, - }; + const std::unordered_set pbf_actions{Options::route, + Options::optimized_route, + Options::trace_route, + Options::centroid, + Options::trace_attributes, + Options::status, + Options::sources_to_targets}; // 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/gurka.cc b/test/gurka/gurka.cc index 4a883d1cc2..76dfe1d993 100644 --- a/test/gurka/gurka.cc +++ b/test/gurka/gurka.cc @@ -603,6 +603,8 @@ baldr::GraphId findNode(valhalla::baldr::GraphReader& reader, std::string do_action(const map& map, valhalla::Api& api, std::shared_ptr reader) { std::cerr << "[ ] Valhalla request is pbf " << std::endl; + + auto format = api.options().format(); if (!reader) reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); valhalla::tyr::actor_t actor(map.config, *reader, true); diff --git a/test/gurka/test_pbf_api.cc b/test/gurka/test_pbf_api.cc index 28d5ddc5f7..a31fb19949 100644 --- a/test/gurka/test_pbf_api.cc +++ b/test/gurka/test_pbf_api.cc @@ -28,28 +28,36 @@ TEST(pbf_api, pbf_in_out) { const auto layout = gurka::detail::map_to_coordinates(ascii_map, 10); auto map = gurka::buildtiles(layout, ways, {}, {}, "test/data/gurka_api_minimal"); - std::unordered_set pbf_actions{ - Options::route, Options::trace_route, Options::optimized_route, - Options::centroid, Options::trace_attributes, Options::status, - }; + std::unordered_set pbf_actions{Options::route, + Options::trace_route, + Options::optimized_route, + Options::centroid, + Options::trace_attributes, + Options::status, + Options::sources_to_targets}; PbfFieldSelector select_all; select_all.set_directions(true); select_all.set_trip(true); select_all.set_status(true); select_all.set_options(true); + select_all.set_matrix(true); for (int action = Options::no_action + 1; action <= Options::Action_MAX; ++action) { // don't have convenient support of these in gurka yet - if (action == Options::sources_to_targets || action == Options::isochrone || - action == Options::expansion) + if (action == Options::isochrone || action == Options::expansion) continue; // do the regular request with json in and out std::string expected_json, request_json; - auto expected_pbf = - gurka::do_action(Options::Action(action), map, {"A", "C", "I", "G"}, "pedestrian", {}, {}, - &expected_json, "break", &request_json); + Api expected_pbf; + if (action == Options::sources_to_targets) { + expected_pbf = gurka::do_action(Options::Action(action), map, {"A", "C"}, {"I", "G"}, + "pedestrian", {}, {}, &expected_json, &request_json); + } else { + expected_pbf = gurka::do_action(Options::Action(action), map, {"A", "C", "I", "G"}, + "pedestrian", {}, {}, &expected_json, "break", &request_json); + } // get some clean input options for pbf requests Api clean_pbf; @@ -69,16 +77,20 @@ TEST(pbf_api, pbf_in_out) { pbf_out.mutable_options()->clear_costings(); pbf_out.mutable_options()->set_format(Options::pbf); pbf_out.mutable_options()->mutable_pbf_field_selector()->CopyFrom(select_all); + + auto format = pbf_out.options().format(); auto pbf_bytes = gurka::do_action(map, pbf_out); Api actual_pbf; EXPECT_TRUE(actual_pbf.ParseFromString(pbf_bytes)); EXPECT_EQ(actual_pbf.trip().SerializeAsString(), expected_pbf.trip().SerializeAsString()); EXPECT_TRUE(actual_pbf.has_options()); - EXPECT_TRUE(actual_pbf.has_trip() || action == Options::status); - EXPECT_TRUE(actual_pbf.has_directions() || action == Options::trace_attributes || - action == Options::status); + EXPECT_TRUE(actual_pbf.has_trip() || action == Options::status || + action == Options::sources_to_targets); + EXPECT_TRUE(actual_pbf.has_directions() || action != Options::trace_route || + action != Options::route); EXPECT_TRUE(actual_pbf.has_status() || action != Options::status); EXPECT_TRUE(actual_pbf.has_info() || action == Options::status); + EXPECT_TRUE(actual_pbf.has_matrix() || action != Options::sources_to_targets); // lets try it again but this time we'll disable all the fields but one Api slimmed; @@ -90,7 +102,8 @@ TEST(pbf_api, pbf_in_out) { Api actual_slimmed; EXPECT_TRUE(actual_slimmed.ParseFromString(pbf_bytes)); EXPECT_FALSE(actual_slimmed.has_options()); - EXPECT_TRUE(actual_slimmed.has_trip() || action == Options::status); + EXPECT_TRUE(actual_slimmed.has_trip() || action == Options::status || + action == Options::sources_to_targets); EXPECT_FALSE(actual_slimmed.has_directions()); EXPECT_FALSE(actual_slimmed.has_status()); EXPECT_TRUE(actual_slimmed.has_info() || action == Options::status); @@ -105,10 +118,11 @@ TEST(pbf_api, pbf_in_out) { EXPECT_TRUE(actual_slimmed.ParseFromString(pbf_bytes)); EXPECT_FALSE(actual_slimmed.has_options()); EXPECT_TRUE(actual_slimmed.has_trip() || action != Options::trace_attributes); - EXPECT_TRUE(actual_slimmed.has_directions() || action == Options::trace_attributes || - action == Options::status); + EXPECT_TRUE(actual_slimmed.has_directions() || action != Options::trace_route || + action != Options::route); EXPECT_TRUE(actual_slimmed.has_status() || action != Options::status); EXPECT_TRUE(actual_slimmed.has_info() || action == Options::status); + EXPECT_TRUE(actual_slimmed.has_matrix() || action != Options::sources_to_targets); } } } @@ -131,6 +145,7 @@ TEST(pbf_api, pbf_error) { EXPECT_FALSE(actual.has_trip()); EXPECT_FALSE(actual.has_directions()); EXPECT_FALSE(actual.has_status()); + EXPECT_FALSE(actual.has_matrix()); EXPECT_EQ(actual.info().errors().size(), 1); } // try again with an action but no locations diff --git a/valhalla/thor/costmatrix.h b/valhalla/thor/costmatrix.h index 199f94f3b6..32a382f16c 100644 --- a/valhalla/thor/costmatrix.h +++ b/valhalla/thor/costmatrix.h @@ -93,15 +93,14 @@ class CostMatrix { * @param mode_costing Costing methods. * @param mode Travel mode to use. * @param max_matrix_distance Maximum arc-length distance for current mode. - * @return time/distance from origin index to all other locations */ - std::vector SourceToTarget(Api& request, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t mode, - const float max_matrix_distance, - const bool has_time = false, - const bool invariant = false); + void SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t mode, + const float max_matrix_distance, + const bool has_time = false, + const bool invariant = false); /** * Clear the temporary information generated during time+distance @@ -278,12 +277,6 @@ class CostMatrix { return infos; }; - /** - * Form a time/distance matrix from the results. - * @return Returns a time distance matrix among locations. - */ - std::vector FormTimeDistanceMatrix(); - private: class TargetMap; diff --git a/valhalla/thor/timedistancebssmatrix.h b/valhalla/thor/timedistancebssmatrix.h index b3e42dc048..fba79edbb4 100644 --- a/valhalla/thor/timedistancebssmatrix.h +++ b/valhalla/thor/timedistancebssmatrix.h @@ -46,12 +46,12 @@ class TimeDistanceBSSMatrix { * to the closest 20 out of 50 locations). * @return time/distance from origin index to all other locations */ - inline std::vector SourceToTarget(Api& request, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t /*mode*/, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations) { + inline void SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t /*mode*/, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations) { LOG_INFO("matrix::TimeDistanceBSSMatrix"); @@ -150,10 +150,10 @@ class TimeDistanceBSSMatrix { */ template - std::vector ComputeMatrix(Api& request, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations); + void ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations); /** * Expand from the node along the forward search path. Immediately expands @@ -245,9 +245,8 @@ class TimeDistanceBSSMatrix { * Form a time/distance matrix from the results. * * @param request the request PBF - * @return Returns a time distance matrix among locations. */ - std::vector FormTimeDistanceMatrix(Api& request); + void FormTimeDistanceMatrix(Api& request); }; } // namespace thor diff --git a/valhalla/thor/timedistancematrix.h b/valhalla/thor/timedistancematrix.h index 80846b6101..6e4e29bf70 100644 --- a/valhalla/thor/timedistancematrix.h +++ b/valhalla/thor/timedistancematrix.h @@ -46,13 +46,13 @@ class TimeDistanceMatrix { * * @return time/distance from all sources to all targets */ - inline std::vector SourceToTarget(Api& request, - baldr::GraphReader& graphreader, - const sif::mode_costing_t& mode_costing, - const sif::travel_mode_t mode, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations, - const bool invariant = false) { + inline void SourceToTarget(Api& request, + baldr::GraphReader& graphreader, + const sif::mode_costing_t& mode_costing, + const sif::travel_mode_t mode, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations, + const bool invariant = false) { LOG_INFO("matrix::TimeDistanceMatrix"); @@ -146,11 +146,11 @@ class TimeDistanceMatrix { */ template - std::vector ComputeMatrix(Api& request, - baldr::GraphReader& graphreader, - const float max_matrix_distance, - const uint32_t matrix_locations = kAllLocations, - const bool invariant = false); + void ComputeMatrix(Api& request, + baldr::GraphReader& graphreader, + const float max_matrix_distance, + const uint32_t matrix_locations = kAllLocations, + const bool invariant = false); /** * Expand from the node along the forward search path. Immediately expands @@ -267,14 +267,12 @@ class TimeDistanceMatrix { * @param origin_dt The origin's date_time string * @param origin_tz The origin's timezone index * @param pred_id The destination edge's GraphId - * - * @return Returns a time distance matrix among locations. */ - std::vector FormTimeDistanceMatrix(Api& request, - baldr::GraphReader& reader, - const std::string& origin_dt, - const uint64_t& origin_tz, - const baldr::GraphId& pred_id); + void FormTimeDistanceMatrix(Api& request, + baldr::GraphReader& reader, + const std::string& origin_dt, + const uint64_t& origin_tz, + const baldr::GraphId& pred_id); }; } // namespace thor diff --git a/valhalla/tyr/serializers.h b/valhalla/tyr/serializers.h index 1e188fb987..6a9be4c435 100644 --- a/valhalla/tyr/serializers.h +++ b/valhalla/tyr/serializers.h @@ -30,10 +30,7 @@ std::string serializeDirections(Api& request); /** * Turn a time distance matrix into json that one can look up location pair results from */ -std::string serializeMatrix(const Api& request, - const std::vector& time_distances, - double distance_scale, - const Matrix::Algorithm& matrix_algo); +std::string serializeMatrix(Api& request); /** * Turn grid data contours into geojson From fafd730d4019dd49450ad3a40f99415b81ca523a Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Sun, 21 May 2023 21:13:57 +0200 Subject: [PATCH 04/14] remove sources & targets from the output, add changelog --- CHANGELOG.md | 1 + proto/matrix.proto | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d80670b413..2309d3f657 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * **Removed** * **Bug Fix** * **Enhancement** + * ADDED: PBF output for matrix endpoint [#4121](https://github.com/valhalla/valhalla/pull/4121) ## Release Date: 2023-05-11 Valhalla 3.4.0 * **Removed** diff --git a/proto/matrix.proto b/proto/matrix.proto index 328f4c90a6..c4323d32e2 100644 --- a/proto/matrix.proto +++ b/proto/matrix.proto @@ -17,8 +17,6 @@ message Matrix { string date_time = 5; } - repeated Location sources = 1; - repeated Location targets = 2; repeated TimeDistance time_distances = 3; Algorithm algorithm = 4; } From f124625435069a1f8093c6a0e1d694292c09887b Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Sun, 21 May 2023 21:58:50 +0200 Subject: [PATCH 05/14] fix valhalla_run_matrix & remove some log statements --- src/tyr/actor.cc | 4 ---- src/valhalla_run_matrix.cc | 23 ++++++++++------------- test/gurka/gurka.cc | 2 -- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/tyr/actor.cc b/src/tyr/actor.cc index 0f332a1409..ae3fa3923e 100644 --- a/src/tyr/actor.cc +++ b/src/tyr/actor.cc @@ -143,10 +143,6 @@ actor_t::matrix(const std::string& request_str, const std::function* int // check the request and locate the locations in the graph pimpl->loki_worker.matrix(*api); // compute the matrix - auto format = api->options().format(); - if (format) { - LOG_ERROR("flaj"); - } auto bytes = pimpl->thor_worker.matrix(*api); // if they want you do to do the cleanup automatically if (auto_cleanup) { diff --git a/src/valhalla_run_matrix.cc b/src/valhalla_run_matrix.cc index 5de085aee1..49465d5137 100644 --- a/src/valhalla_run_matrix.cc +++ b/src/valhalla_run_matrix.cc @@ -44,15 +44,15 @@ std::string GetFormattedTime(uint32_t secs) { // Log results void LogResults(const bool optimize, const valhalla::Options& options, - const std::vector& res) { + const google::protobuf::RepeatedPtrField& res) { LOG_INFO("Results:"); uint32_t idx1 = 0; uint32_t idx2 = 0; uint32_t nlocs = options.sources_size(); for (auto& td : res) { LOG_INFO(std::to_string(idx1) + "," + std::to_string(idx2) + - ": Distance= " + std::to_string(td.dist) + " Time= " + GetFormattedTime(td.time) + - " secs = " + std::to_string(td.time)); + ": Distance= " + std::to_string(td.distance()) + + " Time= " + GetFormattedTime(td.time()) + " secs = " + std::to_string(td.time())); idx2++; if (idx2 == nlocs) { idx2 = 0; @@ -65,7 +65,7 @@ void LogResults(const bool optimize, std::vector costs; costs.reserve(res.size()); for (auto& td : res) { - costs.push_back(static_cast(td.time)); + costs.push_back(static_cast(td.time())); } Optimizer opt; @@ -227,34 +227,31 @@ int main(int argc, char* argv[]) { } // Timing with CostMatrix - std::vector res; CostMatrix matrix; t0 = std::chrono::high_resolution_clock::now(); for (uint32_t n = 0; n < iterations; n++) { - res.clear(); - res = matrix.SourceToTarget(*options.mutable_sources(), *options.mutable_targets(), reader, - mode_costing, mode, max_distance); + request.clear_matrix(); + matrix.SourceToTarget(request, reader, mode_costing, mode, max_distance); matrix.clear(); } t1 = std::chrono::high_resolution_clock::now(); ms = std::chrono::duration_cast(t1 - t0).count(); float avg = (static_cast(ms) / static_cast(iterations)) * 0.001f; LOG_INFO("CostMatrix average time to compute: " + std::to_string(avg) + " sec"); - LogResults(optimize, options, res); + LogResults(optimize, options, request.matrix().time_distances()); // Run with TimeDistanceMatrix TimeDistanceMatrix tdm; for (uint32_t n = 0; n < iterations; n++) { - res.clear(); - res = tdm.SourceToTarget(*options.mutable_sources(), *options.mutable_targets(), reader, - mode_costing, mode, max_distance); + request.clear_matrix(); + tdm.SourceToTarget(request, reader, mode_costing, mode, max_distance); tdm.clear(); } t1 = std::chrono::high_resolution_clock::now(); ms = std::chrono::duration_cast(t1 - t0).count(); avg = (static_cast(ms) / static_cast(iterations)) * 0.001f; LOG_INFO("TimeDistanceMatrix average time to compute: " + std::to_string(avg) + " sec"); - LogResults(optimize, options, res); + LogResults(optimize, options, request.matrix().time_distances()); // Shutdown protocol buffer library google::protobuf::ShutdownProtobufLibrary(); diff --git a/test/gurka/gurka.cc b/test/gurka/gurka.cc index 76dfe1d993..4a883d1cc2 100644 --- a/test/gurka/gurka.cc +++ b/test/gurka/gurka.cc @@ -603,8 +603,6 @@ baldr::GraphId findNode(valhalla::baldr::GraphReader& reader, std::string do_action(const map& map, valhalla::Api& api, std::shared_ptr reader) { std::cerr << "[ ] Valhalla request is pbf " << std::endl; - - auto format = api.options().format(); if (!reader) reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); valhalla::tyr::actor_t actor(map.config, *reader, true); From e2ccd88eff9539dcda1c6022666c9b2e03f0e3be Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Mon, 22 May 2023 07:55:47 +0200 Subject: [PATCH 06/14] fix benchmkar --- bench/thor/costmatrix.cc | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/bench/thor/costmatrix.cc b/bench/thor/costmatrix.cc index 1dbc415bdc..494c769132 100644 --- a/bench/thor/costmatrix.cc +++ b/bench/thor/costmatrix.cc @@ -80,7 +80,8 @@ static void BM_UtrechtCostMatrix(benchmark::State& state) { locations.emplace_back(midgard::PointLL{lng_distribution(gen), lat_distribution(gen)}); } - Options options; + Api request; + auto& options = *request.mutable_options(); options.set_costing_type(Costing::auto_); rapidjson::Document doc; sif::ParseCosting(doc, "/costing_options", options); @@ -88,25 +89,21 @@ static void BM_UtrechtCostMatrix(benchmark::State& state) { auto costs = sif::CostFactory().CreateModeCosting(options, mode); auto cost = costs[static_cast(mode)]; + auto& sources = *options.mutable_sources(); const auto projections = loki::Search(locations, reader, cost); if (projections.size() == 0) { throw std::runtime_error("Found no matching locations"); } - - google::protobuf::RepeatedPtrField sources; - for (const auto& projection : projections) { auto* p = sources.Add(); baldr::PathLocation::toPBF(projection.second, p, reader); } - std::size_t result_size = 0; - thor::CostMatrix matrix; for (auto _ : state) { - auto result = matrix.SourceToTarget(sources, sources, reader, costs, mode, 100000.); + matrix.SourceToTarget(request, reader, costs, mode, 100000.); matrix.clear(); - result_size += result.size(); + request.clear_matrix(); } state.counters["Routes"] = benchmark::Counter(size, benchmark::Counter::kIsIterationInvariantRate); } From 6e507ef5c5a44563457d9d66ae34ac1b27fc502e Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Mon, 22 May 2023 10:33:09 +0200 Subject: [PATCH 07/14] hmm.. matrix tests are failing.. look into it.. --- test/matrix.cc | 97 +++++++++++++++++++++------------------------- test/matrix_bss.cc | 10 ++--- 2 files changed, 49 insertions(+), 58 deletions(-) diff --git a/test/matrix.cc b/test/matrix.cc index 9aad003776..5abd6fde37 100644 --- a/test/matrix.cc +++ b/test/matrix.cc @@ -198,58 +198,57 @@ TEST(Matrix, test_matrix) { CreateSimpleCost(request.options().costings().find(request.options().costing_type())->second); CostMatrix cost_matrix; - std::vector results = - cost_matrix.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_targets(), reader, mode_costing, - sif::TravelMode::kDrive, 400000.0); - for (uint32_t i = 0; i < results.size(); ++i) { - EXPECT_NEAR(results[i].dist, matrix_answers[i].dist, kThreshold) + cost_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); + auto tds = request.matrix().time_distances(); + for (uint32_t i = 0; i < tds.size(); ++i) { + EXPECT_NEAR(tds[i].distance(), matrix_answers[i].dist, kThreshold) << "result " + std::to_string(i) + "'s distance is not close enough" + " to expected value for CostMatrix"; - EXPECT_NEAR(results[i].time, matrix_answers[i].time, kThreshold) + EXPECT_NEAR(tds[i].time(), matrix_answers[i].time, kThreshold) << "result " + std::to_string(i) + "'s time is not close enough" + " to expected value for CostMatrix"; } + request.clear_matrix(); CostMatrix cost_matrix_abort_source; - results = - cost_matrix_abort_source.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_targets(), reader, - mode_costing, sif::TravelMode::kDrive, 100000.0); + cost_matrix_abort_source.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, + 100000.0); + tds = request.matrix().time_distances(); uint32_t found = 0; - for (uint32_t i = 0; i < results.size(); ++i) { - if (results[i].dist < kMaxCost) { + for (uint32_t i = 0; i < tds.size(); ++i) { + if (tds[i].distance() < kMaxCost) { ++found; } } EXPECT_EQ(found, 15) << " not the number of results as expected"; + request.clear_matrix(); CostMatrix cost_matrix_abort_target; - results = - cost_matrix_abort_target.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_targets(), reader, - mode_costing, sif::TravelMode::kDrive, 50000.0); + cost_matrix_abort_target.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, + 50000.0); + tds = request.matrix().time_distances(); found = 0; - for (uint32_t i = 0; i < results.size(); ++i) { - if (results[i].dist < kMaxCost) { + for (uint32_t i = 0; i < tds.size(); ++i) { + if (tds[i].distance() < kMaxCost) { ++found; } } EXPECT_EQ(found, 10) << " not the number of results as expected"; + request.clear_matrix(); TimeDistanceMatrix timedist_matrix; - results = timedist_matrix.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_targets(), reader, - mode_costing, sif::TravelMode::kDrive, 400000.0); - for (uint32_t i = 0; i < results.size(); ++i) { - EXPECT_NEAR(results[i].dist, matrix_answers[i].dist, kThreshold) + timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); + + tds = request.matrix().time_distances(); + for (uint32_t i = 0; i < tds.size(); ++i) { + EXPECT_NEAR(tds[i].distance(), matrix_answers[i].dist, kThreshold) << "result " + std::to_string(i) + "'s distance is not equal to" + " the expected value for TimeDistMatrix"; - EXPECT_NEAR(results[i].time, matrix_answers[i].time, kThreshold) + EXPECT_NEAR(tds[i].time(), matrix_answers[i].time, kThreshold) << "result " + std::to_string(i) + "'s time is not equal to the expected value for TimeDistMatrix"; } @@ -286,10 +285,8 @@ TEST(Matrix, test_timedistancematrix_results_sequence) { CreateSimpleCost(request.options().costings().find(request.options().costing_type())->second); TimeDistanceMatrix timedist_matrix; - std::vector results = - timedist_matrix.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_targets(), reader, - mode_costing, sif::TravelMode::kDrive, 400000.0); + timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); + auto& tds = request.matrix().time_distances(); // expected results are the same as `matrix_answers`, but without the last column std::vector expected_results = { @@ -297,12 +294,12 @@ TEST(Matrix, test_timedistancematrix_results_sequence) { {2311, 2111}, {701, 641}, {0, 0}, {5562, 5177}, {3952, 3707}, {4367, 4107}, }; - for (uint32_t i = 0; i < results.size(); ++i) { - EXPECT_NEAR(results[i].dist, expected_results[i].dist, kThreshold) + for (uint32_t i = 0; i < tds.size(); ++i) { + EXPECT_NEAR(tds[i].distance(), expected_results[i].dist, kThreshold) << "result " + std::to_string(i) + "'s distance is not equal to" + " the expected value for TimeDistMatrix"; - EXPECT_NEAR(results[i].time, expected_results[i].time, kThreshold) + EXPECT_NEAR(tds[i].time(), expected_results[i].time, kThreshold) << "result " + std::to_string(i) + "'s time is not equal to the expected value for TimeDistMatrix"; } @@ -325,30 +322,28 @@ TEST(Matrix, DISABLED_test_matrix_osrm) { CreateSimpleCost(request.options().costings().find(request.options().costing_type())->second); CostMatrix cost_matrix; - std::vector results; - results = cost_matrix.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_targets(), reader, - mode_costing, sif::TravelMode::kDrive, 400000.0); - for (uint32_t i = 0; i < results.size(); ++i) { - EXPECT_EQ(results[i].dist, matrix_answers[i].dist) + cost_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); + auto tds = request.matrix().time_distances(); + for (uint32_t i = 0; i < tds.size(); ++i) { + EXPECT_EQ(tds[i].distance(), matrix_answers[i].dist) << "result " + std::to_string(i) + "'s distance is not close enough to expected value for CostMatrix."; - EXPECT_EQ(results[i].time, matrix_answers[i].time) + EXPECT_EQ(tds[i].time(), matrix_answers[i].time) << "result " + std::to_string(i) + "'s time is not close enough to expected value for CostMatrix."; } + request.clear_matrix(); TimeDistanceMatrix timedist_matrix; - results = timedist_matrix.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_sources(), reader, - mode_costing, sif::TravelMode::kDrive, 400000.0); - for (uint32_t i = 0; i < results.size(); ++i) { - EXPECT_EQ(results[i].dist, matrix_answers[i].dist) + timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); + tds = request.matrix().time_distances(); + for (uint32_t i = 0; i < tds.size(); ++i) { + EXPECT_EQ(tds[i].distance(), matrix_answers[i].dist) << "result " + std::to_string(i) + "'s distance is not equal to the expected value for TimeDistMatrix."; - EXPECT_EQ(results[i].time, matrix_answers[i].time) + EXPECT_EQ(tds[i].time(), matrix_answers[i].time) << "result " + std::to_string(i) + "'s time is not equal to the expected value for TimeDistMatrix"; } @@ -383,14 +378,12 @@ TEST(Matrix, partial_matrix) { CreateSimpleCost(request.options().costings().find(request.options().costing_type())->second); TimeDistanceMatrix timedist_matrix; - std::vector results = - timedist_matrix.SourceToTarget(*request.mutable_options()->mutable_sources(), - *request.mutable_options()->mutable_targets(), reader, - mode_costing, sif::TravelMode::kDrive, 400000.0, - request.options().matrix_locations()); + timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0, + request.options().matrix_locations()); + auto& tds = request.matrix().time_distances(); uint32_t found = 0; - for (uint32_t i = 0; i < results.size(); ++i) { - if (results[i].dist > 0) { + for (uint32_t i = 0; i < tds.size(); ++i) { + if (tds[i].distance() > 0) { ++found; } } diff --git a/test/matrix_bss.cc b/test/matrix_bss.cc index 579cc9cffb..3b88aacb14 100644 --- a/test/matrix_bss.cc +++ b/test/matrix_bss.cc @@ -114,10 +114,8 @@ class MatrixBssTest : public ::testing::Test { ParseApi(make_matrix_request(sources, targets), Options::sources_to_targets, matrix_request); loki_worker.matrix(matrix_request); - auto matrix_results = - timedist_matrix_bss.SourceToTarget(matrix_request.options().sources(), - matrix_request.options().targets(), reader, mode_costing, - sif::TravelMode::kPedestrian, 400000.0); + timedist_matrix_bss.SourceToTarget(matrix_request, reader, mode_costing, + sif::TravelMode::kPedestrian, 400000.0); auto s_size = sources.size(); auto t_size = targets.size(); @@ -140,8 +138,8 @@ class MatrixBssTest : public ::testing::Test { int route_length = legs.begin()->summary().length() * 1000; size_t m_result_idx = i * t_size + j; - int matrix_time = matrix_results[m_result_idx].time; - int matrix_length = matrix_results[m_result_idx].dist; + int matrix_time = matrix_request.matrix().time_distances()[m_result_idx].time(); + int matrix_length = matrix_request.matrix().time_distances()[m_result_idx].distance(); EXPECT_NEAR(matrix_time, route_time, kTimeThreshold); EXPECT_NEAR(matrix_length, route_length, route_length * kDistancePercentThreshold); From 3e88d7bfa0a480899e4959d3204723825b337e43 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Tue, 23 May 2023 14:54:36 +0200 Subject: [PATCH 08/14] todo: proper order for reverse td matrix & off-by-one issues in distance/time --- src/thor/costmatrix.cc | 1 + src/thor/matrix_action.cc | 3 --- src/thor/timedistancebssmatrix.cc | 28 ++++++++------------------- src/thor/timedistancematrix.cc | 18 ++++++++++------- valhalla/thor/timedistancebssmatrix.h | 3 ++- valhalla/thor/timedistancematrix.h | 3 +++ 6 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index 1dc90e9ead..b2759df28f 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -128,6 +128,7 @@ void CostMatrix::SourceToTarget(Api& request, const bool invariant) { LOG_INFO("matrix::CostMatrix"); + request.mutable_matrix()->set_algorithm(Matrix::CostMatrix); // Set the mode and costing mode_ = mode; diff --git a/src/thor/matrix_action.cc b/src/thor/matrix_action.cc index 8da7adc73d..7f12112442 100644 --- a/src/thor/matrix_action.cc +++ b/src/thor/matrix_action.cc @@ -29,13 +29,11 @@ std::string thor_worker_t::matrix(Api& request) { // lambdas to do the real work auto costmatrix = [&](const bool has_time) { - request.mutable_matrix()->set_algorithm(Matrix::CostMatrix); return costmatrix_.SourceToTarget(request, *reader, mode_costing, mode, max_matrix_distance.find(costing)->second, has_time, options.date_time_type() == Options::invariant); }; auto timedistancematrix = [&]() { - request.mutable_matrix()->set_algorithm(Matrix::TimeDistanceMatrix); return time_distance_matrix_.SourceToTarget(request, *reader, mode_costing, mode, max_matrix_distance.find(costing)->second, options.matrix_locations(), @@ -43,7 +41,6 @@ std::string thor_worker_t::matrix(Api& request) { }; if (costing == "bikeshare") { - request.mutable_matrix()->set_algorithm(Matrix::TimeDistanceMatrix); time_distance_bss_matrix_.SourceToTarget(request, *reader, mode_costing, mode, max_matrix_distance.find(costing)->second, options.matrix_locations()); diff --git a/src/thor/timedistancebssmatrix.cc b/src/thor/timedistancebssmatrix.cc index 18ab04f45b..b6e63220eb 100644 --- a/src/thor/timedistancebssmatrix.cc +++ b/src/thor/timedistancebssmatrix.cc @@ -200,11 +200,9 @@ void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, // Initialize destinations once for all origins InitDestinations(graphreader, destinations); - std::vector many_to_many(origins.size() * destinations.size()); for (size_t origin_index = 0; origin_index < origins.size(); ++origin_index) { edgelabels_.reserve(max_reserved_labels_count_); const auto& origin = origins.Get(origin_index); - std::vector one_to_many; current_cost_threshold_ = GetCostThreshold(max_matrix_distance); adjacencylist_.reuse(0.0f, current_cost_threshold_, bucketsize, &edgelabels_); @@ -222,7 +220,7 @@ void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, uint32_t predindex = adjacencylist_.pop(); if (predindex == kInvalidLabel) { // Can not expand any further... - FormTimeDistanceMatrix(request); + FormTimeDistanceMatrix(request, FORWARD, origin_index); break; } @@ -249,14 +247,14 @@ void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, const DirectedEdge* edge = tile->directededge(pred.edgeid()); if (UpdateDestinations(origin, destinations, destedge->second, edge, tile, pred, matrix_locations)) { - FormTimeDistanceMatrix(request); + FormTimeDistanceMatrix(request, FORWARD, origin_index); break; } } // Terminate when we are beyond the cost threshold if (pred.cost().cost > current_cost_threshold_) { - FormTimeDistanceMatrix(request); + FormTimeDistanceMatrix(request, FORWARD, origin_index); break; } @@ -264,19 +262,6 @@ void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, Expand(graphreader, pred.endnode(), pred, predindex, false, false, pred.mode()); } - - // Insert one-to-many into many-to-many - if (FORWARD) { - for (size_t target_index = 0; target_index < destinations.size(); target_index++) { - size_t index = origin_index * origins.size() + target_index; - many_to_many[index] = one_to_many[target_index]; - } - } else { - for (size_t source_index = 0; source_index < destinations.size(); source_index++) { - size_t index = source_index * origins.size() + origin_index; - many_to_many[index] = one_to_many[source_index]; - } - } reset(); } } @@ -535,8 +520,11 @@ bool TimeDistanceBSSMatrix::UpdateDestinations( } // Form the time, distance matrix from the destinations list -void TimeDistanceBSSMatrix::FormTimeDistanceMatrix(Api& request) { - uint32_t idx = 0; +void TimeDistanceBSSMatrix::FormTimeDistanceMatrix(Api& request, + const bool forward, + const uint32_t origin_index) { + uint32_t idx = origin_index; + // TODO: is this sources or targets? needs more complex logic to set the right order for (auto& dest : destinations_) { Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); td.set_from_index(idx / static_cast(request.options().targets().size())); diff --git a/src/thor/timedistancematrix.cc b/src/thor/timedistancematrix.cc index 764f647cf0..58b160c76c 100644 --- a/src/thor/timedistancematrix.cc +++ b/src/thor/timedistancematrix.cc @@ -204,6 +204,7 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, // Initialize destinations once for all origins InitDestinations(graphreader, destinations); + request.mutable_matrix()->mutable_time_distances()->Reserve(origins.size() * destinations.size()); std::vector many_to_many(origins.size() * destinations.size()); for (size_t origin_index = 0; origin_index < origins.size(); ++origin_index) { @@ -231,8 +232,8 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, uint32_t predindex = adjacencylist_.pop(); if (predindex == kInvalidLabel) { // Can not expand any further... - FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, - GraphId{}); + FormTimeDistanceMatrix(request, graphreader, FORWARD, origin_index, origin.date_time(), + time_info.timezone_index, GraphId{}); break; } @@ -256,16 +257,16 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, const DirectedEdge* edge = tile->directededge(pred.edgeid()); if (UpdateDestinations(origin, destinations, destedge->second, edge, tile, pred, time_info, matrix_locations)) { - FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, - pred.edgeid()); + FormTimeDistanceMatrix(request, graphreader, FORWARD, origin_index, origin.date_time(), + time_info.timezone_index, pred.edgeid()); break; } } // Terminate when we are beyond the cost threshold if (pred.cost().cost > current_cost_threshold_) { - FormTimeDistanceMatrix(request, graphreader, origin.date_time(), time_info.timezone_index, - pred.edgeid()); + FormTimeDistanceMatrix(request, graphreader, FORWARD, origin_index, origin.date_time(), + time_info.timezone_index, pred.edgeid()); break; } @@ -546,10 +547,13 @@ bool TimeDistanceMatrix::UpdateDestinations( // Form the time, distance matrix from the destinations list void TimeDistanceMatrix::FormTimeDistanceMatrix(Api& request, GraphReader& reader, + const bool forward, + const uint32_t origin_index, const std::string& origin_dt, const uint64_t& origin_tz, const GraphId& pred_id) { - uint32_t idx = 0; + uint32_t idx = origin_index; + // TODO: is this sources or targets? needs more complex logic to set the right order for (auto& dest : destinations_) { Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); td.set_from_index(idx / static_cast(request.options().targets().size())); diff --git a/valhalla/thor/timedistancebssmatrix.h b/valhalla/thor/timedistancebssmatrix.h index fba79edbb4..b9d8464ec6 100644 --- a/valhalla/thor/timedistancebssmatrix.h +++ b/valhalla/thor/timedistancebssmatrix.h @@ -54,6 +54,7 @@ class TimeDistanceBSSMatrix { const uint32_t matrix_locations = kAllLocations) { LOG_INFO("matrix::TimeDistanceBSSMatrix"); + request.mutable_matrix()->set_algorithm(Matrix::TimeDistanceMatrix); // Set the costings pedestrian_costing_ = mode_costing[static_cast(sif::travel_mode_t::kPedestrian)]; @@ -246,7 +247,7 @@ class TimeDistanceBSSMatrix { * * @param request the request PBF */ - void FormTimeDistanceMatrix(Api& request); + void FormTimeDistanceMatrix(Api& request, const bool forward, const uint32_t origin_index); }; } // namespace thor diff --git a/valhalla/thor/timedistancematrix.h b/valhalla/thor/timedistancematrix.h index 6e4e29bf70..cbb05abaeb 100644 --- a/valhalla/thor/timedistancematrix.h +++ b/valhalla/thor/timedistancematrix.h @@ -55,6 +55,7 @@ class TimeDistanceMatrix { const bool invariant = false) { LOG_INFO("matrix::TimeDistanceMatrix"); + request.mutable_matrix()->set_algorithm(Matrix::TimeDistanceMatrix); // Set the mode and costing mode_ = mode; @@ -270,6 +271,8 @@ class TimeDistanceMatrix { */ void FormTimeDistanceMatrix(Api& request, baldr::GraphReader& reader, + const bool forward, + const uint32_t origin_index, const std::string& origin_dt, const uint64_t& origin_tz, const baldr::GraphId& pred_id); From 41e3065c6ebce99b75e1342194c18a8dac0047e8 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Tue, 6 Jun 2023 21:11:51 +0200 Subject: [PATCH 09/14] [skip ci] start the work to let proto pack the matrix pbf better --- proto/matrix.proto | 16 ++++++---------- src/thor/optimized_route_action.cc | 6 +++--- src/valhalla_run_matrix.cc | 21 +++++++++++---------- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/proto/matrix.proto b/proto/matrix.proto index c4323d32e2..28f4ac354b 100644 --- a/proto/matrix.proto +++ b/proto/matrix.proto @@ -9,14 +9,10 @@ message Matrix { CostMatrix = 1; } - message TimeDistance { - uint32 distance = 1; - double time = 2; - uint32 from_index = 3; - uint32 to_index = 4; - string date_time = 5; - } - - repeated TimeDistance time_distances = 3; - Algorithm algorithm = 4; + repeated uint32 distances = 2; + repeated double times = 3; + repeated uint32 from_indices = 4; + repeated uint32 to_indices = 5; + repeated string date_times = 6; + Algorithm algorithm = 7; } diff --git a/src/thor/optimized_route_action.cc b/src/thor/optimized_route_action.cc index 4715b42992..0da72727ca 100644 --- a/src/thor/optimized_route_action.cc +++ b/src/thor/optimized_route_action.cc @@ -40,7 +40,7 @@ void thor_worker_t::optimized_route(Api& request) { // Set time costs to send to Optimizer. std::vector time_costs; bool reachable = true; - const auto tds = request.matrix().time_distances(); + const auto tds = request.matrix().times(); for (size_t i = 0; i < tds.size(); ++i) { // If any location is completely unreachable then we cant have a connected path if (i % correlated.size() == 0) { @@ -49,9 +49,9 @@ void thor_worker_t::optimized_route(Api& request) { }; reachable = false; } - reachable = reachable || tds[i].time() != kMaxCost; + reachable = reachable || tds.Get(i) != kMaxCost; // Keep the times for the reordering - time_costs.emplace_back(static_cast(tds[i].time())); + time_costs.emplace_back(static_cast(tds.Get(i))); } Optimizer optimizer; diff --git a/src/valhalla_run_matrix.cc b/src/valhalla_run_matrix.cc index 49465d5137..d1b944f5c8 100644 --- a/src/valhalla_run_matrix.cc +++ b/src/valhalla_run_matrix.cc @@ -44,15 +44,16 @@ std::string GetFormattedTime(uint32_t secs) { // Log results void LogResults(const bool optimize, const valhalla::Options& options, - const google::protobuf::RepeatedPtrField& res) { + const valhalla::Matrix& matrix) { LOG_INFO("Results:"); uint32_t idx1 = 0; uint32_t idx2 = 0; uint32_t nlocs = options.sources_size(); - for (auto& td : res) { - LOG_INFO(std::to_string(idx1) + "," + std::to_string(idx2) + - ": Distance= " + std::to_string(td.distance()) + - " Time= " + GetFormattedTime(td.time()) + " secs = " + std::to_string(td.time())); + for (uint32_t i; i < matrix.times().size(); i++) { + auto distance = matrix.distances().Get(i); + LOG_INFO(std::to_string(idx1) + "," + std::to_string(idx2) + ": Distance= " + + std::to_string(distance) + " Time= " + GetFormattedTime(matrix.times().Get(i)) + + " secs = " + std::to_string(distance)); idx2++; if (idx2 == nlocs) { idx2 = 0; @@ -63,9 +64,9 @@ void LogResults(const bool optimize, // Optimize the path auto t10 = std::chrono::high_resolution_clock::now(); std::vector costs; - costs.reserve(res.size()); - for (auto& td : res) { - costs.push_back(static_cast(td.time())); + costs.reserve(matrix.times().size()); + for (uint32_t i; i < matrix.times().size(); i++) { + costs.push_back(static_cast(matrix.times().Get(i))); } Optimizer opt; @@ -238,7 +239,7 @@ int main(int argc, char* argv[]) { ms = std::chrono::duration_cast(t1 - t0).count(); float avg = (static_cast(ms) / static_cast(iterations)) * 0.001f; LOG_INFO("CostMatrix average time to compute: " + std::to_string(avg) + " sec"); - LogResults(optimize, options, request.matrix().time_distances()); + LogResults(optimize, options, request.matrix()); // Run with TimeDistanceMatrix TimeDistanceMatrix tdm; @@ -251,7 +252,7 @@ int main(int argc, char* argv[]) { ms = std::chrono::duration_cast(t1 - t0).count(); avg = (static_cast(ms) / static_cast(iterations)) * 0.001f; LOG_INFO("TimeDistanceMatrix average time to compute: " + std::to_string(avg) + " sec"); - LogResults(optimize, options, request.matrix().time_distances()); + LogResults(optimize, options, request.matrix()); // Shutdown protocol buffer library google::protobuf::ShutdownProtobufLibrary(); From 79d2fae886e19a4702607e5efa095875fadd36d3 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Thu, 8 Jun 2023 18:32:13 +0200 Subject: [PATCH 10/14] revert to flat repeated fields in the matrix PBF in favor of a repeated Message, to enable int packing; solve the right order for forward/reverse tree --- CHANGELOG.md | 1 + proto/matrix.proto | 16 +++--- src/baldr/transitdeparture.cc | 2 + src/baldr/transitschedule.cc | 3 +- src/thor/costmatrix.cc | 20 +++---- src/thor/optimized_route_action.cc | 6 +-- src/thor/timedistancebssmatrix.cc | 21 ++++---- src/thor/timedistancematrix.cc | 54 ++++++++++++------- src/tyr/matrix_serializer.cc | 67 +++++++++++------------- src/valhalla_run_matrix.cc | 21 ++++---- test/matrix.cc | 84 +++++++++++++++--------------- test/matrix_bss.cc | 4 +- valhalla/baldr/graphconstants.h | 1 + valhalla/thor/matrix_common.h | 25 +++------ valhalla/thor/timedistancematrix.h | 3 +- 15 files changed, 168 insertions(+), 160 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2309d3f657..870f2fdeba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Release Date: 2022-??-?? Valhalla 3.4.1 * **Removed** * **Bug Fix** + * FIXED: gcc13 was missing some std header includes [#4154](https://github.com/valhalla/valhalla/pull/4154) * **Enhancement** * ADDED: PBF output for matrix endpoint [#4121](https://github.com/valhalla/valhalla/pull/4121) diff --git a/proto/matrix.proto b/proto/matrix.proto index c4323d32e2..86dcda417b 100644 --- a/proto/matrix.proto +++ b/proto/matrix.proto @@ -9,14 +9,10 @@ message Matrix { CostMatrix = 1; } - message TimeDistance { - uint32 distance = 1; - double time = 2; - uint32 from_index = 3; - uint32 to_index = 4; - string date_time = 5; - } - - repeated TimeDistance time_distances = 3; - Algorithm algorithm = 4; + repeated uint32 distances = 2; + repeated float times = 3; + repeated uint32 from_indices = 4; + repeated uint32 to_indices = 5; + repeated string date_times = 6; + Algorithm algorithm = 7; } diff --git a/src/baldr/transitdeparture.cc b/src/baldr/transitdeparture.cc index 7299a72165..5e9bada19f 100644 --- a/src/baldr/transitdeparture.cc +++ b/src/baldr/transitdeparture.cc @@ -1,3 +1,5 @@ +#include + #include "baldr/transitdeparture.h" #include "midgard/logging.h" diff --git a/src/baldr/transitschedule.cc b/src/baldr/transitschedule.cc index cf3091ecb6..4d91e47225 100644 --- a/src/baldr/transitschedule.cc +++ b/src/baldr/transitschedule.cc @@ -1,5 +1,6 @@ -#include "baldr/transitschedule.h" +#include +#include "baldr/transitschedule.h" #include "midgard/logging.h" namespace valhalla { diff --git a/src/thor/costmatrix.cc b/src/thor/costmatrix.cc index b2759df28f..57ede63165 100644 --- a/src/thor/costmatrix.cc +++ b/src/thor/costmatrix.cc @@ -232,22 +232,24 @@ void CostMatrix::SourceToTarget(Api& request, RecostPaths(graphreader, source_location_list, target_location_list, time_infos, invariant); } - // Form the time, distance matrix from the destinations list - std::vector td; + // Form the matrix PBF output uint32_t count = 0; + valhalla::Matrix& matrix = *request.mutable_matrix(); + reserve_pbf_arrays(matrix, best_connection_.size()); for (const auto& connection : best_connection_) { - Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); uint32_t target_idx = count % target_location_list.size(); uint32_t origin_idx = count / target_location_list.size(); + float time = connection.cost.secs + .5f; auto date_time = get_date_time(source_location_list[origin_idx].date_time(), time_infos[origin_idx].timezone_index, target_edgelabel_[target_idx].front().edgeid(), graphreader, - static_cast(connection.cost.secs + .5f)); - td.set_from_index(origin_idx); - td.set_to_index(target_idx); - td.set_distance(connection.distance); - td.set_time(connection.cost.secs); - td.set_date_time(date_time); + static_cast(time)); + matrix.mutable_from_indices()->Set(count, origin_idx); + matrix.mutable_to_indices()->Set(count, target_idx); + matrix.mutable_distances()->Set(count, connection.distance); + matrix.mutable_times()->Set(count, time); + auto* pbf_date_time = matrix.mutable_date_times()->Add(); + *pbf_date_time = date_time; count++; } } diff --git a/src/thor/optimized_route_action.cc b/src/thor/optimized_route_action.cc index 4715b42992..0da72727ca 100644 --- a/src/thor/optimized_route_action.cc +++ b/src/thor/optimized_route_action.cc @@ -40,7 +40,7 @@ void thor_worker_t::optimized_route(Api& request) { // Set time costs to send to Optimizer. std::vector time_costs; bool reachable = true; - const auto tds = request.matrix().time_distances(); + const auto tds = request.matrix().times(); for (size_t i = 0; i < tds.size(); ++i) { // If any location is completely unreachable then we cant have a connected path if (i % correlated.size() == 0) { @@ -49,9 +49,9 @@ void thor_worker_t::optimized_route(Api& request) { }; reachable = false; } - reachable = reachable || tds[i].time() != kMaxCost; + reachable = reachable || tds.Get(i) != kMaxCost; // Keep the times for the reordering - time_costs.emplace_back(static_cast(tds[i].time())); + time_costs.emplace_back(static_cast(tds.Get(i))); } Optimizer optimizer; diff --git a/src/thor/timedistancebssmatrix.cc b/src/thor/timedistancebssmatrix.cc index b6e63220eb..b283b7112f 100644 --- a/src/thor/timedistancebssmatrix.cc +++ b/src/thor/timedistancebssmatrix.cc @@ -199,6 +199,8 @@ void TimeDistanceBSSMatrix::ComputeMatrix(Api& request, // Initialize destinations once for all origins InitDestinations(graphreader, destinations); + // reserve the PBF vectors + reserve_pbf_arrays(*request.mutable_matrix(), origins.size() * destinations.size()); for (size_t origin_index = 0; origin_index < origins.size(); ++origin_index) { edgelabels_.reserve(max_reserved_labels_count_); @@ -523,15 +525,16 @@ bool TimeDistanceBSSMatrix::UpdateDestinations( void TimeDistanceBSSMatrix::FormTimeDistanceMatrix(Api& request, const bool forward, const uint32_t origin_index) { - uint32_t idx = origin_index; - // TODO: is this sources or targets? needs more complex logic to set the right order - for (auto& dest : destinations_) { - Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); - td.set_from_index(idx / static_cast(request.options().targets().size())); - td.set_to_index(idx % static_cast(request.options().targets().size())); - td.set_distance(dest.distance); - td.set_time(dest.best_cost.secs); - idx++; + valhalla::Matrix& matrix = *request.mutable_matrix(); + for (uint32_t i = 0; i < destinations_.size(); i++) { + auto& dest = destinations_[i]; + float time = dest.best_cost.secs + .5f; + auto pbf_idx = forward ? (origin_index * request.options().targets().size()) + i + : (i * request.options().targets().size()) + origin_index; + matrix.mutable_from_indices()->Set(pbf_idx, forward ? origin_index : i); + matrix.mutable_to_indices()->Set(pbf_idx, forward ? i : origin_index); + matrix.mutable_distances()->Set(pbf_idx, dest.distance); + matrix.mutable_times()->Set(pbf_idx, time); } } diff --git a/src/thor/timedistancematrix.cc b/src/thor/timedistancematrix.cc index 58b160c76c..69ee23fbbc 100644 --- a/src/thor/timedistancematrix.cc +++ b/src/thor/timedistancematrix.cc @@ -200,20 +200,22 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, auto& destinations = FORWARD ? *request.mutable_options()->mutable_targets() : *request.mutable_options()->mutable_sources(); + size_t num_elements = origins.size() * destinations.size(); auto time_infos = SetTime(origins, graphreader); + // thanks to protobuf not handling strings well, we have to collect those + std::vector out_date_times(num_elements); // Initialize destinations once for all origins InitDestinations(graphreader, destinations); - request.mutable_matrix()->mutable_time_distances()->Reserve(origins.size() * destinations.size()); + // reserve the PBF vectors + reserve_pbf_arrays(*request.mutable_matrix(), num_elements); - std::vector many_to_many(origins.size() * destinations.size()); for (size_t origin_index = 0; origin_index < origins.size(); ++origin_index) { // reserve some space for the next dijkstras (will be cleared at the end of the loop) edgelabels_.reserve(max_reserved_labels_count_); auto& origin = origins.Get(origin_index); const auto& time_info = time_infos[origin_index]; - std::vector one_to_many; current_cost_threshold_ = GetCostThreshold(max_matrix_distance); // Construct adjacency list. Set bucket size and cost range based on DynamicCost. @@ -233,7 +235,7 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, if (predindex == kInvalidLabel) { // Can not expand any further... FormTimeDistanceMatrix(request, graphreader, FORWARD, origin_index, origin.date_time(), - time_info.timezone_index, GraphId{}); + time_info.timezone_index, GraphId{}, out_date_times); break; } @@ -258,7 +260,7 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, if (UpdateDestinations(origin, destinations, destedge->second, edge, tile, pred, time_info, matrix_locations)) { FormTimeDistanceMatrix(request, graphreader, FORWARD, origin_index, origin.date_time(), - time_info.timezone_index, pred.edgeid()); + time_info.timezone_index, pred.edgeid(), out_date_times); break; } } @@ -266,7 +268,7 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, // Terminate when we are beyond the cost threshold if (pred.cost().cost > current_cost_threshold_) { FormTimeDistanceMatrix(request, graphreader, FORWARD, origin_index, origin.date_time(), - time_info.timezone_index, pred.edgeid()); + time_info.timezone_index, pred.edgeid(), out_date_times); break; } @@ -277,6 +279,12 @@ void TimeDistanceMatrix::ComputeMatrix(Api& request, reset(); } + + // amend the date_time strings + for (auto& date_time : out_date_times) { + auto* pbf_dt = request.mutable_matrix()->mutable_date_times()->Add(); + *pbf_dt = date_time; + } } template void @@ -551,20 +559,26 @@ void TimeDistanceMatrix::FormTimeDistanceMatrix(Api& request, const uint32_t origin_index, const std::string& origin_dt, const uint64_t& origin_tz, - const GraphId& pred_id) { - uint32_t idx = origin_index; - // TODO: is this sources or targets? needs more complex logic to set the right order - for (auto& dest : destinations_) { - Matrix::TimeDistance& td = *request.mutable_matrix()->mutable_time_distances()->Add(); - td.set_from_index(idx / static_cast(request.options().targets().size())); - td.set_to_index(idx % static_cast(request.options().targets().size())); - td.set_distance(dest.distance); - td.set_time(dest.best_cost.secs); - - auto date_time = get_date_time(origin_dt, origin_tz, pred_id, reader, - static_cast(dest.best_cost.secs + .5f)); - td.set_date_time(date_time); - idx++; + const GraphId& pred_id, + std::vector& out_date_times) { + // when it's forward, origin_index will be the source_index + // when it's reverse, origin_index will be the target_index + valhalla::Matrix& matrix = *request.mutable_matrix(); + for (uint32_t i = 0; i < destinations_.size(); i++) { + auto& dest = destinations_[i]; + float time = dest.best_cost.secs + .5f; + auto pbf_idx = forward ? (origin_index * request.options().targets().size()) + i + : (i * request.options().targets().size()) + origin_index; + matrix.mutable_from_indices()->Set(pbf_idx, forward ? origin_index : i); + matrix.mutable_to_indices()->Set(pbf_idx, forward ? i : origin_index); + matrix.mutable_distances()->Set(pbf_idx, dest.distance); + matrix.mutable_times()->Set(pbf_idx, time); + + // this logic doesn't work with string repeated fields, gotta collect them + // and process them later + auto date_time = + get_date_time(origin_dt, origin_tz, pred_id, reader, static_cast(time)); + out_date_times[pbf_idx] = date_time; } } diff --git a/src/tyr/matrix_serializer.cc b/src/tyr/matrix_serializer.cc index 52ff3634ef..7e6c3d083f 100644 --- a/src/tyr/matrix_serializer.cc +++ b/src/tyr/matrix_serializer.cc @@ -13,14 +13,12 @@ using namespace valhalla::thor; namespace { json::ArrayPtr -serialize_duration(const google::protobuf::RepeatedPtrField& tds, - size_t start_td, - const size_t td_count) { +serialize_duration(const valhalla::Matrix& matrix, size_t start_td, const size_t td_count) { auto time = json::array({}); for (size_t i = start_td; i < start_td + td_count; ++i) { // check to make sure a route was found; if not, return null for time in matrix result - if (tds[i].time() != kMaxCost) { - time->emplace_back(static_cast(tds[i].time())); + if (matrix.times()[i] != kMaxCost) { + time->emplace_back(static_cast(matrix.times()[i])); } else { time->emplace_back(static_cast(nullptr)); } @@ -28,18 +26,17 @@ serialize_duration(const google::protobuf::RepeatedPtrField& tds, - size_t start_td, - const size_t td_count, - const size_t /* source_index */, - const size_t /* target_index */, - double distance_scale) { +json::ArrayPtr serialize_distance(const valhalla::Matrix& matrix, + size_t start_td, + const size_t td_count, + const size_t /* source_index */, + const size_t /* target_index */, + double distance_scale) { auto distance = json::array({}); for (size_t i = start_td; i < start_td + td_count; ++i) { // check to make sure a route was found; if not, return null for distance in matrix result - if (tds[i].time() != kMaxCost) { - distance->emplace_back(json::fixed_t{tds[i].distance() * distance_scale, 3}); + if (matrix.times()[i] != kMaxCost) { + distance->emplace_back(json::fixed_t{matrix.distances()[i] * distance_scale, 3}); } else { distance->emplace_back(static_cast(nullptr)); } @@ -64,12 +61,11 @@ std::string serialize(const Api& request, double distance_scale) { json->emplace("destinations", osrm::waypoints(options.targets())); for (size_t source_index = 0; source_index < options.sources_size(); ++source_index) { - time->emplace_back(serialize_duration(request.matrix().time_distances(), - source_index * options.targets_size(), + time->emplace_back(serialize_duration(request.matrix(), source_index * options.targets_size(), options.targets_size())); - distance->emplace_back( - serialize_distance(request.matrix().time_distances(), source_index * options.targets_size(), - options.targets_size(), source_index, 0, distance_scale)); + distance->emplace_back(serialize_distance(request.matrix(), source_index * options.targets_size(), + options.targets_size(), source_index, 0, + distance_scale)); } json->emplace("durations", time); json->emplace("distances", distance); @@ -97,25 +93,26 @@ json::ArrayPtr locations(const google::protobuf::RepeatedPtrField& tds, - size_t start_td, - const size_t td_count, - const size_t source_index, - const size_t target_index, - double distance_scale) { +json::ArrayPtr serialize_row(const valhalla::Matrix& matrix, + size_t start_td, + const size_t td_count, + const size_t source_index, + const size_t target_index, + double distance_scale) { auto row = json::array({}); for (size_t i = start_td; i < start_td + td_count; ++i) { // check to make sure a route was found; if not, return null for distance & time in matrix // result json::MapPtr map; - if (tds[i].time() != kMaxCost) { + const auto time = matrix.times()[i]; + const auto& date_time = matrix.date_times()[i]; + if (time != kMaxCost) { map = json::map({{"from_index", static_cast(source_index)}, {"to_index", static_cast(target_index + (i - start_td))}, - {"time", static_cast(tds[i].time())}, - {"distance", json::fixed_t{tds[i].distance() * distance_scale, 3}}}); - if (!tds[i].date_time().empty()) { - map->emplace("date_time", tds[i].date_time()); + {"time", static_cast(time)}, + {"distance", json::fixed_t{matrix.distances()[i] * distance_scale, 3}}}); + if (!date_time.empty()) { + map->emplace("date_time", date_time); } } else { map = json::map({{"from_index", static_cast(source_index)}, @@ -135,8 +132,7 @@ std::string serialize(const Api& request, double distance_scale) { if (options.verbose()) { json::ArrayPtr matrix = json::array({}); for (size_t source_index = 0; source_index < options.sources_size(); ++source_index) { - matrix->emplace_back(serialize_row(request.matrix().time_distances(), - source_index * options.targets_size(), + matrix->emplace_back(serialize_row(request.matrix(), source_index * options.targets_size(), options.targets_size(), source_index, 0, distance_scale)); } @@ -151,11 +147,10 @@ std::string serialize(const Api& request, double distance_scale) { auto distance = json::array({}); for (size_t source_index = 0; source_index < options.sources_size(); ++source_index) { - time->emplace_back(serialize_duration(request.matrix().time_distances(), - source_index * options.targets_size(), + time->emplace_back(serialize_duration(request.matrix(), source_index * options.targets_size(), options.targets_size())); distance->emplace_back( - serialize_distance(request.matrix().time_distances(), source_index * options.targets_size(), + serialize_distance(request.matrix(), source_index * options.targets_size(), options.targets_size(), source_index, 0, distance_scale)); } matrix->emplace("distances", distance); diff --git a/src/valhalla_run_matrix.cc b/src/valhalla_run_matrix.cc index 49465d5137..d1b944f5c8 100644 --- a/src/valhalla_run_matrix.cc +++ b/src/valhalla_run_matrix.cc @@ -44,15 +44,16 @@ std::string GetFormattedTime(uint32_t secs) { // Log results void LogResults(const bool optimize, const valhalla::Options& options, - const google::protobuf::RepeatedPtrField& res) { + const valhalla::Matrix& matrix) { LOG_INFO("Results:"); uint32_t idx1 = 0; uint32_t idx2 = 0; uint32_t nlocs = options.sources_size(); - for (auto& td : res) { - LOG_INFO(std::to_string(idx1) + "," + std::to_string(idx2) + - ": Distance= " + std::to_string(td.distance()) + - " Time= " + GetFormattedTime(td.time()) + " secs = " + std::to_string(td.time())); + for (uint32_t i; i < matrix.times().size(); i++) { + auto distance = matrix.distances().Get(i); + LOG_INFO(std::to_string(idx1) + "," + std::to_string(idx2) + ": Distance= " + + std::to_string(distance) + " Time= " + GetFormattedTime(matrix.times().Get(i)) + + " secs = " + std::to_string(distance)); idx2++; if (idx2 == nlocs) { idx2 = 0; @@ -63,9 +64,9 @@ void LogResults(const bool optimize, // Optimize the path auto t10 = std::chrono::high_resolution_clock::now(); std::vector costs; - costs.reserve(res.size()); - for (auto& td : res) { - costs.push_back(static_cast(td.time())); + costs.reserve(matrix.times().size()); + for (uint32_t i; i < matrix.times().size(); i++) { + costs.push_back(static_cast(matrix.times().Get(i))); } Optimizer opt; @@ -238,7 +239,7 @@ int main(int argc, char* argv[]) { ms = std::chrono::duration_cast(t1 - t0).count(); float avg = (static_cast(ms) / static_cast(iterations)) * 0.001f; LOG_INFO("CostMatrix average time to compute: " + std::to_string(avg) + " sec"); - LogResults(optimize, options, request.matrix().time_distances()); + LogResults(optimize, options, request.matrix()); // Run with TimeDistanceMatrix TimeDistanceMatrix tdm; @@ -251,7 +252,7 @@ int main(int argc, char* argv[]) { ms = std::chrono::duration_cast(t1 - t0).count(); avg = (static_cast(ms) / static_cast(iterations)) * 0.001f; LOG_INFO("TimeDistanceMatrix average time to compute: " + std::to_string(avg) + " sec"); - LogResults(optimize, options, request.matrix().time_distances()); + LogResults(optimize, options, request.matrix()); // Shutdown protocol buffer library google::protobuf::ShutdownProtobufLibrary(); diff --git a/test/matrix.cc b/test/matrix.cc index 5abd6fde37..faa0a77564 100644 --- a/test/matrix.cc +++ b/test/matrix.cc @@ -172,10 +172,12 @@ const auto test_request_osrm = R"({ "costing":"auto" })"; -std::vector matrix_answers = {{28, 28}, {2027, 1837}, {2403, 2213}, {4163, 3838}, - {1519, 1398}, {1808, 1638}, {2061, 1951}, {3944, 3639}, - {2311, 2111}, {701, 641}, {0, 0}, {2821, 2626}, - {5562, 5177}, {3952, 3707}, {4367, 4107}, {1825, 1680}}; +std::vector> matrix_answers = {{28, 28}, {2027, 1837}, {2403, 2213}, + {4163, 3838}, {1519, 1398}, {1808, 1638}, + {2061, 1951}, {3944, 3639}, {2311, 2111}, + {701, 641}, {0, 0}, {2821, 2626}, + {5562, 5177}, {3952, 3707}, {4367, 4107}, + {1825, 1680}}; } // namespace const uint32_t kThreshold = 1; @@ -199,13 +201,13 @@ TEST(Matrix, test_matrix) { CostMatrix cost_matrix; cost_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); - auto tds = request.matrix().time_distances(); - for (uint32_t i = 0; i < tds.size(); ++i) { - EXPECT_NEAR(tds[i].distance(), matrix_answers[i].dist, kThreshold) + auto matrix = request.matrix(); + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + EXPECT_NEAR(matrix.distances()[i], matrix_answers[i][1], kThreshold) << "result " + std::to_string(i) + "'s distance is not close enough" + " to expected value for CostMatrix"; - EXPECT_NEAR(tds[i].time(), matrix_answers[i].time, kThreshold) + EXPECT_NEAR(matrix.times()[i], matrix_answers[i][0], kThreshold) << "result " + std::to_string(i) + "'s time is not close enough" + " to expected value for CostMatrix"; } @@ -215,10 +217,10 @@ TEST(Matrix, test_matrix) { cost_matrix_abort_source.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 100000.0); - tds = request.matrix().time_distances(); + matrix = request.matrix(); uint32_t found = 0; - for (uint32_t i = 0; i < tds.size(); ++i) { - if (tds[i].distance() < kMaxCost) { + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + if (matrix.distances()[i] < kMaxCost) { ++found; } } @@ -229,10 +231,10 @@ TEST(Matrix, test_matrix) { cost_matrix_abort_target.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 50000.0); - tds = request.matrix().time_distances(); + matrix = request.matrix(); found = 0; - for (uint32_t i = 0; i < tds.size(); ++i) { - if (tds[i].distance() < kMaxCost) { + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + if (matrix.distances()[i] < kMaxCost) { ++found; } } @@ -242,15 +244,14 @@ TEST(Matrix, test_matrix) { TimeDistanceMatrix timedist_matrix; timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); - tds = request.matrix().time_distances(); - for (uint32_t i = 0; i < tds.size(); ++i) { - EXPECT_NEAR(tds[i].distance(), matrix_answers[i].dist, kThreshold) - << "result " + std::to_string(i) + "'s distance is not equal to" + - " the expected value for TimeDistMatrix"; + matrix = request.matrix(); + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + EXPECT_NEAR(matrix.distances()[i], matrix_answers[i][1], kThreshold) + << "result " + std::to_string(i) + "'s distance is not equal" + + " to expected value for TDMatrix"; - EXPECT_NEAR(tds[i].time(), matrix_answers[i].time, kThreshold) - << "result " + std::to_string(i) + - "'s time is not equal to the expected value for TimeDistMatrix"; + EXPECT_NEAR(matrix.times()[i], matrix_answers[i][0], kThreshold) + << "result " + std::to_string(i) + "'s time is not equal" + " to expected value for TDMatrix"; } } @@ -286,22 +287,21 @@ TEST(Matrix, test_timedistancematrix_results_sequence) { TimeDistanceMatrix timedist_matrix; timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); - auto& tds = request.matrix().time_distances(); + auto& matrix = request.matrix(); // expected results are the same as `matrix_answers`, but without the last column - std::vector expected_results = { + std::vector> expected_results = { {28, 28}, {2027, 1837}, {2403, 2213}, {1519, 1398}, {1808, 1638}, {2061, 1951}, {2311, 2111}, {701, 641}, {0, 0}, {5562, 5177}, {3952, 3707}, {4367, 4107}, }; - for (uint32_t i = 0; i < tds.size(); ++i) { - EXPECT_NEAR(tds[i].distance(), expected_results[i].dist, kThreshold) - << "result " + std::to_string(i) + "'s distance is not equal to" + - " the expected value for TimeDistMatrix"; + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + EXPECT_NEAR(matrix.distances()[i], expected_results[i][1], kThreshold) + << "result " + std::to_string(i) + "'s distance is not equal" + + " to expected value for TDMatrix"; - EXPECT_NEAR(tds[i].time(), expected_results[i].time, kThreshold) - << "result " + std::to_string(i) + - "'s time is not equal to the expected value for TimeDistMatrix"; + EXPECT_NEAR(matrix.times()[i], expected_results[i][0], kThreshold) + << "result " + std::to_string(i) + "'s time is not equal" + " to expected value for TDMatrix"; } } @@ -323,13 +323,13 @@ TEST(Matrix, DISABLED_test_matrix_osrm) { CostMatrix cost_matrix; cost_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); - auto tds = request.matrix().time_distances(); - for (uint32_t i = 0; i < tds.size(); ++i) { - EXPECT_EQ(tds[i].distance(), matrix_answers[i].dist) + auto matrix = request.matrix(); + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + EXPECT_EQ(matrix.distances()[i], matrix_answers[i][1]) << "result " + std::to_string(i) + "'s distance is not close enough to expected value for CostMatrix."; - EXPECT_EQ(tds[i].time(), matrix_answers[i].time) + EXPECT_EQ(matrix.times()[i], matrix_answers[i][0]) << "result " + std::to_string(i) + "'s time is not close enough to expected value for CostMatrix."; } @@ -337,13 +337,13 @@ TEST(Matrix, DISABLED_test_matrix_osrm) { TimeDistanceMatrix timedist_matrix; timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); - tds = request.matrix().time_distances(); - for (uint32_t i = 0; i < tds.size(); ++i) { - EXPECT_EQ(tds[i].distance(), matrix_answers[i].dist) + matrix = request.matrix(); + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + EXPECT_EQ(matrix.distances()[i], matrix_answers[i][1]) << "result " + std::to_string(i) + "'s distance is not equal to the expected value for TimeDistMatrix."; - EXPECT_EQ(tds[i].time(), matrix_answers[i].time) + EXPECT_EQ(matrix.times()[i], matrix_answers[i][0]) << "result " + std::to_string(i) + "'s time is not equal to the expected value for TimeDistMatrix"; } @@ -380,10 +380,10 @@ TEST(Matrix, partial_matrix) { TimeDistanceMatrix timedist_matrix; timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0, request.options().matrix_locations()); - auto& tds = request.matrix().time_distances(); + auto& matrix = request.matrix(); uint32_t found = 0; - for (uint32_t i = 0; i < tds.size(); ++i) { - if (tds[i].distance() > 0) { + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + if (matrix.distances()[i] > 0) { ++found; } } diff --git a/test/matrix_bss.cc b/test/matrix_bss.cc index 3b88aacb14..2bab15a4cf 100644 --- a/test/matrix_bss.cc +++ b/test/matrix_bss.cc @@ -138,8 +138,8 @@ class MatrixBssTest : public ::testing::Test { int route_length = legs.begin()->summary().length() * 1000; size_t m_result_idx = i * t_size + j; - int matrix_time = matrix_request.matrix().time_distances()[m_result_idx].time(); - int matrix_length = matrix_request.matrix().time_distances()[m_result_idx].distance(); + int matrix_time = matrix_request.matrix().times()[m_result_idx]; + int matrix_length = matrix_request.matrix().distances()[m_result_idx]; EXPECT_NEAR(matrix_time, route_time, kTimeThreshold); EXPECT_NEAR(matrix_length, route_length, route_length * kDistancePercentThreshold); diff --git a/valhalla/baldr/graphconstants.h b/valhalla/baldr/graphconstants.h index 6108a8a78e..9605ddda5f 100644 --- a/valhalla/baldr/graphconstants.h +++ b/valhalla/baldr/graphconstants.h @@ -2,6 +2,7 @@ #define VALHALLA_BALDR_GRAPHCONSTANTS_H_ #include +#include #include #include #include diff --git a/valhalla/thor/matrix_common.h b/valhalla/thor/matrix_common.h index efd7b4d5ec..fcbf2d13a8 100644 --- a/valhalla/thor/matrix_common.h +++ b/valhalla/thor/matrix_common.h @@ -56,23 +56,6 @@ struct Destination { } }; -// Time and Distance structure -struct TimeDistance { - uint32_t time; // Time in seconds - uint32_t dist; // Distance in meters - std::string date_time; - - TimeDistance() : time(0), dist(0), date_time("") { - } - - TimeDistance(const uint32_t secs, const uint32_t meters) : time(secs), dist(meters), date_time("") { - } - - TimeDistance(const uint32_t secs, const uint32_t meters, std::string date_time) - : time(secs), dist(meters), date_time(date_time) { - } -}; - // Will return a destination's date_time string inline std::string get_date_time(const std::string& origin_dt, const uint64_t& origin_tz, @@ -134,6 +117,14 @@ inline bool check_matrix_time(Api& request, const Matrix::Algorithm algo) { return false; } +// resizes all PBF sequences except for date_times +inline void reserve_pbf_arrays(valhalla::Matrix& matrix, size_t size) { + matrix.mutable_from_indices()->Resize(size, 0U); + matrix.mutable_to_indices()->Resize(size, 0U); + matrix.mutable_distances()->Resize(size, 0U); + matrix.mutable_times()->Resize(size, 0U); +} + } // namespace thor } // namespace valhalla diff --git a/valhalla/thor/timedistancematrix.h b/valhalla/thor/timedistancematrix.h index cbb05abaeb..230725a6cd 100644 --- a/valhalla/thor/timedistancematrix.h +++ b/valhalla/thor/timedistancematrix.h @@ -275,7 +275,8 @@ class TimeDistanceMatrix { const uint32_t origin_index, const std::string& origin_dt, const uint64_t& origin_tz, - const baldr::GraphId& pred_id); + const baldr::GraphId& pred_id, + std::vector& out_date_times); }; } // namespace thor From 90d2d5e6c69ea08410ef481534d74919054ab8fa Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Thu, 8 Jun 2023 19:07:08 +0200 Subject: [PATCH 11/14] tidy --- src/valhalla_run_matrix.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/valhalla_run_matrix.cc b/src/valhalla_run_matrix.cc index d1b944f5c8..91662a1132 100644 --- a/src/valhalla_run_matrix.cc +++ b/src/valhalla_run_matrix.cc @@ -49,7 +49,7 @@ void LogResults(const bool optimize, uint32_t idx1 = 0; uint32_t idx2 = 0; uint32_t nlocs = options.sources_size(); - for (uint32_t i; i < matrix.times().size(); i++) { + for (uint32_t i = 0; i < matrix.times().size(); i++) { auto distance = matrix.distances().Get(i); LOG_INFO(std::to_string(idx1) + "," + std::to_string(idx2) + ": Distance= " + std::to_string(distance) + " Time= " + GetFormattedTime(matrix.times().Get(i)) + @@ -65,8 +65,8 @@ void LogResults(const bool optimize, auto t10 = std::chrono::high_resolution_clock::now(); std::vector costs; costs.reserve(matrix.times().size()); - for (uint32_t i; i < matrix.times().size(); i++) { - costs.push_back(static_cast(matrix.times().Get(i))); + for (const auto& time : matrix.times()) { + costs.push_back(time); } Optimizer opt; From 31b010b66e3dd3fb5f179843abf7ebd58d742c25 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Thu, 8 Jun 2023 19:18:44 +0200 Subject: [PATCH 12/14] add a test for #4153 --- test/matrix.cc | 77 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 12 deletions(-) diff --git a/test/matrix.cc b/test/matrix.cc index faa0a77564..19b7d77e8d 100644 --- a/test/matrix.cc +++ b/test/matrix.cc @@ -172,12 +172,12 @@ const auto test_request_osrm = R"({ "costing":"auto" })"; -std::vector> matrix_answers = {{28, 28}, {2027, 1837}, {2403, 2213}, - {4163, 3838}, {1519, 1398}, {1808, 1638}, - {2061, 1951}, {3944, 3639}, {2311, 2111}, - {701, 641}, {0, 0}, {2821, 2626}, - {5562, 5177}, {3952, 3707}, {4367, 4107}, - {1825, 1680}}; +// clang-format off +std::vector> matrix_answers = {{28, 28}, {2027, 1837}, {2403, 2213}, {4163, 3838}, + {1519, 1398}, {1808, 1638}, {2061, 1951}, {3944, 3639}, + {2311, 2111}, {701, 641}, {0, 0}, {2821, 2626}, + {5562, 5177}, {3952, 3707}, {4367, 4107}, {1825, 1680}}; +// clang-format on } // namespace const uint32_t kThreshold = 1; @@ -255,7 +255,58 @@ TEST(Matrix, test_matrix) { } } -TEST(Matrix, test_timedistancematrix_results_sequence) { +TEST(Matrix, test_timedistancematrix_forward) { + // Input request is the same as `test_request`, but without the last target + const auto test_request_more_sources = R"({ + "sources":[ + {"lat":52.106337,"lon":5.101728}, + {"lat":52.111276,"lon":5.089717}, + {"lat":52.103105,"lon":5.081005} + ], + "targets":[ + {"lat":52.106126,"lon":5.101497}, + {"lat":52.100469,"lon":5.087099}, + {"lat":52.103105,"lon":5.081005}, + {"lat":52.094273,"lon":5.075254} + ], + "costing":"auto" + })"; + + loki_worker_t loki_worker(config); + + Api request; + ParseApi(test_request_more_sources, Options::sources_to_targets, request); + loki_worker.matrix(request); + thor_worker_t::adjust_scores(*request.mutable_options()); + + GraphReader reader(config.get_child("mjolnir")); + + sif::mode_costing_t mode_costing; + mode_costing[0] = + CreateSimpleCost(request.options().costings().find(request.options().costing_type())->second); + + TimeDistanceMatrix timedist_matrix; + timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); + auto& matrix = request.matrix(); + + // expected results are the same as `matrix_answers`, but without the last origin + // clang-format off + std::vector> expected_results = {{28, 28}, {2027, 1837}, {2403, 2213}, {4163, 3838}, + {1519, 1398}, {1808, 1638}, {2061, 1951}, {3944, 3639}, + {2311, 2111}, {701, 641}, {0, 0}, {2821, 2626}}; + // clang-format on + + for (uint32_t i = 0; i < matrix.times().size(); ++i) { + EXPECT_NEAR(matrix.distances()[i], expected_results[i][1], kThreshold) + << "result " + std::to_string(i) + "'s distance is not equal" + + " to expected value for TDMatrix"; + + EXPECT_NEAR(matrix.times()[i], expected_results[i][0], kThreshold) + << "result " + std::to_string(i) + "'s time is not equal" + " to expected value for TDMatrix"; + } +} + +TEST(Matrix, test_timedistancematrix_reverse) { // Input request is the same as `test_request`, but without the last target const auto test_request_more_sources = R"({ "sources":[ @@ -289,11 +340,13 @@ TEST(Matrix, test_timedistancematrix_results_sequence) { timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); auto& matrix = request.matrix(); - // expected results are the same as `matrix_answers`, but without the last column - std::vector> expected_results = { - {28, 28}, {2027, 1837}, {2403, 2213}, {1519, 1398}, {1808, 1638}, {2061, 1951}, - {2311, 2111}, {701, 641}, {0, 0}, {5562, 5177}, {3952, 3707}, {4367, 4107}, - }; + // expected results are the same as `matrix_answers`, but without the last target + // clang-format off + std::vector> expected_results = {{28, 28}, {2027, 1837}, {2403, 2213}, + {1519, 1398}, {1808, 1638}, {2061, 1951}, + {2311, 2111}, {701, 641}, {0, 0}, + {5562, 5177}, {3952, 3707}, {4367, 4107}}; + // clang-format on for (uint32_t i = 0; i < matrix.times().size(); ++i) { EXPECT_NEAR(matrix.distances()[i], expected_results[i][1], kThreshold) From d7cb82136c03d929774949cf85eb01a34150ef49 Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Fri, 9 Jun 2023 12:05:17 +0200 Subject: [PATCH 13/14] throw in test for osrm matrix format to increase coverage again.. --- src/tyr/matrix_serializer.cc | 7 +++-- test/matrix.cc | 50 +++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/tyr/matrix_serializer.cc b/src/tyr/matrix_serializer.cc index 7e6c3d083f..8ce5bc387f 100644 --- a/src/tyr/matrix_serializer.cc +++ b/src/tyr/matrix_serializer.cc @@ -48,7 +48,7 @@ json::ArrayPtr serialize_distance(const valhalla::Matrix& matrix, namespace osrm_serializers { // Serialize route response in OSRM compatible format. -std::string serialize(const Api& request, double distance_scale) { +std::string serialize(const Api& request) { auto json = json::map({}); auto time = json::array({}); auto distance = json::array({}); @@ -64,8 +64,7 @@ std::string serialize(const Api& request, double distance_scale) { time->emplace_back(serialize_duration(request.matrix(), source_index * options.targets_size(), options.targets_size())); distance->emplace_back(serialize_distance(request.matrix(), source_index * options.targets_size(), - options.targets_size(), source_index, 0, - distance_scale)); + options.targets_size(), source_index, 0, 1.0)); } json->emplace("durations", time); json->emplace("distances", distance); @@ -185,7 +184,7 @@ std::string serializeMatrix(Api& request) { double distance_scale = (request.options().units() == Options::miles) ? kMilePerMeter : kKmPerMeter; switch (request.options().format()) { case Options_Format_osrm: - return osrm_serializers::serialize(request, distance_scale); + return osrm_serializers::serialize(request); case Options_Format_json: return valhalla_serializers::serialize(request, distance_scale); case Options_Format_pbf: diff --git a/test/matrix.cc b/test/matrix.cc index 19b7d77e8d..a2d04b06ae 100644 --- a/test/matrix.cc +++ b/test/matrix.cc @@ -169,7 +169,8 @@ const auto test_request_osrm = R"({ {"lat":52.103105,"lon":5.081005}, {"lat":52.094273,"lon":5.075254} ], - "costing":"auto" + "costing":"auto", + "format": "osrm" })"; // clang-format off @@ -178,6 +179,23 @@ std::vector> matrix_answers = {{28, 28}, {2027, 1837}, {2311, 2111}, {701, 641}, {0, 0}, {2821, 2626}, {5562, 5177}, {3952, 3707}, {4367, 4107}, {1825, 1680}}; // clang-format on + +void check_osrm_response(std::string& res, std::string& algo) { + rapidjson::Document res_doc; + res_doc.Parse(res); + + ASSERT_FALSE(res_doc.HasParseError()); + + std::string status = "Ok"; + EXPECT_EQ(res_doc["code"].GetString(), status) << "Didn't work for " + algo; + EXPECT_EQ(res_doc["algorithm"].GetString(), algo) << "Didn't work for " + algo; + + EXPECT_NEAR(res_doc["distances"].GetArray()[0][0].GetDouble(), 28, 1) << "Didn't work for " + algo; + EXPECT_EQ(res_doc["durations"].GetArray()[0][0].GetInt64(), 28) << "Didn't work for " + algo; + EXPECT_NEAR(res_doc["distances"].GetArray()[3][3].GetDouble(), 1680, 1) + << "Didn't work for " + algo; + EXPECT_EQ(res_doc["durations"].GetArray()[3][3].GetInt64(), 1825) << "Didn't work for " + algo; +} } // namespace const uint32_t kThreshold = 1; @@ -358,8 +376,7 @@ TEST(Matrix, test_timedistancematrix_reverse) { } } -// TODO: it was commented before. Why? -TEST(Matrix, DISABLED_test_matrix_osrm) { +TEST(Matrix, test_matrix_osrm) { loki_worker_t loki_worker(config); Api request; @@ -376,30 +393,15 @@ TEST(Matrix, DISABLED_test_matrix_osrm) { CostMatrix cost_matrix; cost_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); - auto matrix = request.matrix(); - for (uint32_t i = 0; i < matrix.times().size(); ++i) { - EXPECT_EQ(matrix.distances()[i], matrix_answers[i][1]) - << "result " + std::to_string(i) + - "'s distance is not close enough to expected value for CostMatrix."; - - EXPECT_EQ(matrix.times()[i], matrix_answers[i][0]) - << "result " + std::to_string(i) + - "'s time is not close enough to expected value for CostMatrix."; - } - request.clear_matrix(); + auto json_res = tyr::serializeMatrix(request); + std::string algo = "costmatrix"; + check_osrm_response(json_res, algo); TimeDistanceMatrix timedist_matrix; timedist_matrix.SourceToTarget(request, reader, mode_costing, sif::TravelMode::kDrive, 400000.0); - matrix = request.matrix(); - for (uint32_t i = 0; i < matrix.times().size(); ++i) { - EXPECT_EQ(matrix.distances()[i], matrix_answers[i][1]) - << "result " + std::to_string(i) + - "'s distance is not equal to the expected value for TimeDistMatrix."; - - EXPECT_EQ(matrix.times()[i], matrix_answers[i][0]) - << "result " + std::to_string(i) + - "'s time is not equal to the expected value for TimeDistMatrix"; - } + json_res = tyr::serializeMatrix(request); + algo = "timedistancematrix"; + check_osrm_response(json_res, algo); } const auto test_request_partial = R"({ From 026428536374e9e463edd3d0e300c2f213d42bde Mon Sep 17 00:00:00 2001 From: nilsnolde Date: Tue, 13 Jun 2023 16:45:09 +0200 Subject: [PATCH 14/14] reserve pbf date_times --- valhalla/thor/matrix_common.h | 1 + 1 file changed, 1 insertion(+) diff --git a/valhalla/thor/matrix_common.h b/valhalla/thor/matrix_common.h index fcbf2d13a8..1afa4dbd21 100644 --- a/valhalla/thor/matrix_common.h +++ b/valhalla/thor/matrix_common.h @@ -123,6 +123,7 @@ inline void reserve_pbf_arrays(valhalla::Matrix& matrix, size_t size) { matrix.mutable_to_indices()->Resize(size, 0U); matrix.mutable_distances()->Resize(size, 0U); matrix.mutable_times()->Resize(size, 0U); + matrix.mutable_date_times()->Reserve(size); } } // namespace thor