osm-fr / osmose-backend

Part of osmose that runs the analysis, and send the results to the frontend.
GNU General Public License v3.0
90 stars 115 forks source link

Polygon on border marked as "Should be polygon" #1947

Closed Famlam closed 1 year ago

Famlam commented 1 year ago

See https://osmose.openstreetmap.fr/nl/issue/cbdb8e5a-6eab-0cc7-c7cb-1d58b018c098 Item 1170 / class 4

This polygon (https://www.openstreetmap.org/way/1059958114) is located on the border of the Dutch province of Zeeland, mostly located outside the province border. It seems this is a false positive related to the border, not to the way itself.

Shouldn't this one have been filtered out already? I thought ways which crossed the extract border were excluded?

frodrigo commented 1 year ago

Shouldn't this one have been filtered out already? I thought ways which crossed the extract border were excluded?

No issue marker outside the border are excluded. But here is depend where the marker is set.

Famlam commented 1 year ago

Ah ok. I'll check (soon) whether this is a rare case (in which case I'll mark it as false positive and just close the issue) or whether it's more common. In the latter case, probably we should just check that node[1] != node[last]. That should also work for invalid (due to being incomplete) polygons.

Famlam commented 1 year ago

It appears to be a relatively common issue. For example, the province of Zeeland (from the initial post) has two: https://osmose.openstreetmap.fr/nl/issue/cbdb8e5a-6eab-0cc7-c7cb-1d58b018c098 https://osmose.openstreetmap.fr/nl/issue/8a989242-5c8f-ea0d-a679-9ac9e7e8d210

An example from Chubu: https://osmose.openstreetmap.fr/nl/issue/24b70783-3872-83b0-3071-9672416b9c51

Venezuela & surrounding have a lot: https://osmose.openstreetmap.fr/nl/map/#zoom=8&lat=2.695&lon=-64.132&item=1170&level=1&tags=&fixable=&class=4 (Zoom out a bit to the right to find many more for Brazil)

The trick to find them is to add the following line to the SQL query: ways.nodes[1] = ways.nodes[array_length(ways.nodes,1)] AND NOT ST_IsClosed(ways.linestring) The nodes id check will work if not all nodes are downloaded. St_IsClosed does not work if not all nodes are downloaded. Hence, the difference between the two are the bad items.

Since there is already an "invalid polygon" check in another analyser, I suspect it's fine to add ways.nodes[1] != ways.nodes[array_length(ways.nodes,1)] to this analyser to exclude such cases

Famlam commented 1 year ago

Are both PR's already live? The Zeeland & Chubu cases have disappeared, but around Venezuela nothing changed despite todays timestamp

frodrigo commented 1 year ago

Are both PR's already live? The Zeeland & Chubu cases have disappeared, but around Venezuela nothing changed despite todays timestamp

Not sure, I reployed all.

Famlam commented 1 year ago

Lets keep an eye on https://osmose.openstreetmap.fr/nl/issue/e71a07ba-4304-273c-0f80-4321e83e25c1 When I last checked (with the version of osmosis where 1-node-inside-extract ways were also being closed) this one should've been gone. However, since this is a 3-node-inside-extract way (with the first=last node outside the extract) it should disappear anyway.

EDIT: confirmed with osm110, way 1118949000 is not supposed to be detected anymore.

The output that osm110 gives for Venezuela, item 1170, class 4 ```xml ```

p.s.: also the osm110 output isn't perfect yet, e.g. ways 1118949009, 1118949010, 1118582455 and others should also be removed. Will debug this at a later stage, but it seems I need to add something like (NOT ST_IsClosed(ways.linestring) OR ST_NPoints(ways.linestring) > 3), although that'd still fail for cases like https://www.openstreetmap.org/way/1118668190. Maybe ST_IsRing instead of NPoints?