Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

matrix pbf output #4121

Merged
merged 19 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ffcd93a
start with matrix pbf output; only specified the proto schema so far …
nilsnolde May 20, 2023
6331656
only change the matrix functions signatures to accept an Api object r…
nilsnolde May 20, 2023
8cec20c
finish the implementation; adapt all matrix algos to keep track of th…
nilsnolde May 21, 2023
fafd730
remove sources & targets from the output, add changelog
nilsnolde May 21, 2023
f124625
fix valhalla_run_matrix & remove some log statements
nilsnolde May 21, 2023
e2ccd88
fix benchmkar
nilsnolde May 22, 2023
6e507ef
hmm.. matrix tests are failing.. look into it..
nilsnolde May 22, 2023
3e88d7b
todo: proper order for reverse td matrix & off-by-one issues in dista…
nilsnolde May 23, 2023
41e3065
[skip ci] start the work to let proto pack the matrix pbf better
nilsnolde Jun 6, 2023
79d2fae
revert to flat repeated fields in the matrix PBF in favor of a repeat…
nilsnolde Jun 8, 2023
46733c4
Merge branch 'nn-matrix-pbf' of github.com:valhalla/valhalla into nn-…
nilsnolde Jun 8, 2023
ef053ef
Merge branch 'master' into nn-matrix-pbf
nilsnolde Jun 8, 2023
90d2d5e
tidy
nilsnolde Jun 8, 2023
31b010b
add a test for #4153
nilsnolde Jun 8, 2023
d7cb821
throw in test for osrm matrix format to increase coverage again..
nilsnolde Jun 9, 2023
0264285
reserve pbf date_times
nilsnolde Jun 13, 2023
2d5caf0
Merge branch 'master' into nn-matrix-pbf
nilsnolde Jun 17, 2023
44a0369
Merge branch 'master' into nn-matrix-pbf
nilsnolde Jul 4, 2023
49b4429
Merge branch 'master' into nn-matrix-pbf
nilsnolde Jul 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* UPDATED: French translations, thanks to @xlqian [#4159](https://github.com/valhalla/valhalla/pull/4159)
* ADDED: CI runs a spell check on the PR to detect spelling mistakes [#4179](https://github.com/valhalla/valhalla/pull/4179)
* ADDED: `preferred_side_cutoff` parameter for locations [#4182](https://github.com/valhalla/valhalla/pull/4182)
* ADDED: PBF output for matrix endpoint [#4121](https://github.com/valhalla/valhalla/pull/4121)

## Release Date: 2023-05-11 Valhalla 3.4.0
* **Removed**
Expand Down
13 changes: 5 additions & 8 deletions bench/thor/costmatrix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,33 +80,30 @@ 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);
sif::TravelMode mode;
auto costs = sif::CostFactory().CreateModeCosting(options, mode);
auto cost = costs[static_cast<size_t>(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<valhalla::Location> 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);
}
Expand Down
3 changes: 2 additions & 1 deletion proto/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions proto/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions proto/matrix.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
syntax = "proto3";
option optimize_for = LITE_RUNTIME;
package valhalla;
import public "common.proto";

message Matrix {
enum Algorithm {
TimeDistanceMatrix = 0;
CostMatrix = 1;
}

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;
}
2 changes: 1 addition & 1 deletion proto/options.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/proto_conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
46 changes: 25 additions & 21 deletions src/thor/costmatrix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,23 +119,25 @@ void CostMatrix::clear() {

// Form a time distance matrix from the set of source locations
// to the set of target locations.
std::vector<TimeDistance> CostMatrix::SourceToTarget(
google::protobuf::RepeatedPtrField<valhalla::Location>& source_location_list,
google::protobuf::RepeatedPtrField<valhalla::Location>& 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) {
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");
request.mutable_matrix()->set_algorithm(Matrix::CostMatrix);

// Set the mode and costing
mode_ = mode;
costing_ = mode_costing[static_cast<uint32_t>(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);
Expand Down Expand Up @@ -230,24 +232,26 @@ std::vector<TimeDistance> CostMatrix::SourceToTarget(
RecostPaths(graphreader, source_location_list, target_location_list, time_infos, invariant);
}

// Form the time, distance matrix from the destinations list
std::vector<TimeDistance> 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_) {
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<uint64_t>(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));
}
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<uint64_t>(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++;
}
return td;
}

// Initialize all time distance to "not found". Any locations that
Expand Down
47 changes: 21 additions & 26 deletions src/thor/matrix_action.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,27 @@
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) {
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,
max_matrix_distance.find(costing)->second,
options.matrix_locations());
return tyr::serializeMatrix(request, time_distances, distance_scale, MatrixType::TimeDist);
time_distance_bss_matrix_.SourceToTarget(request, *reader, mode_costing, mode,
max_matrix_distance.find(costing)->second,

Check warning on line 45 in src/thor/matrix_action.cc

View check run for this annotation

Codecov / codecov/patch

src/thor/matrix_action.cc#L44-L45

Added lines #L44 - L45 were not covered by tests
options.matrix_locations());
return tyr::serializeMatrix(request);

Check warning on line 47 in src/thor/matrix_action.cc

View check run for this annotation

Codecov / codecov/patch

src/thor/matrix_action.cc#L47

Added line #L47 was not covered by tests
}

MatrixType matrix_type = MatrixType::Cost;
Matrix::Algorithm matrix_algo = Matrix::CostMatrix;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the MatrixType enum we kept around and replaced it with the PBF's one.

switch (source_to_target_algorithm) {
case SELECT_OPTIMAL:
// TODO - Do further performance testing to pick the best algorithm for the job
Expand All @@ -64,11 +58,11 @@
// 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;

Check warning on line 65 in src/thor/matrix_action.cc

View check run for this annotation

Codecov / codecov/patch

src/thor/matrix_action.cc#L65

Added line #L65 was not covered by tests
break;
default:
break;
Expand All @@ -77,32 +71,33 @@
case COST_MATRIX:
break;
case TIME_DISTANCE_MATRIX:
matrix_type = MatrixType::TimeDist;
matrix_algo = Matrix::TimeDistanceMatrix;

Check warning on line 74 in src/thor/matrix_action.cc

View check run for this annotation

Codecov / codecov/patch

src/thor/matrix_action.cc#L74

Added line #L74 was not covered by tests
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);
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) {
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, MatrixType::Cost);
costmatrix(has_time);
} else {
if (has_time && options.prioritize_bidirectional()) {
add_warning(request, 300);
}
return tyr::serializeMatrix(request, timedistancematrix(), distance_scale, MatrixType::TimeDist);
timedistancematrix();
}
return tyr::serializeMatrix(request);
}
} // namespace thor
} // namespace valhalla
16 changes: 8 additions & 8 deletions src/thor/optimized_route_action.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<TimeDistance> 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),
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 =
Expand All @@ -41,17 +40,18 @@ void thor_worker_t::optimized_route(Api& request) {
// Set time costs to send to Optimizer.
std::vector<float> time_costs;
bool reachable = true;
for (size_t i = 0; i < td.size(); ++i) {
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) {
if (!reachable) {
throw valhalla_exception_t{441, " at index " + std::to_string(i / correlated.size())};
};
reachable = false;
}
reachable = reachable || td[i].time != kMaxCost;
reachable = reachable || tds.Get(i) != kMaxCost;
// Keep the times for the reordering
time_costs.emplace_back(static_cast<float>(td[i].time));
time_costs.emplace_back(static_cast<float>(tds.Get(i)));
}

Optimizer optimizer;
Expand Down