Don't allow A* loops for bicycles

[leonardehrenfried]

Summary

Don't allow loops in A* computations for bikes. They can lead to timeouts when the start/destination is near with a high number of visibility edges.

Issue

closes #3564

Unit tests

none

Code style

Yes

Documentation

Yes

Changelog

Yes

[leonardehrenfried]

I'm having a bit of a problem with the BikeRentalSnapshotTest. It seems that when I remove the loop for cyclists, then arriveBy and departAt searches return different results.

Both are, IMO, valid routes.

It seems that the looping code has masked a problem where the arriveBy and departAt intersection traversal code returns different results.

When I use a intersection cost model that returns a constant cost, then the difference disappears.

[leonardehrenfried]

Actually, I managed to sidestep the failing test by moving the start location ever so slightly to the east.

[leonardehrenfried]

As discussed in the dev meeting, I separated the arriveBy and departAt tests thereby documenting the unexpected difference.

[t2gran]

I looked through the code and it looks ok, but @gmellemstrand will do the final review from Entur.

[miklcct]

I'm having a bit of a problem with the BikeRentalSnapshotTest. It seems that when I remove the loop for cyclists, then arriveBy and departAt searches return different results.

Both are, IMO, valid routes.

It seems that the looping code has masked a problem where the arriveBy and departAt intersection traversal code returns different results.

When I use a intersection cost model that returns a constant cost, then the difference disappears.

The problem is not where the arriveBy and departAt intersection traversal code returns different results. It is A* not keeping the state of different back edge leading to different turn costs, so sometimes a non-optimal result is returned. A test case is provided in #6777 .

[abyrd]

Good catch @miklcct, thanks for tracking down the source of the asymmetric routing behavior in this older commit. The decision to not allow loops on bicycles may have been a necessary compromise at the time to maintain performance. But the underlying reason why certain bicycle states were incomparable (allowing looping paths) was to ensure correct handling of turn costs. Making all bike states comparable is only half of the necessary change - the other half would be deactivating all turn restrictions for bicycles.

The state comparability logic here looks incomplete to me but maybe I'm missing something. The comments (even in current versions of the code four years later) are all about loops, and allowing loops "near the start and end" - there may be some misinterpretation of what this logic is for.

[miklcct]

However bicycles are subject to turn restrictions and I won't be happy if the router tells me a route to turn right where prohibited, forcing me to push my bike against turning traffic.