Skip to content
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 landuse encoded value based on closed-ring ways tagged with landuse=* #2765

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

easbar
Copy link
Member

@easbar easbar commented Mar 6, 2023

I built a quick prototype, which:

  • stores the way tags and OSM node Ids for all OSM ways tagged with landuse=* in memory during the first pass of OSMReader

  • finds all the corresponding node coordinates in the second pass

  • builds an area index for all these (closed-ring) polygons before we read the OSM ways again in the second pass

  • finds the areas each edge is contained in and feeds them to the tag parsers

  • adds a tag parser for the new landuse encoded value, which uses the smallest of these areas to set the landuse value

This way we can, e.g., use custom models that prefer forest areas, or slow down the speed in residential areas etc.
We can also just look at the landuse values using path details:

image

My first quick test resulted in an import time of 117s compared to 80s for a map of Bavaria with the master branch.

The biggest issue I ran into so far was that landuse areas are often mapped as multipolygons many forest areas seem to be mapped as relations with such polygons as members. Parsing the relations wouldn't be much harder, but since we only get to see them at the end of the first pass (after the ways) we would have to read the OSM file a third time :(

@easbar
Copy link
Member Author

easbar commented Mar 6, 2023

The biggest issue I ran into so far was that many forest areas seem to be mapped as relations with such polygons as members.

