Respect interpolated matches when building matched route #3646
Conversation
| int prev_idx = first_idx; | ||
| for (int curr_idx = first_idx + 1; curr_idx <= last_idx; ++curr_idx) { | ||
| const MatchResult& curr_match = match_results[curr_idx]; | ||
| const MatchResult& prev_match = match_results[prev_idx]; | ||
|
|
||
| // skip if we do not need to cut | ||
| if (!curr_match.is_break_point && curr_idx != last_idx) { |
There was a problem hiding this comment.
so before this was just looping through non-interpolated points and only copied the segments when we were at the last segment of the passed range. this would skip updating segments whose first or last matched point was interpolated. now there'll be a whole lot more allocations to the route vector below, only to keep the indices of the matched points correctly on the segments. in #3645 I tried to get around this by only adding segments to the route once we have a segment transition which was much less work for the algo. maybe I'll revisit that. what do you think?
There was a problem hiding this comment.
I "benchmarked" master vs this branch on a 200 point trace_attributes request, 10000 loops per run. in sweden, somewhere remote-ish, and dense gps sampling (i.e. routing takes relatively less time, so I'd imagine a bigger impact of this PRs changes):
0.1296 secs (master) vs 0.1304 secs (this branch)
I really tried hard to make the other approach work which would've reduced the vector allocations tremendously, probably also less than master, but I just couldn't tweak the algo properly. so many cases, so much headache.
| for (int cut_begin_idx = prev_idx, cut_end_idx = prev_idx + 1; cut_end_idx <= curr_idx; | ||
| ++cut_end_idx) { |
There was a problem hiding this comment.
loop through all points in between the two stateful points (and skip unmatched ones)
There was a problem hiding this comment.
that's basically it here, didn't change anything else
| )"; | ||
|
|
||
| const gurka::ways ways = {{"AB", {{"highway", "primary"}}}, | ||
| {"BC", {{"highway", "primary"}}}, | ||
| {"CD", {{"highway", "primary"}}}}; | ||
|
|
||
| const double gridsize = 10; | ||
| const double gridsize = 9; |
There was a problem hiding this comment.
9 meters ensures that node 5 will be set "interpolated" (the default interpolation_distance is 10)
| DirectionsLeg::Maneuver::kRight, | ||
| DirectionsLeg::Maneuver::kDestination}); | ||
|
|
||
| gurka::assert::raw::expect_path_length(result_route, 0.099, 0.001); |
There was a problem hiding this comment.
the path is identical but the grid size is now 9 meters instead of 10, however the length is only 1 meter different? that is confusing, i would think it would be 12 meters different (1 meter for every space in the ascii)
There was a problem hiding this comment.
there seem to be 11 chars in total in the path so it should be 11*9==99 so that seems right but, why wasnt it 11*10 ==110 before??
There was a problem hiding this comment.
If you look at the map again, I added one more node, so one more grid cell, easy to miss really
|
@nilsnolde i made a branch of your branch and tried a different approach in which i refactored cut segments to remember the last position it searched from within the two match points. this way there is no more iteration than needed. i moved the check for continuing down after that search and then inside the if with the continue im going to try to patch the segment that touches that match point. i think this should fix the issue without extra looping or allocations. just need another free night to do the actual patching inside that if. i'll push it up when i get a chance so you can take a look. |
|
for others following along i pushed up my slight refactor to this branch: https://github.com/valhalla/valhalla/tree/kk_interpolated_match_index im hopeful that if we fill out the |
|
@kevinkreiser did it more properly over in #3670 , closing this one |
fixes #3371
for more context see #3645 where I tried to change too much and it was very hard to keep the same results. now I just introduced one more nested loop more or less which fixes the problem and passes the tests