Move complex turn restriction check out of can_form_shortcut()

[PhilippVoigt]

This is a workaround to make shortcut recovery reliable. Currently it fails in certain cases where complex turn restrictions flags are changed (especially removed) during graph validation. This makes shortcuts that were generated correctly and deterministically by shortcut builder, suddenly ambiguous to the shortcut recovery.

Like before, shortcuts are never built across restrictions, but with this change the shortcut builder and shortcut recovery are equally aware of the restricted edges.

[kevinkreiser]

@PhilippVoigt for some reason this is not very intuitive to me, would you mind doing a bit more of an explanation here?

to me it looks like the shared method that checks for the ability to use an edge in a shortcut now lacks a check for complex restrictions.

that would mean that the recovery method is unaware of it but also i see that its added back to the shortcut building step. without thinking too hard about it it seems to me that would make the two code paths disagree.

what am i missing here?

[PhilippVoigt]

@kevinkreiser I can at least try to describe my intuition behind this.

Since I did not fully understand the graph validation logic as to why certain complex restrictions that were once added to the edges are removed afterwards, all I could assume was the following:

It is not guaranteed that an edge which belonged to a complex restriction during shortcut creation is still part of that restriction in the final graph that is traversed during shortcut recovery. Since that condition is not stable throughout the graph generation process, we cannot consider it as one of the global requirements for an edge to be part of a shortcut.

However, we still want to avoid shortcuts beginning or ending in the middle of a restriction, which the proposed solution ensures. Now during recovery I see this happening: The traversal will eventually reach the node at which the edge with a complex restriction starts or ends. We are not checking the condition, so whether the restriction is still there or has been rewritten during validation doesn't matter. Instead, I see two scenarios happening here:

1. The traversal will find more than 2 eligible edges connected to that node and bail. One of those eligible edges may or may not have been a complex restriction before or may still be.
2. The traversal finds only 2 eligible edges at that node. If one of these was or still is a complex restriction, we may find a shortcut at that node and our newly added stop criteria will tell us if it's the one we are looking for. If there is no shortcut the traversal will continue unnecessarily until it finds an unrelated shortcut and bails.

Ultimately I placed the complex restriction check where I did because this is where I found other conditions related to roundabouts which are also not part of the shared can_form_shortcutmethod. All that these conditions do, is adding less shortcuts which the recovery should now be able to cope with. Intuitively, this makes sense to me, but of course I may have overlooked something crucial. Looking forward to your feedback :)

[PhilippVoigt]

Hi @kevinkreiser, did you get a chance to think about this again? I would be happy to hear your thoughts on my assumptions 😊

[nilsnolde]

Hi @PhilippVoigt maybe this kinda thing would be better discussed in person in a session? We meet twice a week to review or even pair-code. Would you have time tmrw from 2:30 - 3:30 pm Berlin time? If so, you can drop me an email on nils@gis-ops.com and I can share the link with you.

[kevinkreiser]

some how the gtfs tests are failing now? im not sure how they are releated!

[nilsnolde]

Hm, it's the one taking the current time and it's failing with Date and time is invalid. Format is YYYY-MM-DDTHH:MM. Some fuckup in how we convert the time I bet. Hm.. Yeah off the top of my head, I'd guess it's taking day 0, since today is 27th haha but not sure, will look. Otherwise we'll have to avoid CI runs on 27th;) Well, no, it should be 28th.. Anyways, will fix later while it's still today;)

[nilsnolde]

I fixed it in #4089