Use Timestamp Information to Limit the Route Between States in the HMM #840

kevinkreiser · 2017-07-20T17:57:12Z

the whole point of this is that it fixes #620

given a trace with time information at each point, we can in a similar way to what is done with max_route_distance_factor, disallow the adding of edge labels to the route if the route is too long in estimated time with respect to the actual time given in the measurements. basically we prune the route search space to not entertain paths that traveled too much distance or time much time with some factor of the original time and distance traveled between two measurements.

there are a couple of other things in this pr as well...

the first is that i've updated the trace_* actions to parse the time, accuracy and radius readings out of the shape parameter. this is still not supported within encoded shapes but can be added in the future. its useful for the tests, which i need to add more to convince myself that we really are excluding impossibly fast paths. although...

the other change i had to make was for the traffic segment matcher tests. since it cares about times by default adding the max_route_time_factor meant the absurdly fast times i was putting into those tests broke them. i had to multiply the numbers by 100 to get those tests to pass with the max_route_time_factor set to 5

…omputing the transmission cost

…shouldnt involve time

…ameter

…shapes when fail occurs

kevinkreiser · 2017-07-20T18:04:05Z

src/meili/routing.cc

@@ -38,7 +38,8 @@ bool LabelSet::put(const baldr::GraphId& nodeid, const baldr::GraphId& edgeid,
  // Create a new label and push it to the queue
  if (it == node_status_.end()) {
    const uint32_t idx = labels_.size();
-    if (sortcost < max_cost_) {
+    // We only add the labels if we are under the limits for distance and for time or time limit is 0
+    if (cost.cost < max_dist_ && (max_time_ == 0.f || cost.secs < max_time_)) {


this is the important part, we dont let the path expand if the distance or time is unreasonable

kevinkreiser · 2017-07-20T18:04:12Z

src/meili/routing.cc

@@ -84,7 +85,8 @@ bool LabelSet::put(uint16_t dest,
  // Create a new label and push it to the queue
  if (it == dest_status_.end()) {
    const uint32_t idx = labels_.size();
-    if (sortcost < max_cost_) {
+    // We only add the labels if we are under the limits for distance and for time or time limit is 0
+    if (cost.cost < max_dist_ && (max_time_ == 0.f || cost.secs < max_time_)) {


…o routing rather than labelset

dnesbitt61 · 2017-07-20T21:07:39Z

src/meili/routing.cc

@@ -572,18 +574,21 @@ find_shortest_path(baldr::GraphReader& reader,
          if (endtile == nullptr) {
            continue;


consider putting endtile (and check for null) inside conditional for adding label. The vast majority case is that the label will not be added due to exceeding cost or time threshold, and very small chance of endtile being null (and saving computation of cost).

ptpt · 2017-07-21T12:12:54Z

Why moving the max cost check out from LabelSet::put? It might be simpler if you initialize LabelSet with the max cost max_time_ == 0.f ? max_dist_ : max_time_ in https://github.com/valhalla/valhalla/blob/master/src/meili/map_matching.cc#L49.

kevinkreiser · 2017-07-21T12:16:19Z

@ptpt indeed, i had it that way and it was simpler: daf4687#diff-ce82e081c326c4ceefc3894638159018L54

but @dnesbitt61 said that wrt the future:

wondering if the decision whether to add a label should be in routing rather than pushed down to Labelset...just thinking that maybe LabelSet should be a low-level library for storing labels and updating the priority_queue and that the routing class should be where the decision to add a label is made? Not sure if we could/would move LabelSet to be a shared common library between meili and thor, but embedding meili specific logic may make that more difficult.`

ptpt · 2017-07-21T16:59:40Z

I see. Make sense to keep it outside. Having this logic in LabelSet is also fine if we think of it as a queue with capacity.

ptpt

Some comments.

ptpt · 2017-07-21T17:04:25Z

src/meili/map_matching.cc

@@ -46,11 +51,12 @@ State::route(const std::vector<const State*>& states,
  }

  // Route
-  labelset_ = std::make_shared<LabelSet>(std::ceil(max_route_distance));
+  labelset_ = std::make_shared<LabelSet>(max_route_distance);


Here should be passing max_route_time as well if it is available.

i did that in a previous commit, but then i reverted it since i moved the check of the max time out of the labelset. how would i make use of it if i pass in max_route_distance?

Sorry false alarm. It should be max_route_distance, because the sort cost in the queue is always distance.

ptpt · 2017-07-21T17:07:49Z

src/meili/routing.cc

@@ -15,12 +15,11 @@ namespace valhalla {

 namespace meili {

-LabelSet::LabelSet(const float max_cost, const float bucket_size) {
+LabelSet::LabelSet(const float max_dist, const float bucket_size) {


Here LabelSet only cares about cost regardless it is distance or time, max_cost would be a more proper name than max_dist.

i only changed it because on one of the commits i was still passing in time and it seemed odd to name one cost and one time. so i named them both what they were, dist and time. now that its just dist again i can rename it back to cost.

ok i did that!

ptpt · 2017-07-21T17:10:29Z

src/meili/routing.cc

-            &destinations, &search_radius, &search_rad2, &costing, &travelmode, &label_cost,
-            &label_turn_cost, &turn_cost_table, &heuristic, &expand]
-           (const baldr::GraphId& node, const uint32_t label_idx, const bool from_transition) {
+  expand = [&](const baldr::GraphId& node, const uint32_t label_idx, const bool from_transition) {


…e the match

ptpt · 2017-07-21T18:20:57Z

🚢

dnesbitt61 · 2017-07-24T12:49:33Z

kevinkreiser added 7 commits July 19, 2017 08:27

add time travel factor

ab13426

prepare tests for changes to matching using the time factor

bbebf37

make use of the time and the time factor by constraining routes and c…

41f7215

…omputing the transmission cost

Merge remote-tracking branch 'origin/master' into kk_timetravel

6cd95aa

revert back to the papers original transition cost calculation as it …

26927f5

…shouldnt involve time

allow trace_* actions to parse time and accuracy out of the shape par…

8a8d023

…ameter

make use of accuracy and time in the tests and show geojson of all 3 …

283ddbf

…shapes when fail occurs

kevinkreiser requested review from ptpt and dnesbitt61 July 20, 2017 17:57

kevinkreiser commented Jul 20, 2017

View reviewed changes

kevinkreiser added 3 commits July 20, 2017 14:05

allow changes to max_route_distance/time_factor on the fly

2fd09e4

move decision about whether or not to add a label based on limits int…

daf4687

…o routing rather than labelset

Merge remote-tracking branch 'origin/kk_timetravel' into kk_timetravel

2d26367

dnesbitt61 reviewed Jul 20, 2017

View reviewed changes

kevinkreiser added 2 commits July 21, 2017 09:05

show the shape when edge walk fails

8750567

move and simplify checks in meili routing

15641ad

ptpt reviewed Jul 21, 2017

View reviewed changes

kevinkreiser added 3 commits July 21, 2017 13:28

add two tests that show how the presence and absence of time influenc…

3ef5ada

…e the match

revert name change

45eaa46

resolve conflict

a5794bf

fix conflict

c86ac00

kevinkreiser merged commit f574145 into master Jul 24, 2017

kevinkreiser mentioned this pull request Jul 25, 2017

analysing samples with an without timestamp rejection opentraffic/reporter-quality-testing-rig#9

Closed

kevinkreiser deleted the kk_timetravel branch July 25, 2017 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Timestamp Information to Limit the Route Between States in the HMM #840

Use Timestamp Information to Limit the Route Between States in the HMM #840

kevinkreiser commented Jul 20, 2017

kevinkreiser Jul 20, 2017

kevinkreiser Jul 20, 2017

dnesbitt61 Jul 20, 2017

ptpt commented Jul 21, 2017 •

edited

kevinkreiser commented Jul 21, 2017

ptpt commented Jul 21, 2017

ptpt left a comment

ptpt Jul 21, 2017

kevinkreiser Jul 21, 2017

ptpt Jul 21, 2017

ptpt Jul 21, 2017

kevinkreiser Jul 21, 2017

kevinkreiser Jul 21, 2017

ptpt Jul 21, 2017

ptpt commented Jul 21, 2017

dnesbitt61 commented Jul 24, 2017

		@@ -572,18 +574,21 @@ find_shortest_path(baldr::GraphReader& reader,
		if (endtile == nullptr) {
		continue;

Use Timestamp Information to Limit the Route Between States in the HMM #840

Use Timestamp Information to Limit the Route Between States in the HMM #840

Conversation

kevinkreiser commented Jul 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptpt commented Jul 21, 2017 • edited

kevinkreiser commented Jul 21, 2017

ptpt commented Jul 21, 2017

ptpt left a 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

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

ptpt commented Jul 21, 2017

dnesbitt61 commented Jul 24, 2017

ptpt commented Jul 21, 2017 •

edited