Support snapping to multiple ways at an input location

[mjjbell]

Issue

link

This PR improves routing results by adding support for snapping to multiple ways at input locations.

Context

Input locations are snapped to positions on ways, which act as the source, target or via waypoint positions on the route.

Currently, OSRM only snaps an input location to one way. In certain scenarios, there are multiple ways to choose from:

• Intersections
• Connection points between two edges of a way
• Overlapping roads (e.g. bridges, tunnels)
• Edges that are part of via-ways in turn restrictions

The choice of selected way can impact the discovered route. Some good examples of the effect this has are discussed in #4465

This PR makes the change to use all ways at the snapped location as source/target candidates for routing search, ensuring we always find the best route, and not the one dependent on the way selected.

Design Choice

The decision to support multiple candidates for a snapped location was chosen for a few reasons:

• It doesn’t require special case logic for particular intersections
• It can be extended to support snapping to multiple equidistant locations from the input
• It acts as a first step towards supporting snapping to the k-nearest locations

The downside of this approach is that using multiple source/target candidates increases the routing search space. However, in practice this has minimal effect on performance, given:

• Most locations will still only snap to one way
• Even with multiple candidates for an input, most candidates will be connected by a valid turn, so many paths will be quickly pruned.

Implementation

PhantomNodeCandidates

We extend the PhantomNode representation for a snapped location to PhantomNodeCandidates, a vector of snapped locations.

Algorithms that snap large numbers of locations, such as table requests, will generate many small vectors. The implementation reuses the vectors fetched from the geospatial query result throughout the whole request. The performance hit appears to be minimal.

Snapping to connection between two edges of a way segment

All the routing algorithms assume a way segment only appears once in the search.
When snapping to the connection between two edges of a segment, we have to decide which edge will represent the segment in the search.

Segment edges are compressed and include forward and reverse directions. To decide which compressed edge to choose, we consider which is most likely to be useful as a source/target for a route. This can depend on edge traffic updates, approach parameters, etc.)

The implementation makes the choice between two compressed edges of a way segment in a consistent way. In very rare situations, it’s possible that some potential routes will not be found by having to select one over the other.

Nearest vs Big SCC

The existing snapping logic considers whether the snapped way is part of a tiny strongly connected component with other snapped locations, and fallbacks to the nearest candidate from the big SCC if this is not the case. This PR keeps and extends this logic for snapping to multiple candidates.

Geospatial Queries

The geospatial API for finding graph segments nearest to the location had many similar functions which represented permutations of parameter combinations. Given the logic for filtering is now more complex, I’ve consolidated the API functions and moved the parameter selection into one place.

Routing Algorithms

trip, table, and route now use the multiple candidate snapping. I did not include match, as my depth of understanding of the map matching algorithm is insufficient to figure out how “nearest candidates at the same location” would be represented in the output. Maybe a good follow-up task.

Shortest Path

Internally, the shortest path search function was passed in a list of endpoint candidates for each leg of the route. I have changed this to be a list of waypoint candidates instead, which makes it clear that the target candidates of the previous leg are the source candidates of the next leg. Now that candidates are vectors of PhantomNodes, it also avoids making a copy of each vector.

Not surprisingly, the dynamic programming approach for the shortest path search is made more complicated with multiple source/target nodes. I’ve tried as much as possible to avoid unnecessary vector allocations when tracking state between the legs.

Libosrm / HTTP API changes

Returning waypoint information in API results is now trickier as we have multiple PhantomNodes representing each waypoint.

To avoid a breaking change, the solution I went with is to:

• Use the location of the first candidate (this is fine as all candidates at a waypoint have the same location)
• Combine the names of the roads of each of the waypoint candidates
• Concatenate the hint strings (so hint size in the result is now len(candidates)*HINT_SIZE. I'm assuming changing the hint size is not an API break?

For hints in particular, this allows the hint string to contain all the information for multiple candidates. If we only returned the hint for the PhantomNode selected for the particular API call, it could lead to suboptimal results as a hint for future requests.

EDIT: This is a breaking change in the request parameters. hints are now vector<vector<hint>>

Tests

The changes to existing test values should be self explanatory. A couple of them have weird properties that might need cleaning up:

• One relies on negative turn penalty duration (which creates a negative step duration in the result)
• One includes an only_* restriction into a one-way street going the other way

In both cases, they have been changed to the point that they still succeed as before, although they might need revisiting to question the assumptions being made.

Whilst creating new tests, I also noticed some weird behaviours when combining zero speed traffic updates with a source and target on the same segment. It’s quite a niche scenario though, so I omitted tests for this in the PR and will create an issue with details if anyone is interested.

Performance

I’ll provide a full analysis for the effect on query performance in another comment. The initial findings show a small performance hit, at the expense of finding shorter routes, especially when many waypoints snap to intersections.

Tasklist

• CHANGELOG.md entry (How to write a changelog entry)
• update relevant Wiki pages
• add tests (see testing documentation
• review
• adjust for comments
• cherry pick to release branch

[danpat]

This will fix #4465, and possibly a bunch of tickets linked to that one.

@mjjbell I think you don't need to consider the /match service here - it already returns "all edges within radius N" and adds them to the Hidden Markov Model to find the best path.

Amazing contribution, thank you, this has been an issue for a very long time.

[mjjbell]

Performance Analysis

Preparation

OSRM

I used germany-latest.osm.pbf as input, osrm-extract with the default car profile and osrm-contract to route using the CH algorithm (I plan to do an MLD analysis later).

Test Coordinates

I created a collection of test coordinates for use as request parameters in the performance tests.
Using the same germany-latest.osm.pbf, all nodes are processed with libosmium to ensure we get the same coordinate truncation as used by OSRM data files - this enables us to test out the snapping behaviour at intersections. Nodes are bucketed by vertex degree.

We then randomly generate waypoint lists (represented by the truncated coordinates of the selected nodes). For testing, we create five sets of waypoint lists to test performance against a variety of input sizes.
Given we are interested in how the snapping behaviour changes the results when we have multiple ways to chose from, most of the waypoint lists are filtered to include nodes with degree >= 3.

Set	Number of waypoint lists	Number nodes in each waypoint list	Node properties
1	20408	2
2	19291	2	All nodes have degree >= 3
3	17387	11	All nodes have degree >= 3
4	7557	101	All nodes have degree >= 3
5	762	1001	All nodes have degree >= 3

The coordinate sets are available in this gist

Test Requests

The test coordinates are used to run the following requests via libosrm:

• Route
• Route with continue_straight=false
• Table
• Trip

In the case of Route and Trip requests, the waypoint lists act as waypoints in the returned route.
For Table requests, a waypoint list acts as both source and target locations, creating an NxN table result.

Tests are performed using libosrm versions of Project-OSRM:master at commit 9d81eb3 and mjjbell:mbell/snap_candidates

The request duration and route weight/duration response values are recorded and used to compare the performance of both branches.
N.B. for the purposes of comparing weights, any absolute difference of <= 0.2 is treated as the same weight - this is to accommodate for insignificant movements in the snapped locations between the branches.

Machine specs

Lenovo Thinkpad X1
intel i7 - 6 Cores, 12MB Cache, 64GB RAM
500 GB Local SSD
Ubuntu 20.04 LTS

Libraries

gcc 9
boost 1.71
lua 5.2
tbb v2020.1

[mjjbell]

Results - Route

Coordinate set 1

Request Duration

Percentile	master (μs)	snap_candidates(μs)	diff(μs)	% diff
0	189	201	12	6.35%
10	1372	1389	17	1.24%
20	1714	1729	15	0.88%
30	1978	1995	17	0.86%
40	2228	2244	16	0.72%
50	2460	2481	21	0.85%
60	2682	2702	20	0.75%
70	2906	2929	23	0.79%
80	3164	3183	19	0.60%
90	3553	3547	-6	-0.17%
99	4575	4590	15	0.33%
100	13183	13434	251	1.90%

Smallest Route Weight

master	same	snap_candidates
0	20339	69

Coordinate set 2

Request Duration

Percentile	master (μs)	snap_candidates(μs)	diff(μs)	% diff
0	218	229	11	5.05%
10	1291	1313	22	1.70%
20	1606	1628	22	1.37%
30	1852	1879	27	1.46%
40	2095	2118	23	1.10%
50	2315	2334	19	0.82%
60	2529	2549	20	0.79%
70	2747	2772	25	0.91%
80	3001	3019	18	0.60%
90	3378	3362	-16	-0.47%
99	4371	4302	-69	-1.58%
100	16385	16270	-115	-0.70%

Smallest Route Weight

master	same	snap_candidates
0	18985	234

Coordinate set 3

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	1187	1251	64	5.39%
10	20472	20762	290	1.42%
20	21744	21951	207	0.95%
30	22684	22861	177	0.78%
40	23538	23694	156	0.66%
50	24318	24477	159	0.65%
60	25114	25316	202	0.80%
70	26017	26206	189	0.73%
80	27132	27352	220	0.81%
90	28843	28916	73	0.25%
99	69394	65226	-4168	-6.01%
100	82659	81083	-1576	-1.91%

Smallest Route Weight

master	same	snap_candidates
0	16381	1006

Coordinate set 4

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	5977	5838	-139	-2.33%
10	218193	221424	3231	1.48%
20	222619	225899	3280	1.47%
30	225791	228930	3139	1.39%
40	228472	231357	2885	1.26%
50	231110	233607	2497	1.08%
60	233850	236019	2169	0.93%
70	237234	238720	1486	0.63%
80	242820	241960	-860	-0.35%
90	266953	246876	-20077	-7.52%
99	287465	282789	-4676	-1.63%
100	348364	361102	12738	3.66%

Smallest Route Weight

master	same	snap_candidates
0	4562	2995

Coordinate set 5

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	66182	65036	-1146	-1.73%
10	676659	679042	2383	0.35%
20	2250095	2271579	21484	0.95%
30	2268765	2287675	18910	0.83%
40	2280586	2299932	19346	0.85%
50	2290470	2310654	20184	0.88%
60	2304508	2324956	20448	0.89%
70	2320816	2343423	22607	0.97%
80	2347262	2370331	23069	0.98%
90	2375039	2405981	30942	1.30%
99	2452844	2463975	11131	0.45%
100	2623389	2523901	-99488	-3.79%

Smallest Route Weight

master	same	snap_candidates
0	86	676

[mjjbell]

Results - Route continue_straight=false

We start the results from coordinate set 3, as continue_straight=false will have no effect on waypoint lists of length two.

Coordinate set 3

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	2223	2299	76	3.42%
10	18161	18016	-145	-0.80%
20	19383	19090	-293	-1.51%
30	20340	19963	-377	-1.85%
40	21160	20749	-411	-1.94%
50	21929	21511	-418	-1.91%
60	22739	22281	-458	-2.01%
70	23601	23127	-474	-2.01%
80	24658	24142	-516	-2.09%
90	26242	25603	-639	-2.44%
99	65394	29350	-36044	-55.12%
100	78483	78273	-210	-0.27%

Smallest Route Weight

master	same	snap_candidates
0	15990	1397

Coordinate set 4

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	7559	7843	284	3.76%
10	190380	193361	2981	1.57%
20	194161	197038	2877	1.48%
30	196893	199942	3049	1.55%
40	199394	202247	2853	1.43%
50	201640	204357	2717	1.35%
60	203800	206534	2734	1.34%
70	206402	208995	2593	1.26%
80	209833	211817	1984	0.95%
90	215237	216509	1272	0.59%
99	250923	241023	-9900	-3.95%
100	323250	326410	3160	0.98%

Smallest Route Weight

master	same	snap_candidates
0	3465	4092

Coordinate set 5

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	74904	63922	-10982	-14.66%
10	1953409	1993940	40531	2.07%
20	1971011	2010467	39456	2.00%
30	1983935	2024351	40416	2.04%
40	1992464	2037172	44708	2.24%
50	2003905	2057112	53207	2.66%
60	2017461	2074207	56746	2.81%
70	2035575	2091534	55959	2.75%
80	2053559	2106061	52502	2.56%
90	2072882	2122799	49917	2.41%
99	2112569	2162705	50136	2.37%
100	2414390	2423875	9485	0.39%

Smallest Route Weight

master	same	snap_candidates
0	15	747

[mjjbell]

Results - Table

N.B. Here we are not able to compare route weights, as the table response does not contain them. Instead we compare route durations. In some cases we do get shorter duration results with master. Upon inspection of these results, snap_candidates results have smaller weight. See the following illustrative example:

master

snap_candidates

Coordinate set 1

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	459	384	-75	-16.34%
10	786	771	-15	-1.91%
20	839	827	-12	-1.43%
30	878	870	-8	-0.91%
40	913	906	-7	-0.77%
50	946	941	-5	-0.53%
60	980	978	-2	-0.20%
70	1018	1016	-2	-0.20%
80	1065	1063	-2	-0.19%
90	1135	1134	-1	-0.09%
99	1367	1324	-43	-3.15%
100	19759	14208	-5551	-28.09%

Smallest Route Duration

master	same	snap_candidates
3	81495	134

Coordinate set 2

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	379	384	5	1.32%
10	763	743	-20	-2.62%
20	818	800	-18	-2.20%
30	858	841	-17	-1.98%
40	892	878	-14	-1.57%
50	925	913	-12	-1.30%
60	960	949	-11	-1.15%
70	997	987	-10	-1.00%
80	1042	1035	-7	-0.67%
90	1109	1107	-2	-0.18%
99	1336	1354	18	1.35%
100	18832	18663	-169	-0.90%

Smallest Route Duration

master	same	snap_candidates
1	76415	460

Coordinate set 3

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	3868	3713	-155	-4.01%
10	4799	4694	-105	-2.19%
20	4939	4840	-99	-2.00%
30	5039	4951	-88	-1.75%
40	5132	5040	-92	-1.79%
50	5215	5125	-90	-1.73%
60	5299	5212	-87	-1.64%
70	5395	5306	-89	-1.65%
80	5505	5417	-88	-1.60%
90	5676	5577	-99	-1.74%
99	6410	6047	-363	-5.66%
100	55243	55735	492	0.89%

Smallest Route Duration

master	same	snap_candidates
81	2080480	23266

Coordinate set 4

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	45902	45955	53	0.12%
10	49471	48684	-787	-1.59%
20	49967	49205	-762	-1.53%
30	50355	49591	-764	-1.52%
40	50697	49903	-794	-1.57%
50	51029	50210	-819	-1.60%
60	51354	50536	-818	-1.59%
70	51677	50886	-791	-1.53%
80	52\068	51273	-795	-1.53%
90	52783	51873	-910	-1.72%
99	93967	53723	-40244	-42.83%
100	103247	101436	-1811	-1.75%

Smallest Route Duration

master	same	snap_candidates
3840	76183721	901396

Coordinate set 5

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	675348	607830	-67518	-10.00%
10	682147	677855	-4292	-0.63%
20	686247	681076	-5171	-0.75%
30	689809	683411	-6398	-0.93%
40	692327	685563	-6764	-0.98%
50	696535	688071	-8464	-1.22%
60	710840	691110	-19730	-2.78%
70	724902	695422	-29480	-4.07%
80	728627	722568	-6059	-0.83%
90	733552	729589	-3963	-0.54%
99	743412	744403	991	0.13%
100	886189	763301	-122888	-13.87%

Smallest Route Duration

master	same	snap_candidates
39196	754484539	9002060

[mjjbell]

Results - Trip

Running Trip on coordinate sets 1 and 2 is effectively finding the shortest route between two endpoints in either direction.
For coordinate sets 3,4 and 5 we begin to see some results having a significantly smaller weight when run on master. This is to be expected, given that these routes are just approximations of the optimal route, so it's possible that snap_candidates is finding worse approximations in some cases.

Coordinate set 1

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	846	776	-70	-8.27%
10	3353	3354	1	0.03%
20	4047	4025	-22	-0.54%
30	4581	4560	-21	-0.46%
40	5097	5063	-34	-0.67%
50	5566	5544	-22	-0.40%
60	6027	5985	-42	-0.70%
70	6495	6459	-36	-0.55%
80	7039	7021	-18	-0.26%
90	7828	7840	12	0.15%
99	9844	10575	731	7.43%
100	19114	59637	40523	212.01%

Smallest Route Weight

master	same	snap_candidates
0	20335	73

Coordinate set 2

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	980	1004	24	2.45%
10	3172	3204	32	1.01%
20	3818	3890	72	1.89%
30	4332	4444	112	2.59%
40	4824	5065	241	5.00%
50	5281	5607	326	6.17%
60	5718	6076	358	6.26%
70	6165	6567	402	6.52%
80	6703	7117	414	6.18%
90	7469	7867	398	5.33%
99	9486	10655	1169	12.32%
100	23285	60152	36867	158.33%

Smallest Route Weight

master	same	snap_candidates
0	18923	296

Coordinate set 3

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	4845	4389	-456	-9.41%
10	17714	18073	359	2.03%
20	18394	18762	368	2.00%
30	18916	19283	367	1.94%
40	19391	19749	358	1.85%
50	19840	20188	348	1.75%
60	20305	20619	314	1.55%
70	20818	21123	305	1.47%
80	21432	21709	277	1.29%
90	22418	22552	134	0.60%
99	50839	24958	-25881	-50.91%
100	72122	74177	2055	2.85%

Smallest Route Weight

master	same	snap_candidates
4	15913	1470

Coordinate set 4

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	50307	48584	-1723	-3.42%
10	110669	117229	6560	5.93%
20	111964	118632	6668	5.96%
30	113020	119616	6596	5.84%
40	113879	120459	6580	5.78%
50	114741	121297	6556	5.71%
60	115596	122210	6614	5.72%
70	116532	123186	6654	5.71%
80	117755	124439	6684	5.68%
90	119783	126681	6898	5.76%
99	162562	170316	7754	4.77%
100	223655	227874	4219	1.89%

Smallest Route Weight

master	same	snap_candidates
69	3370	4118

Coordinate set 5

Request Duration

Percentile	master (μs)	snap_candidates (μs)	diff (μs)	% diff
0	653879	625922	-27957	-4.28%
10	1474263	1580410	106147	7.20%
20	1480059	1587731	107672	7.27%
30	1483672	1593048	109376	7.37%
40	1487356	1596728	109372	7.35%
50	1491191	1602011	110820	7.43%
60	1496461	1609048	112587	7.52%
70	1506838	1621422	114584	7.60%
80	1520878	1643598	122720	8.07%
90	1558687	1673506	114819	7.37%
99	1576503	1699391	122888	7.79%
100	1753056	1799930	46874	2.67%

Smallest Route Weight

master	same	snap_candidates
77	15	670

[mjjbell]

Review

In summary, with this change we are able to find better results, especially when the inputs snap to locations with multiple ways. In the cases where we can compare optimal route weights, it does not produce any worse results.

The performance effect on request duration appears to differ on each service.

Route has a small performance penalty.

Table has no performance penalty.

Trip has a significant slowdown for large numbers of waypoints. This appears to be disproportionately large and more than the expected penalty of compounding the internal Route and table requests it performs. Further investigation is required.

[jcoupey]

Thanks @mjjbell for this comprehensive impact study!

For coordinate sets 3,4 and 5 we begin to see some results having a significantly smaller weight when run on master

Things are a bit fuzzy for the trip plugin and hard to evaluate based on weight only: my understanding is that the trip plugin does the TSP optimization based on durations, but the underlying durations matrix is computed on a weight-based objective (see #5930 for a discussion around this behavior).

Trip has a significant slowdown for large numbers of waypoints. This appears to be disproportionately large and more than the expected penalty of compounding the internal Route and table requests it performs.

The expected changes are probably rather on the solving side than on internal route and table overhead. As you mentioned, only approximation solution are provided, and master and snap_candidates are likely to follow quite different solving paths, leading to varying computing times. As a result the subset of the "X% fastest requests" may be significantly different between the two branches.

[mjjbell]

Things are a bit fuzzy for the trip plugin and hard to evaluate based on weight only: my understanding is that the trip plugin does the TSP optimization based on durations, but the underlying durations matrix is computed on a weight-based objective (see #5930 for a discussion around this behavior).

Ah yes, that actually does explain it. Thanks!

[mjjbell]

    Scenario: Middle Island Over Bridge
        Given the node map
            """
              a
              |
             .b.
            c   h
            |   |
            |   |
            1   2
            |   |
            d   g
             'e'
              |
              f
            """

        And the ways
            | nodes | name   | oneway |
            | ab    | road   | no     |
            | ef    | road   | no     |
            | bc    | road   | yes    |
            | cd    | bridge | yes    |
            | de    | road   | yes    |
            | eg    | road   | yes    |
            | gh    | bridge | yes    |
            | hb    | road   | yes    |

        When I route I should get
            | waypoints | turns         | route       | intersections						  |
            | a,f       | depart,arrive | road,road   | true:180,false:0 true:180,false:0 true:180;true:0 |
            | c,f       | depart,arrive | bridge,road | true:180,false:0 true:180;true:0		  |
            | 1,f       | depart,arrive | bridge,road | true:180,false:0 true:180;true:0                  |
            | f,a       | depart,arrive | road,road   | true:0,true:0 false:180,true:0 false:180;true:180 |
            | g,a       | depart,arrive | bridge,road | true:0,true:0 false:180;true:180                  |
            | 2,a       | depart,arrive | bridge,road | true:0,true:0 false:180;true:180                  |

When routing from c to f in this merging segregated roads test graph, the test would fail only for the contraction hierarchy algorithm on OS X, as it finds the named route road, road instead of bridge, road. I investigated this as it seemed a bit puzzling.

Segregated Road Merging

With this PR there are now two possible start candidates on a c to f route - bc and cd.
Linux/Ubuntu is selecting cd, and OS X is selecting bc.

Looking at the merging logic for combining segregated roads:

• The latter name of two connected edges is used. Hence, all the roads are merged and we're left with the name of ef as the second value in the route.
• The first value comes from first edge of the route, which is never merged, Therefore, for Linux cd is bridge, so we get bridge,road. For OS X, bc is road, so after merging we are left with road,road.

This explains why the merging logic creates different results given the selected start edge, but doesn’t explain why different start edges are selected on different platforms.

Contraction Order

I extracted the contraction hierarchy forward and backward graphs for both platforms.

Linux

OS X

Clearly, the ordering is different. My understanding of the contraction logic is that when there are no criteria that indicate one node has higher rank than the other, one of the nodes is (deterministically) chosen at random.

I discovered this when looking at an edge case from a more trivial example, which I will discuss in a separate issue, but I expect this difference in randomness between the platforms is what creates the different contraction order here too.

CH search chooses the first route it finds when there are multiple routes of equal weight.
If we follow the search path for finding the identical weight shortest routes bc -> cd -> de -> ef and cd -> de -> ef, we will see that for Linux, cd as a starting point is found first, whereas for OS X, we will find bc first.

Linux

OS X

So this explains the difference. To fix this, I explicitly added a numbered input location so that it only snaps to one edge. Other tests also required explicit inputs to continue to pass - it’s possible some of these will have displayed these platform differences too.

I expect this is one of many subtle differences that can occur from snapping to multiple candidates.

[mjjbell]

| restriction | cb       | bx     | b        | only_right_turn  |

This turn restriction routes you into a one-way road going in the other direction. I'm not sure if this was intended, but with the start location now snapping to both cb and dc, it was creating a different shortest route. I added a numbered input location to ensure it snaps only to dc as it did previously.

[mjjbell]

The use of negative turn penalties is the cause of the increase in speed/time here.

Previously, the start would snap to cf (which would be combined with cd in the route result annotation).
Now with multiple intersection candidates to choose from, the route does actually start from cd.

There is special case logic for preventing negative weights/durations on the first turn. It didn't apply previously when starting at cf as the negative duration was on the second turn, but it does now.
This leads to the shortest route for this PR actually having larger duration than previously.

I can see historical discussions about the value of negative turn penalty weights, but I'm not sure if that also applies for negative durations. In any case, I've just updated the expected value to reflect this turn clamping logic.

[mjjbell]

Also worth mentioning - this PR will enable a fix for snapping target locations to via ways used in turn restrictions, discussed here.

The fix is quite simple once this change is made: mjjbell@3771e99

[mjjbell]

(I plan to do an MLD analysis later)

I've completed the same analysis for the MLD algorithm. The relative performance change is very similar to the CH result detailed above, so to save some scrolling I've put the MLD results in this gist.

[SiarheiFedartsou]

Hi @mjjbell
Could you please summarize what is left to do to consider this PR as ready to merge?

[SiarheiFedartsou]

It's ready.
Just needs to be reviewed and rebased.

Cool, I am not very familiar with this part, but I will try to do a review in the nearest days.

Do you think we will need someone from OSRM maintainers to approve it too? I guess it can be difficult taking into account that you are the only one active recently :)

[mjjbell]

It's ready.
Just needs to be reviewed and rebased.

Cool, I am not very familiar with this part, but I will try to do a review in the nearest days.

Do you think we will need someone from OSRM maintainers to approve it too? I guess it can be difficult taking into account that you are the only one active recently :)

Yes it will need a maintainer to approve for it to be mergeable. Repo ownership still resides with (ex-) Mapbox folk, so I can't change the situation myself.
Related: #5209

[SiarheiFedartsou]

Hey @mjjbell

EDIT: This is a breaking change in the request parameters. hints are now vector<vector>

Do you mean breaking change from libosrm point of view? Can we may be leave BaseParameters::hints parameters as is and add new one(may be BaseParameters::extended_hints - not sure what is the best name here) and use new if it is non-empty and fallback to the old one in the case if not?

In general I had a chance to just briefly look through PR and it seems it is LGTM, but I think I will need to spend a bit more time on review, but just want to ask do you have time to rebase it? Or I can help you with it if you want? The same question is about other your PRs which seems to be open for a while:

[SiarheiFedartsou]

@mjjbell
It is a breaking change you mentioned, right? Is it possible to support both old and new version of API? I mean do not touch existing field and just add std::vector<std::vector<Hint>> extended_hints;? I would even make it std::vector<WaypointHints> extended_hints and put this vector of hints to WaypointHints structure to be able to extend this structure in the future if needed without breaking changes.

[mjjbell]

👍 I like this idea. This would make it backwards compatible with existing API users getting the existing (sub-optimal) behaviour, new users get the improved behaviour. Most people who use the HTTP API (such as osrm-frontend) will get the improvement for free.

[mjjbell]

Thanks, I'll rebase.
Yes it would it be a breaking change to libosrm, so I like the idea of having an extended hints field.

The previous plan included a proposal to remove hints (#4741) as it exposes internal data structures, so I think a lightweight backwards compatible solution makes sense for now, and we can reserve the breaking change for when we have a hints replacement.

[SiarheiFedartsou]

This bugprone-macro-parentheses looks too annoying tbh 🤔

[mjjbell]

So having tried to implement an additional field for hints in the request and response fields, I can see it complicates a lot of the code.

What I have realised however, is both the HTTP and libosrm responses contain the base64 encoding of hints.
For HTTP, the hint is passed back as a base64 request argument without any changes.
For libosrm, it needs to returned as a Hint argument. This is only possible by calling Hint::FromBase64.

Therefore, I've inverted the change slightly. Hintnow contains a vector of SegmentHints, that represents the multiple waypoints that can represent the input location.

From the libosrm user's perspective, nothing needs to change, they still call Hint::FromBase64 on the base64 response to get the Hint to pass into the next request. This will still work on old base64 values that the user may have stored from previous OSRM versions.

If the user happens to edit some of the internals of Hint or serializes the value separately from the base64 representation, then this would be a breaking change. However, this is the fundamental problem mentioned in #4741. Previous changes to PhantomCandidate have not been reflected in a breaking API change, so I'm satisfied that anyone who's tweaking/serializing internals will not be assuming it is stable.

In summary, I think this is good to go now 👍

[SiarheiFedartsou]

Not sure if it is not breaking change too - as I understand API of Hint class is part of public API too, BUT tbh I don’t think it is a big problem: even though someone uses it for something(I doubt it) it will be quite easy to migrate.

[SiarheiFedartsou]

Sorry didn’t notice comment #5953 (comment)

[ntzm]

Thanks everyone for your work on this, this has simplified something I'm doing significantly :)