-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for multiple via-way restrictions #5907
Conversation
Given the extract extra arguments "--parse-conditional-restrictions" | ||
# time stamp for 10am on Tues, 02 May 2017 GMT | ||
Given the contract extra arguments "--time-zone-file=test/data/tz/{timezone_names}/guinea.geojson --parse-conditionals-from-now=1493719200" | ||
Given the customize extra arguments "--time-zone-file=test/data/tz/{timezone_names}/guinea.geojson --parse-conditionals-from-now=1493719200" | ||
Given the node map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of these tests were duplicated by 879de34. This is just removing the duplication.
I don't claim to understand every detail but I would like to applaud the impressively detailed documentation & that you've kept an eye on resource usage. Well done! |
Wow, this is pretty huge! I was always struggling with this in most foss routers. E.g. TomTom data is FULL of multi-via-way turn restrictions. Very sweet, thanks so much! |
Impressive job! I will thoroughly review it on a weekend :) In the meanwhile - all builds are failing with boost/container_hash/hash.hpp: No such file or directory, could you please take a look on that? |
Currently OSRM only supports turn restrictions with a single via-node or one via-way. OSM allows for multiple via-ways to represent longer and more complex restrictions. This PR extends the use of duplicate nodes for representng via-way turn restrictions to also support multi via-way restrictions. Effectively, this increases the edge-based graph size by the number of edges in multi via-way restrictions. However, given the low number of these restrictions it has little effect on total graph size. In addition, we add a new step in the extraction phase that constructs a restriction graph to support more complex relationships between restrictions, such as nested restrictions and overlapping restrictions.
c430c98
to
a50c28c
Compare
Here's a breakdown of The PR has minimal effect on runtime and memory usage - within the normal variation between runs. In terms of the graph output, all the regions show a minor increase in the number of small strongly connected components. This is to be expected - the multi-via implementation is creating more nodes that have reduced (and potentially no) outgoing edges. I did not analyse the performance of subsequent CH or MLD steps, as there is no significant change in the extracted graph size or structure to warrant it. |
As change is big, let's wait for more opinions. @mjjbell Thank you for fixing restrictions (and conditional restrictions!) |
I think my only concern is unexpected side effects for routes that start/end within the new expanded graphs. I see the tests where you check routes that depart/arrive within the restriction graph - do you feel that these tests encompass all the possible behaviours? How does a coordinate snapped to an EdgeBasedNode within the restriction graph decide which part of the new restriction graph to select? |
Coordinates can't be snapped to duplicate nodes in the EBG. As the restriction graph nodes are added as duplicates, it shouldn't be possible. My understanding is that the RTree will use the edge segments inserted here by the So this gives the desired behaviour that you can't be snapped into a restriction path. I'm reasonably confident that the tests cover this, and that we would have seen issues with the single-via paths if it was happening. However, I can add a test that focusses specifically on this behaviour to avoid future regressions. |
Thinking this through now - I think there is an edge case when the target is snapped to a node that has a duplicate from the restriction graph. Let me see I can reproduce in the tests. |
This is the minimal test case. Snapping the target onto a via path fails.
In which case, we probably do need to include duplicate EBG nodes in the RTree. That brings up your question of which of the original/duplicates to snap to. I think the answer would be, all of them. They each have a path from the target that the others do not. That leads me to believe this can't be fixed until there is routing support for multiple phantom candidates at endpoints. |
Right, I've been linking to #4465 as kind of the gathering point for issues related to only supporting a single start location. Given how relatively uncommon these types of restrictions are, and the fairly low chance that folks will be requesting start/endpoints within them, we can just live with the edge case. 👍 Thanks for this contribution, it's awesome :-) |
Thanks! I was just asked by a mapper how to map this and he was a bit sad that OSRM and Graphhopper deployed at OSM website failed to work properly (so what is the point of mapping this? etc). He was much happier when I found this PR and was able to mention that it will change :) Thanks again for implementing this! |
@zhysong Version deployed there is likely using older version of OSRM software. Older than v5.24.0 ( https://github.com/Project-OSRM/osrm-backend/releases ) |
@zhysong yes, this is the edge-case mentioned in the earlier comment. Routes with destinations on via ways will be suboptimal or sometimes not found. A combination of #5953 and mjjbell@3771e99 will fix this. |
@mjjbell I merged your code (#5953 and mjjbell/osrm-backend@3771e99) to retest, it can fix detour issue (destination is on the via way of multiple via-ways complex restriction). |
Issue
link
OSRM currently supports restrictions for the following types of turns:
OSM allows for turn restrictions with multiple via-ways to represent longer and more complex restrictions. This PR adds support for multi via-way turn restrictions.
Impact
OSM data has approximately 8500 multi via-way turn restrictions. These turns will now be restricted when route planning.
Context
In the edge-based graph, a via-node restriction is applied by not creating an edge that represents the turn.
For single via-way restrictions, an edge-based node is created that duplicates the via-way node.
The from-via turn is replaced with a turn onto the duplicated node. The restriction instructions inform which
edges to create from the duplicate node back to the routing graph.
See #4255 and #2681 for more details about the current implementation.
Supporting multi via-way restrictions
This PR extends the use of duplicate nodes to support multi via-way restrictions.
In addition, with multi-via restrictions comes the possibility of nested and overlapping restrictions.
For this, we construct a restriction graph to correctly represent these relationships when generating the edge-based graph.
Restriction graph
The restriction graph is used to represent all possible turn restrictions within the routing graph.
Each node is equivalent to an edge-based graph node, along with the restriction path prefix that was taken to reach this node.
For example, the following three nodes duplicate the same edge-based node cd, but represent different restriction paths.
The restriction graph edges represent turns like they do in the edge-based routing graph. In particular, restriction paths that have the same prefix will share the same restriction graph nodes.
The restriction graph also has an additional feature called transfer edges, which are edges between nodes on two different restriction paths. A transfer edge from node A to B can be added if the path leading to node B is a suffix of the path represented by A.
Transfer edges allow us to represent overlapping restrictions without needing additional logic for tracking path history or multiple path states.
The final node in a restriction path stores the restriction instructions. Nested restrictions can lead to new instructions added to a node. For example, in this case the nested node restriction also applies to the via way restriction path.
Example Scenario
The construction and use of the restriction graph can be illustrated with the following scenario.
Take the above scene with the following restrictions applied.
Given a list of turn restrictions, the graph is created in multiple steps.
Step 1: create prefix trees for all restriction paths.
The restriction instructions are added to the final node in its path.
Step 2: add transfer edges between restriction paths that overlap.
Traverse each restriction path, tracking where the suffix of our current path matches the prefix of any other. If it does, there's opportunity to transfer to the other path if the transfer would not be restricted and that turn does not take us
further on our current path. Nested restrictions are also added from paths that match the suffix.
Step 3: Index start and via nodes
Start nodes and via nodes are indexed by the current edge of the path they represent for easy retrieval.
Step 4: Apply to edge-based graph
During edge-based routing graph construction:
Complexity
In the worst case, for each edge in a restriction via-way we have to create a duplicate node and duplicate all its turns in the edge-based graph. Given the low number of via-way restrictions in OSM, this will have low impact on the extracted graph size.
Whilst this level of turn restriction complexity is mostly hypothetical, it allows us to avoid building special-case logic for the subset of real-world occurrences, especially at complex intersections. Here is an example.
Other Changes
Non-compressible nodes in via-way
In the current implementation, a restriction with a via-way containing a non-compressible node (e.g. an intersection junction) would be filtered out.
To add support for these types of restrictions, we change a WayRestriction to track all nodes within the via way so that we know which edges remain after node compression. This requires changes to data preparation so that way nodes are tracked and not just turn masks.
Conditional/unconditional restrictions
Conditional and unconditional restrictions are combined to use the same representation. This makes it easier to construct the
restriction graph and deal with nested and overlapping restrictions. Given that turn restrictions all have conditional fields when parsed, this does not add any additional memory usage.
Performance
Initial testing shows negligible change in memory and time usage by
osrm-extract
. I will run some more thorough tests and add in a separate post.Circumventing restrictions
When testing this I noticed some multi-via restrictions are not strict enough and could likely lead to
unexpected routing behaviour. A good example of this is performing an extra loop to circumvent the restriction because too much of the initial path is included (example).
Tasklist
Requirements / Relations
Closes #4439
Possible next steps:
Restriction parsing optimisations: #4102
Supporting multiple from/to relations in no_entry and no_exit restrictions: #4241