Skip to content

Commit 4abbb79

Browse files
kevinkreiserdanpaz
kevinkreiser
and
danpaz
authored
bidirectional a* routing with specified time (valhalla#2660)
* make it so that you can specify you want to use the same time for the whole route * let bidirectional use the localtime without accounting for time travel * lint away a gotch for future us * move historical traffic customization into unit test helper function * force triplegbuilder to name the algorithms used in making the leg * more structure in tests * add unit tests for algorithm selection * try reorganizing test target common dependencies * make just one test library target * fix reach will port to another pr * add traffic to the test, future proof time tracking in bidira* as per comments * fix tests. still need lint * tidy * more tidy with one tidy skip * satisfy clang on osx * more overrides to make apple happy * even more overrides to make apple happy * even more overrides to make apple happy * more lint * cleanup items mention in review * lint * fix typos, hardened tests found a missing spot in bidira* * tidy * for older protobuf * comment about how now timedep failover to bidira* is somewhat timedependent instead of completely time independent * small cleanups for predicted speed decoding * changelog entry Co-authored-by: Daniel Paz-Soldan <[email protected]>
1 parent d64ad33 commit 4abbb79

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

76 files changed

+2018
-1371
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@
191191
* CHANGED: Move turn_lane_direction helper to odin/util [#2675](https://github.com/valhalla/valhalla/pull/2675)
192192
* ADDED: Add annotations to osrm response including speed limits, unit and sign conventions [#2668](https://github.com/valhalla/valhalla/pull/2668)
193193
* ADDED: Added functions for predicted speeds encoding-decoding [#2674](https://github.com/valhalla/valhalla/pull/2674)
194+
* ADDED: Time invariant routing via the bidirectional algorithm. This has the effect that when time dependent routes (arrive_by and depart_at) fall back to bidirectional due to length restrictions they will actually use the correct time of day for one of the search directions [#2660](https://github.com/valhalla/valhalla/pull/2660)
194195

195196
## Release Date: 2019-11-21 Valhalla 3.0.9
196197
* **Bug Fix**

‎bench/CMakeLists.txt

+2-7
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,13 @@ add_dependencies(benchmarks utrecht_tiles)
1717
macro (add_valhalla_benchmark target_file)
1818
set(target_name benchmark-${target_file})
1919
add_executable(${target_name}
20-
${target_file}.cc
21-
${VALHALLA_SOURCE_DIR}/third_party/microtar/src/microtar.c)
20+
${target_file}.cc)
2221
set_target_properties(${target_name} PROPERTIES FOLDER "Benchmarks")
2322
set_target_properties(${target_name}
2423
PROPERTIES
2524
COMPILE_DEFINITIONS
2625
VALHALLA_SOURCE_DIR="${VALHALLA_SOURCE_DIR}/")
27-
target_link_libraries(${target_name} valhalla benchmark::benchmark)
26+
target_link_libraries(${target_name} valhalla_test benchmark::benchmark)
2827
add_dependencies(benchmarks ${target_name})
2928
add_dependencies(${target_name} utrecht_tiles)
3029
# Add a custom target running the benchmark
@@ -36,10 +35,6 @@ macro (add_valhalla_benchmark target_file)
3635
set_target_properties(run-${target_name} PROPERTIES FOLDER "Benchmarks")
3736
add_dependencies(run-${target_name} utrecht_tiles)
3837
add_dependencies(run-benchmarks run-${target_name})
39-
target_include_directories(benchmark-${target_file} SYSTEM PRIVATE
40-
${VALHALLA_SOURCE_DIR}/third_party/microtar/src
41-
${VALHALLA_SOURCE_DIR}/third_party/protozero/include
42-
${VALHALLA_SOURCE_DIR}/third_party/libosmium/include)
4338
endmacro()
4439

4540
add_subdirectory(meili)

‎bench/thor/routes.cc

+8-8
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include "midgard/pointll.h"
1010
#include "sif/autocost.h"
1111
#include "sif/costfactory.h"
12-
#include "test/util/traffic_utils.h"
12+
#include "test.h"
1313
#include "thor/bidirectional_astar.h"
1414
#include <valhalla/proto/options.pb.h>
1515

@@ -83,7 +83,7 @@ constexpr float kMaxRange = 256;
8383

8484
static void BM_UtrechtBidirectionalAstar(benchmark::State& state) {
8585
const auto config = build_config("generated-live-data.tar");
86-
valhalla_tests::utils::build_live_traffic_data(config);
86+
test::build_live_traffic_data(config);
8787

8888
std::mt19937 gen(0); // Seed with the same value for consistent benchmarking
8989
{
@@ -105,10 +105,10 @@ static void BM_UtrechtBidirectionalAstar(benchmark::State& state) {
105105
} else {
106106
}
107107
};
108-
valhalla_tests::utils::customize_live_traffic_data(config, generate_traffic);
108+
test::customize_live_traffic_data(config, generate_traffic);
109109
}
110110

111-
auto clean_reader = valhalla_tests::utils::make_clean_graphreader(config.get_child("mjolnir"));
111+
auto clean_reader = test::make_clean_graphreader(config.get_child("mjolnir"));
112112

113113
// Generate N random route queries within the Utrect bounding box;
114114
const int N = 2;
@@ -191,7 +191,7 @@ static void BM_UtrechtBidirectionalAstar(benchmark::State& state) {
191191
void customize_traffic(const boost::property_tree::ptree& config,
192192
baldr::GraphId& target_edge_id,
193193
const int target_speed) {
194-
valhalla_tests::utils::build_live_traffic_data(config);
194+
test::build_live_traffic_data(config);
195195
// Make some updates to the traffic .tar file.
196196
// Generate traffic data
197197
std::function<void(baldr::GraphReader&, baldr::TrafficTile&, int, baldr::TrafficSpeed*)>
@@ -206,7 +206,7 @@ void customize_traffic(const boost::property_tree::ptree& config,
206206
current->speed1 = target_speed >> 1;
207207
}
208208
};
209-
valhalla_tests::utils::customize_live_traffic_data(config, generate_traffic);
209+
test::customize_live_traffic_data(config, generate_traffic);
210210
}
211211

212212
BENCHMARK(BM_UtrechtBidirectionalAstar)->Unit(benchmark::kMillisecond);
@@ -219,7 +219,7 @@ static void BM_GetSpeed(benchmark::State& state) {
219219
const auto tgt_speed = 50;
220220
customize_traffic(config, tgt_edge_id, tgt_speed);
221221

222-
auto clean_reader = valhalla_tests::utils::make_clean_graphreader(config.get_child("mjolnir"));
222+
auto clean_reader = test::make_clean_graphreader(config.get_child("mjolnir"));
223223

224224
auto tile = clean_reader->GetGraphTile(baldr::GraphId(tgt_edge_id));
225225
if (tile == nullptr) {
@@ -251,7 +251,7 @@ static void BM_Sif_Allowed(benchmark::State& state) {
251251
auto tgt_speed = 100;
252252
customize_traffic(config, tgt_edge_id, tgt_speed);
253253

254-
auto clean_reader = valhalla_tests::utils::make_clean_graphreader(config.get_child("mjolnir"));
254+
auto clean_reader = test::make_clean_graphreader(config.get_child("mjolnir"));
255255

256256
Options options;
257257
create_costing_options(options);

‎docs/api/turn-by-turn/api-reference.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ A multimodal request with a filter for certain Onestop IDs:
267267
| Options | Description |
268268
| :------------------ | :----------- |
269269
| `avoid_locations` | A set of locations to exclude or avoid within a route can be specified using a JSON array of avoid_locations. The avoid_locations have the same format as the locations list. At a minimum each avoid location must include latitude and longitude. The avoid_locations are mapped to the closest road or roads and these roads are excluded from the route path computation.|
270-
| `date_time` | This is the local date and time at the location.<ul><li>`type`<ul><li>0 - Current departure time.</li><li>1 - Specified departure time</li><li>2 - Specified arrival time. Not yet implemented for multimodal costing method.</li></ul></li><li>`value` - the date and time is specified in ISO 8601 format (YYYY-MM-DDThh:mm) in the local time zone of departure or arrival. For example "2016-07-03T08:06"</li></ul><ul><b>NOTE: This option is not supported for Valhalla's matrix service.</b><ul> |
270+
| `date_time` | This is the local date and time at the location.<ul><li>`type`<ul><li>0 - Current departure time.</li><li>1 - Specified departure time</li><li>2 - Specified arrival time. Not yet implemented for multimodal costing method.</li></li>3 - Invariant specified time. Time does not vary over the course of the path. Not implemented for multimodal or bike share routing</li></ul></li><li>`value` - the date and time is specified in ISO 8601 format (YYYY-MM-DDThh:mm) in the local time zone of departure or arrival. For example "2016-07-03T08:06"</li></ul><ul><b>NOTE: This option is not supported for Valhalla's matrix service.</b><ul> |
271271
| `out_format` | Output format. If no `out_format` is specified, JSON is returned. Future work includes PBF (protocol buffer) support. |
272272
| `id` | Name your route request. If `id` is specified, the naming will be sent thru to the response. |
273273
| `linear_references` | When present and `true`, the successful `route` response will include a key `linear_references`. Its value is an array of base64-encoded [OpenLR location references][openlr], one for each graph edge of the road network matched by the input trace. |

‎proto/options.proto

+1
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ message Options {
156156
current = 0;
157157
depart_at = 1;
158158
arrive_by = 2;
159+
invariant = 3;
159160
}
160161

161162
optional Units units = 1; // kilometers or miles

‎proto/trip.proto

+1
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ message TripLeg {
327327
optional BoundingBox bbox = 9;
328328
optional ShapeAttributes shape_attributes = 10;
329329
repeated Incident incidents = 11;
330+
repeated string algorithms = 12;
330331
}
331332

332333
message TripRoute {

‎src/baldr/predictedspeeds.cc

+7-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace valhalla {
44
namespace baldr {
5+
56
// DCT-III constants for speed decoding and normalization
67
constexpr float k1OverSqrt2 = 0.707106781f; // 1 / sqrt(2)
78
constexpr float kPiBucketConstant = 3.14159265f / 2016.0f;
@@ -51,18 +52,19 @@ class BucketCosTable final {
5152
};
5253

5354
std::array<int16_t, kCoefficientCount> compress_speed_buckets(const float* speeds) {
54-
const auto& cost_table = BucketCosTable::GetInstance();
5555
std::array<float, kCoefficientCount> coefficients;
5656
coefficients.fill(0.f);
57+
5758
// DCT-II with speed normalization
5859
for (uint32_t bucket = 0; bucket < kBucketsPerWeek; ++bucket) {
5960
// Get a pointer to the precomputed cos values for this bucket
60-
const float* cos_values = cost_table.get(bucket);
61+
const float* cos_values = BucketCosTable::GetInstance().get(bucket);
6162
for (uint32_t c = 0; c < kCoefficientCount; ++c) {
6263
coefficients[c] += cos_values[c] * speeds[bucket];
6364
}
6465
}
6566
coefficients[0] *= k1OverSqrt2;
67+
6668
std::array<int16_t, kCoefficientCount> result;
6769
for (size_t i = 0; i < coefficients.size(); ++i) {
6870
result[i] = static_cast<int16_t>(roundf(kSpeedNormalization * coefficients[i]));
@@ -76,9 +78,8 @@ float decompress_speed_bucket(const int16_t* coefficients, uint32_t bucket_idx)
7678

7779
// DCT-III with speed normalization
7880
float speed = *coefficients * k1OverSqrt2;
79-
++coefficients;
80-
++b;
81-
for (uint32_t k = 1; k < kCoefficientCount; ++k, ++coefficients, ++b) {
81+
const auto* coef_end = coefficients + kCoefficientCount;
82+
for (++b, ++coefficients; coefficients < coef_end; ++coefficients, ++b) {
8283
speed += *coefficients * *b;
8384
}
8485
return speed * kSpeedNormalization;
@@ -113,4 +114,4 @@ std::array<int16_t, kCoefficientCount> decode_compressed_speeds(const std::strin
113114
}
114115

115116
} // namespace baldr
116-
} // namespace valhalla
117+
} // namespace valhalla

‎src/loki/trace_route_action.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,7 @@ void loki_worker_t::init_trace(Api& request) {
174174

175175
void loki_worker_t::trace(Api& request) {
176176
init_trace(request);
177-
const auto& costing = Costing_Enum_Name(request.options().costing());
178-
if (costing == "multimodal") {
177+
if (request.options().costing() == Costing::multimodal) {
179178
throw valhalla_exception_t{140, Options_Action_Enum_Name(request.options().action())};
180179
};
181180
}

‎src/mjolnir/pbfgraphparser.cc

+13-14
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ constexpr uint32_t kAbsurdRoadClass = 777777;
3737

3838
// Convenience method to get a number from a string. Uses try/catch in case
3939
// stoi throws an exception
40-
int get_number(const std::string& tag, const std::string& value) {
40+
int get_number(const std::string& tag, const std::string& value) { // NOLINT
4141
int num = -1;
4242
try {
4343
num = stoi(value);
@@ -682,18 +682,18 @@ struct graph_callback : public OSMPBF::Callback {
682682
tag_handlers_["bridge"] = [this]() { way_.set_bridge(tag_.second == "true" ? true : false); };
683683
tag_handlers_["seasonal"] = [this]() { way_.set_seasonal(tag_.second == "true" ? true : false); };
684684
tag_handlers_["bike_network_mask"] = [this]() { way_.set_bike_network(std::stoi(tag_.second)); };
685-
tag_handlers_["bike_national_ref"] = [this]() {
686-
if (!tag_.second.empty())
687-
way_.set_bike_national_ref_index(osmdata_.name_offset_map.index(tag_.second));
688-
};
689-
tag_handlers_["bike_regional_ref"] = [this]() {
690-
if (!tag_.second.empty())
691-
way_.set_bike_regional_ref_index(osmdata_.name_offset_map.index(tag_.second));
692-
};
693-
tag_handlers_["bike_local_ref"] = [this]() {
694-
if (!tag_.second.empty())
695-
way_.set_bike_local_ref_index(osmdata_.name_offset_map.index(tag_.second));
696-
};
685+
// tag_handlers_["bike_national_ref"] = [this]() {
686+
// if (!tag_.second.empty())
687+
// way_.set_bike_national_ref_index(osmdata_.name_offset_map.index(tag_.second));
688+
// };
689+
// tag_handlers_["bike_regional_ref"] = [this]() {
690+
// if (!tag_.second.empty())
691+
// way_.set_bike_regional_ref_index(osmdata_.name_offset_map.index(tag_.second));
692+
// };
693+
// tag_handlers_["bike_local_ref"] = [this]() {
694+
// if (!tag_.second.empty())
695+
// way_.set_bike_local_ref_index(osmdata_.name_offset_map.index(tag_.second));
696+
// };
697697
tag_handlers_["destination"] = [this]() {
698698
if (!tag_.second.empty()) {
699699
way_.set_destination_index(osmdata_.name_offset_map.index(tag_.second));
@@ -2156,7 +2156,6 @@ void PBFGraphParser::ParseRelations(const boost::property_tree::ptree& pt,
21562156

21572157
void PBFGraphParser::ParseNodes(const boost::property_tree::ptree& pt,
21582158
const std::vector<std::string>& input_files,
2159-
const std::string& ways_file,
21602159
const std::string& way_nodes_file,
21612160
const std::string& bss_nodes_file,
21622161
OSMData& osmdata) {

‎src/mjolnir/util.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,8 @@ bool build_tile_set(const boost::property_tree::ptree& config,
267267
if (start_stage <= BuildStage::kParseNodes && BuildStage::kParseNodes <= end_stage) {
268268
// Read the OSM protocol buffer file. Callbacks for nodes
269269
// are defined within the PBFParser class
270-
PBFGraphParser::ParseNodes(config.get_child("mjolnir"), input_files, ways_bin, way_nodes_bin,
271-
bss_nodes_bin, osm_data);
270+
PBFGraphParser::ParseNodes(config.get_child("mjolnir"), input_files, way_nodes_bin, bss_nodes_bin,
271+
osm_data);
272272

273273
// Free all protobuf memory - cannot use the protobuffer lib after this!
274274
if (release_osmpbf_memory) {

‎src/sif/autocost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include <cassert>
1313

1414
#ifdef INLINE_TEST
15-
#include "test/test.h"
15+
#include "test.h"
1616
#include "worker.h"
1717
#include <random>
1818
#endif

‎src/sif/bicyclecost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#include <cassert>
1111

1212
#ifdef INLINE_TEST
13-
#include "test/test.h"
13+
#include "test.h"
1414
#include "worker.h"
1515
#include <random>
1616
#endif

‎src/sif/motorcyclecost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#include <cassert>
1111

1212
#ifdef INLINE_TEST
13-
#include "test/test.h"
13+
#include "test.h"
1414
#include "worker.h"
1515
#include <random>
1616
#endif

‎src/sif/motorscootercost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#include <cassert>
1111

1212
#ifdef INLINE_TEST
13-
#include "test/test.h"
13+
#include "test.h"
1414
#include "worker.h"
1515
#include <random>
1616
#endif

‎src/sif/nocost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#include "sif/dynamiccost.h"
99

1010
#ifdef INLINE_TEST
11-
#include "test/test.h"
11+
#include "test.h"
1212
#include "worker.h"
1313
#include <random>
1414
#endif

‎src/sif/pedestriancost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#include "sif/costconstants.h"
99

1010
#ifdef INLINE_TEST
11-
#include "test/test.h"
11+
#include "test.h"
1212
#include "worker.h"
1313
#include <random>
1414
#endif

‎src/sif/transitcost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include "worker.h"
88

99
#ifdef INLINE_TEST
10-
#include "test/test.h"
10+
#include "test.h"
1111
#include <random>
1212
#endif
1313

‎src/sif/truckcost.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include <cassert>
1010

1111
#ifdef INLINE_TEST
12-
#include "test/test.h"
12+
#include "test.h"
1313
#include "worker.h"
1414
#include <random>
1515
#endif

‎src/thor/astar.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ void AStarPathAlgorithm::Init(const midgard::PointLL& origll, const midgard::Poi
7979
// the destination (increase distance for lower densities and decrease
8080
// for higher densities) and the distance between origin and destination
8181
// (increase for shorter distances).
82-
void AStarPathAlgorithm::ModifyHierarchyLimits(const float dist, const uint32_t density) {
82+
void AStarPathAlgorithm::ModifyHierarchyLimits(const float dist, const uint32_t /*density*/) {
8383
// TODO - default distance below which we increase expansion within
8484
// distance. This is somewhat temporary to address route quality on shorter
8585
// routes - hopefully we will mark the data somehow to indicate how to
@@ -247,7 +247,7 @@ AStarPathAlgorithm::GetBestPath(valhalla::Location& origin,
247247
GraphReader& graphreader,
248248
const sif::mode_costing_t& mode_costing,
249249
const TravelMode mode,
250-
const Options& options) {
250+
const Options& /*options*/) {
251251
// Set the mode and costing
252252
mode_ = mode;
253253
costing_ = mode_costing[static_cast<uint32_t>(mode_)];
@@ -348,7 +348,7 @@ AStarPathAlgorithm::GetBestPath(valhalla::Location& origin,
348348

349349
// Add an edge at the origin to the adjacency list
350350
void AStarPathAlgorithm::SetOrigin(GraphReader& graphreader,
351-
valhalla::Location& origin,
351+
const valhalla::Location& origin,
352352
const valhalla::Location& destination,
353353
const uint32_t seconds_of_week) {
354354
// Only skip inbound edges if we have other options

0 commit comments

Comments
 (0)