The OSM wiki even says that landuse should not be tagged on relations, but there are currently around 1.8mio such relations in OSM :(

"Although multipolygons are technically relations relation, their usage is generally accepted for tags that only list usage on areas area (and for which the wiki states these tags should NOT be used on relations). This is an exception since multipolygons are relations specifically meant to represent areas (polygons). For instance: natural=* is used 2,5 million times on relations; landuse=* 1,8 million times (as per Taginfo, 2023). "

For the Bavaria map I used there were around 634k ways using the landuse tag, and around 23k such multipolygon relations, with a total of around 125k (way) members. So around 16% of the areas were given as members of landuse relations.

@easbar easbar temporarily deployed to benchmarks March 6, 2023 21:03 — with GitHub Actions Inactive
@easbar
Copy link
Member Author

easbar commented Mar 6, 2023

I added support for multipolygon landuse relations in my second commit (using a third pass of the OSM file). At least for Bavaria this improves the handling of landuse=forest a lot. Here is a bike route that tries to stay in the forest (indicated by the red color in the path details diagram here) as much as possible:

image

@easbar easbar temporarily deployed to benchmarks March 8, 2023 12:54 — with GitHub Actions Inactive
@easbar
Copy link
Member Author

easbar commented Mar 8, 2023

With #2770 the third (0th) pass only took around 11s for Bavaria:

Finished reading OSM file. pass0: 11s,  pass1: 21s,  pass2: 79s,  total: 111s

So using the landuse relations does slow down the import (because of the area lookups) (~62s total in master), but doing the third run (which only reads the relations) isn't making it so much worse.

# Conflicts:
#	core/src/main/java/com/graphhopper/routing/ev/DefaultEncodedValueFactory.java
#	core/src/main/java/com/graphhopper/routing/util/parsers/DefaultTagParserFactory.java
@easbar easbar temporarily deployed to benchmarks March 18, 2023 07:00 — with GitHub Actions Inactive
# Conflicts:
#	core/src/main/java/com/graphhopper/reader/osm/OSMReader.java
@easbar easbar temporarily deployed to benchmarks March 23, 2023 09:51 — with GitHub Actions Inactive
@ratrun
Copy link
Contributor

ratrun commented Apr 1, 2023

This branch looks very promising, although it is quite expensive in terms of RAM and preparation time. I tested the extract for Austria with four bicycle profiles active and had to increase the RAM size from 3GB to 5GB.
I just noticed a litte draw-back and this is that in some areas the way does not overlap with the landuse polygon, it is just very very close. In such cases it would be great if it would be possible to utilize the near by landuse polygon. Do you see the possibility for an enhancment to take a lets say a 20 meters margin into account?

@easbar
Copy link
Member Author

easbar commented Apr 18, 2023

I just noticed a litte draw-back and this is that in some areas the way does not overlap with the landuse polygon, it is just very very close. In such cases it would be great if it would be possible to utilize the near by landuse polygon. Do you see the possibility for an enhancement to take a lets say a 20 meters margin into account?

Yes, maybe that is actually quite a good idea. Also because in some cases for example forest areas are separated by roads like here:

image

even though the road clearly leads 'through' the forest.

@otbutz
Copy link
Contributor

otbutz commented Apr 18, 2023

@easbar
Copy link
Member Author

easbar commented Apr 18, 2023

Should be doable using jts?

I did not look into it yet, but yes this sounds like what we need, thanks! Precision isn't very important for us here I think, we just need to widen the polygons a bit. The tricky part might be dealing with overlapping landuse polygons then.

@otbutz
Copy link
Contributor

otbutz commented Apr 18, 2023

JTS should be able to properly merge them.

@easbar
Copy link
Member Author

easbar commented Apr 18, 2023

JTS should be able to properly merge them.

I meant landuse polygons with different tag values. For example when a road is located near the border between an forest and a farmland area and we extend both polygons we need to decide which landuse value we like to use for this road.

Btw here is a nice visualization of the landuse tag: https://osmlanduse.org

# Conflicts:
#	core/src/main/java/com/graphhopper/reader/osm/OSMReader.java
@easbar easbar temporarily deployed to benchmarks April 18, 2023 08:15 — with GitHub Actions Inactive
@otbutz
Copy link
Contributor

otbutz commented Apr 18, 2023

I thought you meant overlapping parts of multipolygons. Aren't different overlapping landuse polygons already quite common? It would be nice if they weren't and everyone made proper multipolygons with holes in them, but in reality there's going to be quite a lot of overlap.

You'll most likely need to define some kind of sorting behaviour. e.g preferring non-buffered matches and polygons with a smaller area.

@otbutz
Copy link
Contributor

otbutz commented Apr 18, 2023

It's probably easier to leave the polygons as they are and instead make the search a little less precise.

@karussell
Copy link
Member

(Or instead of a point in polygon lookup there is a "circle (or bbox) with polygon" intersection possibility)

} else {
customAreas = emptyList();
if (areaIndex != null) customAreas = areaIndex.query(middleLat, middleLon);
if (osmAreaIndex != null) osmAreas = osmAreaIndex.query(middleLat, middleLon);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @karussell pointed out, we should query by envelope here.

public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay readerWay, IntsRef relationFlags) {
List<CustomArea> osmAreas = readerWay.getTag("gh:osm_areas", null);
if (!osmAreas.isEmpty()) {
osmAreas.sort(Comparator.comparing(CustomArea::getArea));
Copy link
Contributor

@otbutz otbutz Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe create a buffer around the edge and search for the OSM area with biggest "hit ratio" ? e.g sorting based on the size of the intersecting area.

@otbutz
Copy link
Contributor

otbutz commented Apr 19, 2023

I'd love to see a follow up PR that uses the landuse information to augment the urban density EV.

@easbar easbar temporarily deployed to benchmarks April 26, 2023 16:12 — with GitHub Actions Inactive
# Conflicts:
#	core/src/main/java/com/graphhopper/reader/osm/OSMReader.java
#	core/src/main/java/com/graphhopper/reader/osm/WaySegmentParser.java
…r ile-de-france

for comparison: after pass1 we'd normally use around 76MB after pass1 and around 420MB after pass2 for way segment parser and another ~140MB for the graph (a total of around 560MB)
@easbar easbar temporarily deployed to benchmarks April 29, 2023 11:21 — with GitHub Actions Inactive
@easbar easbar temporarily deployed to benchmarks April 29, 2023 12:33 — with GitHub Actions Inactive
@easbar easbar temporarily deployed to benchmarks April 29, 2023 12:34 — with GitHub Actions Inactive
@easbar easbar temporarily deployed to benchmarks April 29, 2023 12:36 — with GitHub Actions Inactive
@easbar easbar temporarily deployed to benchmarks April 29, 2023 13:34 — with GitHub Actions Inactive
@ratrun
Copy link
Contributor

ratrun commented Apr 30, 2023

I created PR #2812 which implements the mentioned search buffer from above. @easbar Can you please check?

@easbar easbar temporarily deployed to benchmarks April 30, 2023 08:14 — with GitHub Actions Inactive
@easbar
Copy link
Member Author

easbar commented Apr 30, 2023

although it is quite expensive in terms of RAM and preparation time. I tested the extract for Austria with four bicycle profiles active and had to increase the RAM size from 3GB to 5GB.

The RAM requirement should be much lower now, can you please try again? Note that I also removed the relation handling for now, and maybe do this in a follow-up.

@easbar
Copy link
Member Author

easbar commented Apr 30, 2023

Also because in some cases for example forest areas are separated by roads like here

The same problem exists for residential areas where roads might be excluded explicitly from the residential landuse areas, like here (extract from Berlin):

image

Then again, in these cases the roads are probably often tagged as highway=residential, too.
Querying with a buffer like you proposed should fix this.

@ratrun
Copy link
Contributor

ratrun commented Apr 30, 2023

Querying with a buffer like you proposed should fix this.

Yes, it does:

BerlinLanduse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants