-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Area Routing #7161
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
base: master
Are you sure you want to change the base?
Area Routing #7161
Conversation
cool feature! seems the linked screenshots don't show tho in the PR |
I re-uploaded them. Do they show now? |
They do. Thanks a lot |
Had a first look at the change. First of all, great feature! I think this has been on the todo list for more than 10 years 😀 The main question I have is that we should probably own the Dijkstra implementation that is used and not fall back to boost. |
Do we have a general dislike for boost? Just asking ... |
Perhaps 😉 Also, routing is our expertise and we should own all routing logic. |
Dijkstra in itself is trivial to implement. The hardest piece is the priority queue, if we don't want to take the one in boost. They use a 4-ary heap. A binary heap will do for our problem sizes. I will look into this. |
The good news is that heap implementations already exist in our code base:
|
just tweaked the settings s.t. GitHub Actions should now run automatically on push. |
I have now implemented Dijkstra and a binary heap that works with it. The d-ary heap in Shouldn't we drop / update the macos-x64 build since it is based on apple-clang-14? All other builds use clang >= 16. |
a15ab31
to
d89c932
Compare
Let me know when the change is ready for reviewing. 😉 |
5e1be1a
to
86fabdb
Compare
a36aa05
to
d15c029
Compare
@DennisOSRM I think this is ready for review. |
std::vector<std::pair<const osmium::Way &, extractor::ExtractionWay>> &, | ||
std::vector<extractor::InputTurnRestriction> &, | ||
std::vector<extractor::InputManeuverOverride> &) override final | ||
const extractor::ManeuverOverrideRelationParser &) override final | ||
{ | ||
} | ||
|
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.
TODO
- entrances to buildings along the perimeter of the area,
- PT stations in the middle of the area
This could also include other routable points like a bridge/tunnel/stairs starting in the middle of an area.
For example: this, otherwise unconnected, bridge starting/ending in two parking lots (OSM):
Further, we could include POIs in the middle of routable areas. Perhaps include specific classes like tourism=artwork
and amenity=drinking_water
, or more simply anything with a name=
tag?
My inbuilt assumption is mappers will lessen the use of artificial routes within a routable area, once apps support area routing, increasing the need of routing to individual POIs.
(comment pinned to a line of code to allow for threaded discussion)
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.
The main difficulty is to get all information into one place: in a typical OSM file all nodes come first, then all ways, then all relations. When you get to the area relation in the OSM file the node tags are long gone, so either you save all interesting nodes with their tags in memory (expensive in memory) or you save all interesting relations and make a second pass over the OSM file to get all the nodes and tags (less expensive in memory but expensive in cpu).
Further, if you want to include nodes that are 'inside' the area but not members of the area relation, you have to perform point-in-polygon tests for all nodes against all interesting areas. That means building an rtree of areas and a second pass over all nodes testing them all (very expensive in cpu).
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.
What percent of OSM files will the meshing run on? And what percent will have POIs inside? Perhaps we just have to keep the happy path fast, and the other to be bounded and reasonable?
Could do:
- Run all steps including the new area meshing (as your PR does currently)
- If meshing occurred – breaking off from the happy path
- Create lat/lng bounding box for each meshed area (fast to check against; granted some corner cases at the poles and wrapping of the world)
- Ignore any mesh area that is trivial small, like <3m width and <3m height, letting free space routing handle the case
- Second pass:
- Filter nodes:
- Fast filter: ignore nodes not in a one of the meshes' bounding boxes
- Slower filter: full check for point-in-polygon
- Slower filter: ignore nodes that are with-in 3m of an existing/new path in the mesh, letting free space routing handle the point
- Limiting step: keep at max ~10k nodes that pass the filters (further reduced later)
- Perhaps prioritizing keeping nodes with
name=
or other likely source/destination POI?
- Perhaps prioritizing keeping nodes with
- Store remaining nodes, then gather ways, relations
- Filter to POIs w/
name=
tags or other likely source/destination POI?
- Filter to POIs w/
- Limiting step: keep at max ~100 nodes (not POIs) for each mesh areas
- For any mesh area that we found POIs inside, rerun the meshing
- Filter nodes:
Upside: happy path is fast, low and bounded memory, bounded compute
Downside: need to remesh if new POIs are added inside of a mesh area, two passes when area mesh code runs
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.
This could also include other routable points like a bridge/tunnel/stairs starting in the middle of an area.
For example: this, otherwise unconnected, bridge starting/ending in two parking lots (OSM):
As a sidenote because it was used as an example, amenity=parking
areas cannot safely be assumed to be fully traversible. Very often the mapped parking lot area also includes grassy medians, etc... that cannot be walked/driven over.
The parking lot in the example isn't even mapped as an area, just as single nodes, so this wouldn't work there either way.
Attempting to automatically connect and route between two otherwise unconnected ways just because they are in close proximity but that are not known to lie in a routable area is absolutely not a good idea. That way lies madness.
This PR provides routing over areas.
It builds a visibility graph of the area, then runs dijkstra from each entry point to reduce the number of edges, then adds the remaining edges as ways to the router.
From the OSM site, without area routing, following mapped-for-the-router ways:
With area routing, also note the better directions:
Documentation
Issue
Fixes #64 #2655 #7146
Tasklist
TODO