Update node weights if traffic data is applied #3430
Conversation
| @@ -914,6 +916,8 @@ EdgeID Contractor::LoadEdgeExpandedGraph( | |||
| previous_osm_node_id = segmentblocks[i].this_osm_node_id; | |||
| } | |||
|
|
|||
| node_weights[inbuffer.source] = new_weight; | |||
There was a problem hiding this comment.
This is the only bit that's actually new here.
|
@danpat I have serious doubts if the fix you did is actually the cause of the problem, but I cannot fully understand whats going on, yet. The code using the node weights is required to find self-loops. So starting on a segment, ending on a segment and driving a loop in between (e.g. bearings front/back in the wrong direction) or if you consider a longer slow road with a fast route going around. The fix you did might, by chance, actually prevent the issue from surfacing, but the only difference we should see on traffic stacks is whether or not we find a traffic loop. As long as a direct path exists, the (which should be a given in map matching), the effect should not be visible at all. |
|
Aside from potential other problems. This PR can be merged still to update the node weights to be more accurate. |
|
@danpat I think it is okay to skip on the test-case here as in it's current form it is mostly a performance fix for the loop pruning heuristic in the CH code. |
|
Technically it's also a question of loop correctness (which is broken to begin with, some other issues here). So I'd be ok with skipping the test as well. (see #2016) |
TheMarex
force-pushed
the
fix/node_weight_traffic_update
branch
from
7989b92 to
fef6ab3
Compare
|
Rebased and squashed, waiting on travis then merging. |
|
Damn forgot to merge this before doing the next RC. |
Issue
This is an attempt at fixing #3429. Our extractor saves the edge-based-node weights into the
.enwfile. However, if traffic data is applied, those node weights are invalidated, yet we still use them during graph contraction.This PR moves the location that the
.enwfile is loaded and updates node weights with new values if traffic data is supplied.With this change, can no longer reproduce the problem described in #3429
@MoKob @TheMarex can you sanity check this please?
TODO: an isolated test case - I have so far only reproduced this problem with large sets of internal data, I haven't narrowed it down to something I document as a cucumber test.
Tasklist