Support live traffic data #2268

danpat · 2020-03-11T07:07:39Z

Issue

These changes add support for looking at real-time traffic data when performing route calculations.

Tasklist

Add tests
Add #fixes with the issue number that this PR addresses
Generally use squash merge to rebase and clean comments before merging
Update the changelog

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

valhalla/baldr/traffictile.h

…e separate mmap for traffic.

src/baldr/graphreader.cc

valhalla/baldr/traffictile.h

purew · 2020-03-11T20:22:14Z

valhalla/baldr/traffictile.h

+namespace valhalla {
+namespace baldr {
+namespace traffic {
+struct Speed {


Could we have some warning comments about any changes to the data layout has to be made with great care.

Would it be useful with a unittest to verify that it stays a POD? https://en.cppreference.com/w/cpp/types/is_pod

we have tests like this for the regular tiles as well and yeah valhalla only increments major version when we make breaking changes to the structurs. this would be subject to the same thing

purew · 2020-03-11T20:22:50Z

valhalla/baldr/traffictile.h

+};
+
+// per-speed-tile header
+struct TileHeader {


Same about warning comments here.

…ugs in TrafficTile initialization that the test uncovered.

…hat tests live-updating of the mmapp-ed traffic tar file (still WIP, no routing speed lookups implemented yet)

…ds. Still very much WIP, but modifying the live speeds file will update the router in-situ

.gitmodules

…exposed, and can be reset for test cases.

…binding generation with other languages.

valhalla/baldr/graphtile.h

… flags in GetSpeed(). Add tests for both

kevinkreiser · 2020-03-27T13:28:19Z

src/baldr/graphreader.cc

@@ -390,8 +413,12 @@ const GraphTile* GraphReader::GetGraphTile(const GraphId& graphid) {
      return nullptr;
    }

+    auto traffic_ptr = tile_extract_->traffic_tiles.find(base);


in the future we will probably need to decouple the idea of flat file and memory mapped for graph and traffic tiles. it would be nice to fall back to flat files for traffic tiles as well and then to fall back to network calls etc. but all that can come once we figure something out for cache invalidation for traffic tiles from file vs memmap. i just wanted to point out the future work here is all, not asking for changes

kevinkreiser · 2020-03-27T13:30:21Z

src/sif/autocost.cc

@@ -391,6 +391,10 @@ bool AutoCost::Allowed(const baldr::DirectedEdge* edge,
                       const uint64_t current_time,
                       const uint32_t tz_index,
                       bool& has_time_restrictions) const {
+
+  if (tile->IsClosedDueToTraffic(edge))


we need to add this to many more costings, all the others in this file (unless they all call this function via inheritance) as well as all in motorcycle, motorscooter and truck

purew · 2020-03-27T13:55:51Z

src/baldr/graphreader.cc

+          auto id = GraphTile::GetTileId(c.first);
+          tiles[id] = std::make_pair(const_cast<char*>(c.second.first), c.second.second);
+        } catch (...) {
+          // skip files we dont understand


Should we at least warn about there being things we don't understand?

…edgeid directly to reduce the work done by the check.

src/baldr/graphreader.cc

…why it's safe to do nothing on an exception here.

…t accidentally changed

mandeepsandhu · 2020-03-27T20:50:00Z

test/gurka/test_traffic.cc

+    // Traffic data works like this:
+    //   1. There is a separate .tar file containing tile entries matching the main tiles
+    //   2. Each tile is a fixed-size, with a header, and entries
+    // This loop iterates ofer the routing tiles, and creates blank


s/ofer/over

danpat · 2020-03-28T01:27:30Z

@kevinkreiser That windows build failure looks like a compiler bug:

https://developercommunity.visualstudio.com/content/problem/468431/error-c2580-multiple-versions-of-a-defaulted-speci.html

Should I put some effort into working around it?

kevinkreiser · 2020-03-29T17:07:39Z

@danpat at the very worst we can trivially implement those functions instead of using default. i actually wondered why we needed the functions in the structs at all. both speed and incident can be initialized with {}. in practice all of these data structures are read-only so it must be just for the tests?

kevinkreiser · 2020-03-29T17:17:37Z

valhalla/baldr/traffictile.h

+  const Speed getTrafficForDirectedEdge(const uint32_t directed_edge_offset) const {
+    if (header == nullptr)
+      return {};
+    assert(directed_edge_offset < header->directed_edge_count);


we should throw here that way a running server will catch and return an error rather than crash and burn.

kevinkreiser · 2020-03-29T17:18:48Z

valhalla/baldr/traffictile.h

+  Tile(const Tile& other) = default;
+  Tile& operator=(const Tile& other) = default;
+
+  const Speed getTrafficForDirectedEdge(const uint32_t directed_edge_offset) const {


index would be a more consistent name than offset here

… into the Speed class for less repetitinon.

kevinkreiser · 2020-03-30T01:25:28Z

valhalla/baldr/traffictile.h

    if (header == nullptr)
-      return {};
+      return INVALID_SPEED;


haha this is exactly what i did in my attempt earlier today to change this locally 😄

kevinkreiser · 2020-03-30T01:27:03Z

valhalla/baldr/graphtile.h

@@ -594,9 +594,9 @@ class GraphTile {
  inline bool IsClosedDueToTraffic(const GraphId& edge_id) const {
    if (!traffic_tile())
      return false;
-    auto live_speed = traffic_tile.getTrafficForDirectedEdge(edge_id.id());
+    auto& live_speed = traffic_tile.getTrafficForDirectedEdge(edge_id.id());


technically this auto includes volatile right? nitpick but it wouldnt hurt to put it just for understandings sake or easy readabiltiy

valhalla/baldr/traffictile.h

…ems on x86_64), and add a couple more for future-proofing.

…ct layout.

kevinkreiser · 2020-03-30T13:26:35Z

other than the assert this looks ready to go!

…or on, not a precondition

* master: Support live traffic data (#2268) Update CHANGELOG.md Properly utilize CircleCI caches per build (#2288) Time allowed updates & Gurka Conditional Restrictions Test (#2286)

Loading/initialization logic for traffic tiles.

e185489

kevinkreiser reviewed Mar 11, 2020

View reviewed changes

valhalla/baldr/traffictile.h Outdated Show resolved Hide resolved

kevinkreiser reviewed Mar 11, 2020

View reviewed changes

valhalla/baldr/traffictile.h Show resolved Hide resolved

danpat added 2 commits March 11, 2020 08:49

Fix double-buffer implementation, simplify pointer population, and us…

9eeb3d1

…e separate mmap for traffic.

Merge branch 'master' into danpat_live_traffic

0535a13

mandeepsandhu reviewed Mar 11, 2020

View reviewed changes

src/baldr/graphreader.cc Outdated Show resolved Hide resolved

valhalla/baldr/traffictile.h Outdated Show resolved Hide resolved

valhalla/baldr/traffictile.h Outdated Show resolved Hide resolved

valhalla/baldr/traffictile.h Outdated Show resolved Hide resolved

purew reviewed Mar 11, 2020

View reviewed changes

valhalla/baldr/traffictile.h

};

// per-speed-tile header

struct TileHeader {

Copy link

Contributor

purew Mar 11, 2020

Choose a reason for hiding this comment

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

Same about warning comments here.

danpat added 3 commits March 11, 2020 18:12

Add test for Traffic Tile pointer mapping/offset calc, and fix some b…

97b3ce0

…ugs in TrafficTile initialization that the test uncovered.

Add tests for traffic tile initialization, and also a skeleton test t…

fee047f

…hat tests live-updating of the mmapp-ed traffic tar file (still WIP, no routing speed lookups implemented yet)

Add basic working (no time factor considered yet) live updatable spee…

60eb6eb

…ds. Still very much WIP, but modifying the live speeds file will update the router in-situ

mandeepsandhu reviewed Mar 16, 2020

View reviewed changes

.gitmodules Show resolved Hide resolved

danpat and others added 9 commits March 17, 2020 16:30

Refactor GraphReader a little bit so that the static tile_extract is …

14f3665

…exposed, and can be reset for test cases.

Include traffic tarfile in valhalla_build_config

1748c2f

Make traffictile.h conditionally a simple POD-like header for easier …

fed4727

…binding generation with other languages.

Make interface C-only if C_ONLY_INTERFACE is defined

db21912

Cleanup extern C

a02978d

Put GetSpeed back in the header as an inlined function

27ee1f4

Fix astar tests after changes to gurka::findEdge

e044e03

Fix missing header and formatting.

09d8b8d

Fix typo

68b5511

kevinkreiser reviewed Mar 24, 2020

View reviewed changes

valhalla/baldr/graphtile.h Outdated Show resolved Hide resolved

danpat added 4 commits March 26, 2020 14:54

Merge branch 'master' into danpat_live_traffic

fe85c7e

Set proper access on auto costing with live traffic. Also honour flow…

ffd1f36

… flags in GetSpeed(). Add tests for both

Merge branch 'master' into danpat_live_traffic

7080b79

Update CHANGELOG

6fc7afd

danpat marked this pull request as ready for review March 27, 2020 01:50

kevinkreiser reviewed Mar 27, 2020

View reviewed changes

purew reviewed Mar 27, 2020

View reviewed changes

danpat added 2 commits March 27, 2020 11:10

Add traffic closure checking to all relevant costing models, and use …

d432d21

…edgeid directly to reduce the work done by the check.

Log a warning when we can't parse a tile path in a tarfile

76c8b38

kevinkreiser reviewed Mar 27, 2020

View reviewed changes

src/baldr/graphreader.cc Outdated Show resolved Hide resolved

danpat added 4 commits March 27, 2020 11:47

Revert log_warn and put an annoyingly verbose comment in place about …

0248aab

…why it's safe to do nothing on an exception here.

Add some simple compile-time checks to ensure struct layouts don't ge…

ec49cc7

…t accidentally changed

Fix test comment.

a1229e0

Make traffic test descriptions more verbose.

9192b3f

mandeepsandhu reviewed Mar 27, 2020

View reviewed changes

kevinkreiser reviewed Mar 29, 2020

View reviewed changes

Remove incidents for now (will implement later). Move validity checks…

437fc96

… into the Speed class for less repetitinon.

kevinkreiser reviewed Mar 30, 2020

View reviewed changes

valhalla/baldr/traffictile.h Outdated Show resolved Hide resolved

danpat added 4 commits March 29, 2020 18:32

Remove is_scale for now, we're not using it - reclaim bit as spare.

e6b489b

Reorder tile header so spares are at the end (and avoid packing probl…

b3603bf

…ems on x86_64), and add a couple more for future-proofing.

Fix initialization of INVALID_SPEED which changed with the Speed stru…

1414bfc

…ct layout.

Fix unit test for traffic structs.

db71134

danpat added 3 commits March 30, 2020 07:59

Replace assert with exception - this is a runtime check we should err…

9253a7d

…or on, not a precondition

Use .age=0 as a validity flag instead of congestion.

a4a1182

Add some constants and helpers

8a6ed01

kevinkreiser approved these changes Mar 30, 2020

View reviewed changes

danpat merged commit 5f5588f into master Mar 30, 2020

danpat deleted the danpat_live_traffic branch March 30, 2020 17:49

AlexKaravaev mentioned this pull request May 26, 2020

Update real-time speed docs. #2163

Closed

LHBruneton-C2C mentioned this pull request Feb 1, 2021

Dynamic speed 'how to' guide #2823

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support live traffic data #2268

Support live traffic data #2268

danpat commented Mar 11, 2020 •

edited

Loading

purew Mar 11, 2020

purew Mar 11, 2020

kevinkreiser Mar 18, 2020

purew Mar 11, 2020

kevinkreiser Mar 27, 2020

kevinkreiser Mar 27, 2020 •

edited

Loading

purew Mar 27, 2020

mandeepsandhu Mar 27, 2020

danpat commented Mar 28, 2020

kevinkreiser commented Mar 29, 2020

kevinkreiser Mar 29, 2020

kevinkreiser Mar 29, 2020

kevinkreiser Mar 30, 2020

kevinkreiser Mar 30, 2020

kevinkreiser commented Mar 30, 2020

Support live traffic data #2268

Support live traffic data #2268

Conversation

danpat commented Mar 11, 2020 • edited Loading

Issue

Tasklist

Requirements / Relations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkreiser Mar 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danpat commented Mar 28, 2020

kevinkreiser commented Mar 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkreiser commented Mar 30, 2020

danpat commented Mar 11, 2020 •

edited

Loading

kevinkreiser Mar 27, 2020 •

edited

Loading