Landuse buffer #2812

ratrun · 2023-04-30T05:47:37Z

Here I implemented a 10 meters search buffer and fixed a typo.

easbar · 2023-04-30T08:08:45Z

core/src/main/java/com/graphhopper/routing/ev/Landuse.java

@@ -23,7 +23,7 @@
 public enum Landuse {
    OTHER("other"), FARMLAND("farmland"), RESIDENTIAL("residential"),
    GRASS("grass"), FOREST("forest"), MEADOW("meadow"), ORCHARD("orchard"), FARMYARD("farmyard"),
-    INDUSTRIAL("industrial"), VINEYARD("vineyard"), CEMETRY("cemetry"), COMMERCIAL("commercial"), ALLOTMENTS("allotments"),
+    INDUSTRIAL("industrial"), VINEYARD("vineyard"), CEMETERY("cemetery"), COMMERCIAL("commercial"), ALLOTMENTS("allotments"),


Nice catch! I fixed this directly in the main branch

easbar · 2023-04-30T08:09:07Z

core/src/main/java/com/graphhopper/routing/util/AreaIndex.java

-        // todonow: do we really need the prepared geometry for osm areas?
        IndexedCustomArea<T> indexedCustomArea = new IndexedCustomArea<>(area, pgf.create(border));
        index.insert(border.getEnvelopeInternal(), indexedCustomArea);


Did you try anything in this direction?

No, I didn't. I'm also not sure what you mean by that comment.

Sorry, I didn't want to delete your comment. I restored it now.

Ok, I tried this in the meantime. The SRTRee only allows querying for rectangular shapes, so use it for the envelopes of the areas. But once we query the index and get the envelopes we need a more precise intersection check and this would be too slow if we did not create a prepared geometry for each area (I tested this and it was about 10x slower). So we need to prepare the geometries beforehand, even though it does not really matter what we put into the SRTree

easbar · 2023-04-30T08:13:12Z

I think I agree we should do something along this line, but did you try it and check the results? How does it affect the quality of the landuse encoded value and the performance of the import? Did you look for some examples where the landuse association is improved and maybe some where this leads to problems? Also I'm not sure if we should change it here, because it would affect the other usages of the area index as well.

ratrun · 2023-04-30T13:01:16Z

but did you try it and check the results? How does it affect the quality of the landuse encoded value and the performance of the import?

Did you look for some examples where the landuse association is improved and maybe some where this leads to problems?

So far I tested with the display feature of the graph in my home area only. From what I could see the problematic cases all work as they should.

Also I'm not sure if we should change it here, because it would affect the other usages of the area index as well.

OK, then we need another method.

…into landuse-buffer # Conflicts: # core/src/main/java/com/graphhopper/routing/ev/Landuse.java

ratrun · 2023-04-30T15:16:45Z

I ran a benchmark with the austria latest OSM file and compared your implementation against mine:
Your version: [main] INFO org.eclipse.jetty.server.Server - Started @330185ms
My version: [main] INFO org.eclipse.jetty.server.Server - Started @526005ms

I was able to reduce -Xmx5000m -Xms5000m into -Xmx4500m -Xms4500m, with 4000 I get an OOM exception.

So my version is unfortunately significantly slower 😞 . Anyway, I still think that it is an improvement which is worth the increased time. Do you have an idea how this can be optimised?

otbutz · 2023-05-02T08:18:18Z

@ratrun PreparedPolygon.intersects(Geometry) has an optimization if the polygon itself is a rectangle. So you would need to create a prepared version of your bbox polygon and switch the order in the intersection check method.

You could also use DistanceCalcEarth.createBBox().

@easbar i fear that these calculations might over- or underflow near the lat/lon extrema. There are no bound checks or logic to shift something like -180.0004 to 179.9996

graphhopper/core/src/main/java/com/graphhopper/util/DistanceCalcEarth.java

Lines 105 to 117 in 948294f

    
           public BBox createBBox(double lat, double lon, double radiusInMeter) { 
        
               if (radiusInMeter <= 0) 
        
                   throw new IllegalArgumentException("Distance must not be zero or negative! " + radiusInMeter + " lat,lon:" + lat + "," + lon); 
        
               // length of a circle at specified lat / dist 
        
               double dLon = (360 / (calcCircumference(lat) / radiusInMeter)); 
        
               // length of a circle is independent of the longitude 
        
               double dLat = (360 / (DistanceCalcEarth.C / radiusInMeter)); 
        
               // Now return bounding box in coordinates 
        
               return new BBox(lon - dLon, lon + dLon, lat - dLat, lat + dLat); 
        
           }

otbutz · 2023-05-02T08:23:05Z

It might also be worth to apply the TopologyPreservingSimplifier to the landuse polygons.

otbutz · 2023-05-02T10:56:54Z

core/src/main/java/com/graphhopper/routing/util/AreaIndex.java

+            new Coordinate(lon+rLon,lat-rLat),
+            new Coordinate(lon-rLon,lat-rLat)
+        };
+        Polygon poly = gf.createPolygon(coordinates);


Suggested change

Polygon poly = gf.createPolygon(coordinates);

Polygon poly = (Polygon) gf.toGeometry(searchEnv);

easbar · 2023-05-02T16:34:42Z

@ratrun PreparedPolygon.intersects(Geometry) has an optimization if the polygon itself is a rectangle. So you would need to create a prepared version of your bbox polygon and switch the order in the intersection check method.

But then we would have to create a prepared rectangle for every query (=for every edge), don't we?. As we only do a few (often only one) lookup for each rectangle preparation this could be even slower? Or maybe I did not understand what you mean? Also right now we first select the 'center' point for each edge and then use it as the center of a rectangle we query with. I'd rather use the entire edge and draw a buffer around it. And actually I'd rather buffer the landuse polygons, and then query with the edge, because we can easily parallelize the landuse buffering. I'm working on this currently.

@easbar i fear that these calculations might over- or underflow near the lat/lon extrema. There are no bound checks or logic to shift something like -180.0004 to 179.9996

I'm also not sure if we need this, since we use the JTS calculations on the unprojected coordinates anyway. Of course this is not accurate, but then we can just as well skip these calculations. And like I said I do not see much value in first converting an edge into a single point and then extending it to become a rectangle.

It might also be worth to apply the TopologyPreservingSimplifier to the landuse polygons.

Good idea, but when I tried this unfortunately the queries did not become much faster while running the simplification took a significant amount of time.

ratrun · 2023-05-02T18:13:07Z

Thanks for all the feedback! In f4ba483 I implemented the usage of the BBox and generate the polygon from the searchEnv. The result of a new benchmark was [main] INFO org.eclipse.jetty.server.Server - Started @554058ms, which is a bit slower compared to the previous version.

…check method

ratrun · 2023-05-02T19:19:48Z

PreparedPolygon.intersects(Geometry) has an optimization if the polygon itself is a rectangle. So you would need to create a prepared version of your bbox polygon and switch the order in the intersection check method.

If I understood it correctly I gave this a try in commit a4d34e4. The result of my benchmark was [main] INFO org.eclipse.jetty.server.Server - Started @645787ms, so it was even more slow.

I'd rather use the entire edge and draw a buffer around it. And actually I'd rather buffer the landuse polygons, and then query with the edge, because we can easily parallelize the landuse buffering.

I`m courious about this outcome here. I currently do not believe that such an approach could be faster, but we'll see.

otbutz · 2023-05-03T06:45:23Z

But then we would have to create a prepared rectangle for every query (=for every edge), don't we?. As we only do a few (often only one) lookup for each rectangle preparation this could be even slower?

Good point. Buffering the landuse polygons is the best way to scale this.

easbar · 2023-05-03T08:24:23Z

So my version is unfortunately significantly slower. Anyway, I still think that it is an improvement which is worth the increased time.

Can we take a step back and think about what we want to use landuse for? @ratrun which of the landuse values do you think are even relevant for routing? According to taginfo the most relevant values are:

farmland 21.92%
residential 21.10%
grass 13.94%
forest 12.98%
meadow 10.14%
orchard 3.26%
farmyard 3.08%
industrial 2.65%
vineyard 1.60%
cemetery 1.20%
commercial 1.03%
allotments 0.84%
retail 0.76%

Do you have some real examples in OSM where using the landuse tag improves the routes?

otbutz · 2023-05-03T08:46:16Z

IMHO, the most important use case would be the identification of built-up areas. Essentially a data-driven approach to improving the classification results obtained from #2637

{
  "speed": [
    {
      "if": "development == RESIDENTIAL || landuse == RESIDENTIAL",
      "limit_to": "50"
    }
  ]
}

easbar · 2023-05-03T08:57:17Z

the most important use case would be the identification of built-up areas

Yes, that is what I think as well. The thing I don't know is if the landuse tag can improve the development/urban_density. Do you think it does and did you find examples for this, yet?

otbutz · 2023-05-03T09:04:02Z

I would think so, yes. The heuristic approach is based on the density of the graph. A linear settlement with no driveways might be enough to cause a false-negative.

ratrun · 2023-05-03T16:22:13Z

Can we take a step back and think about what we want to use landuse for?

I would like to use landuse for greenery bicycle routing. There is an application based on graphhopper which does this for foot, but it is currently using a postgresql based approach. See trailrouter blogpost. For this purpose it is probably not so much relevant what the exact landuse value is, but rather if it is "greenery" or not. Please note that in addition to landuse we will need the natural areas later on as well in order to get the optimal result.

Do you have some real examples in OSM where using the landuse tag improves the routes?

Maybe we can use the example from this blogpost for the footprofile. Or this one from my area. With greenry routing I hope that the city of Vienna can be avoided.

otbutz · 2023-05-04T07:45:56Z

If a route avoids residential landuse, wouldn't it be considered greenery?

ratrun · 2023-05-04T14:59:01Z

If a route avoids residential landuse, wouldn't it be considered greenery?

Basically yes. But I expect problems with landuse=grass, this and the following list also needs to be considered as not greenery:

residential
industrial
commercial

otbutz · 2023-05-04T15:07:50Z

I would combine them to be honest. Residential is a bit misleading because the classification should rather be called "built-up" which includes residential, industrial, etc

easbar · 2023-05-05T10:49:58Z

But I expect problems with landuse=grass,

Why is landuse=grass not 'greenery'? Do you just want to stay out of populated areas (villages, cities and such)? Did you try urban_density to achieve this? Or do you want to distinguish also between different landuse values outside of populated areas (like staying in the forest rather than on farmland or something)?

easbar · 2023-05-05T10:52:24Z

core/src/main/java/com/graphhopper/routing/util/AreaIndex.java

    }

    public List<T> query(double lat, double lon) {
-        Envelope searchEnv = new Envelope(lon, lon, lat, lat);
+        BBox bbox = dc.createBBox(lon, lat, 10);


This should be lat, lon, 10 not lon, lat, 10, no?

Thanks for detecting this bug! I ran a new benchmark with the corrected order of the parameters and got following result: [main] INFO org.eclipse.jetty.server.Server - Started @596397ms, which is just a bit slower.

easbar · 2023-05-05T10:53:22Z

Or this one from my area. With greenry routing I hope that the city of Vienna can be avoided.

That should work fairly well using urban_density

easbar · 2023-05-05T10:56:54Z

A linear settlement with no driveways might be enough to cause a false-negative.

Yes, this is a case where urban_density might fail to identify a settlement. The landuse=residential tag fixes this if it encircles the whole settlement as it is often done for small villages.

ratrun · 2023-05-05T11:38:29Z

Why is landuse=grass not 'greenery'?

According to the wiki it is to be used for small areas. From what I have seen so far it is also used most often in urban areas. There people seem to map every small green spot.

ratrun · 2023-05-05T11:43:40Z

That should work fairly well using urban_density

You are probably right. But with the landuse encoded value it would also be possible to prefer forest with a higher priority compared to the other greenery values. This would be useful during the hot summer periods.

easbar · 2023-05-05T11:51:57Z

But with the landuse encoded value it would also be possible to prefer forest with a higher priority compared to the other greenery values

Ok so besides urban_density we would only need landuse=forest?

ratrun · 2023-05-05T12:15:57Z

Ok so besides urban_density we would only need landuse=forest?

I think that this would be sufficient, but other people might have other needs.

otbutz · 2023-05-05T13:07:27Z

According to the wiki it is to be used for small areas. From what I have seen so far it is also used most often in urban areas. There people seem to map every small green spot.

That would be an argument for me to ignore it.

ratrun added 2 commits April 30, 2023 07:42

Fix typo for cemetery

f2b64fb

Search buffer of about 10 meters

25f0be0

ratrun mentioned this pull request Apr 30, 2023

Add landuse encoded value based on closed-ring ways tagged with landuse=* #2765

Draft

easbar reviewed Apr 30, 2023

View reviewed changes

Merge branch 'landuse' of https://github.com/graphhopper/graphhopper …

6c133c7

…into landuse-buffer # Conflicts: # core/src/main/java/com/graphhopper/routing/ev/Landuse.java

otbutz reviewed May 2, 2023

View reviewed changes

Restore deleted comment

5cb28da

Using BBox and generate polygon from serchEnv

f4ba483

Create PreparedPolygon BBox and switch the order in the intersection …

a4d34e4

…check method

ratrun marked this pull request as draft May 4, 2023 15:45

easbar reviewed May 5, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Landuse buffer #2812

Landuse buffer #2812

ratrun commented Apr 30, 2023

easbar Apr 30, 2023

easbar Apr 30, 2023

ratrun Apr 30, 2023

ratrun May 2, 2023

easbar May 2, 2023

easbar commented Apr 30, 2023

ratrun commented Apr 30, 2023 •

edited

ratrun commented Apr 30, 2023

otbutz commented May 2, 2023 •

edited

otbutz commented May 2, 2023

otbutz May 2, 2023

easbar commented May 2, 2023

ratrun commented May 2, 2023

ratrun commented May 2, 2023

otbutz commented May 3, 2023

easbar commented May 3, 2023 •

edited

otbutz commented May 3, 2023

easbar commented May 3, 2023

otbutz commented May 3, 2023 •

edited

ratrun commented May 3, 2023

otbutz commented May 4, 2023

ratrun commented May 4, 2023

otbutz commented May 4, 2023

easbar commented May 5, 2023

easbar May 5, 2023

ratrun May 5, 2023

easbar commented May 5, 2023

easbar commented May 5, 2023 •

edited

ratrun commented May 5, 2023

ratrun commented May 5, 2023

easbar commented May 5, 2023

ratrun commented May 5, 2023

otbutz commented May 5, 2023

	Polygon poly = gf.createPolygon(coordinates);
	Polygon poly = (Polygon) gf.toGeometry(searchEnv);

Landuse buffer #2812

Are you sure you want to change the base?

Landuse buffer #2812

Conversation

ratrun commented Apr 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easbar commented Apr 30, 2023

ratrun commented Apr 30, 2023 • edited

ratrun commented Apr 30, 2023

otbutz commented May 2, 2023 • edited

otbutz commented May 2, 2023

Choose a reason for hiding this comment

easbar commented May 2, 2023

ratrun commented May 2, 2023

ratrun commented May 2, 2023

otbutz commented May 3, 2023

easbar commented May 3, 2023 • edited

otbutz commented May 3, 2023

easbar commented May 3, 2023

otbutz commented May 3, 2023 • edited

ratrun commented May 3, 2023

otbutz commented May 4, 2023

ratrun commented May 4, 2023

otbutz commented May 4, 2023

easbar commented May 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easbar commented May 5, 2023

easbar commented May 5, 2023 • edited

ratrun commented May 5, 2023

ratrun commented May 5, 2023

easbar commented May 5, 2023

ratrun commented May 5, 2023

otbutz commented May 5, 2023

ratrun commented Apr 30, 2023 •

edited

otbutz commented May 2, 2023 •

edited

easbar commented May 3, 2023 •

edited

otbutz commented May 3, 2023 •

edited

easbar commented May 5, 2023 •

edited