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

OSM Map features are not exactly clipped by AOI #1904

Closed
manjitapandey opened this issue Nov 21, 2024 · 13 comments
Closed

OSM Map features are not exactly clipped by AOI #1904

manjitapandey opened this issue Nov 21, 2024 · 13 comments
Assignees
Labels
backend Related to backend code effort:medium Likely a day or two enhancement New feature or request priority:low Backlog of tasks that will be addressed in time testing:ready Ready for testing

Comments

@manjitapandey
Copy link
Contributor

Describe the bug
While generating the map features from osm, the features are not exactly clipped by AOI but rather map features outside of AOI to few buffer distance is visible.

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'create project'
  2. Generate osm map features
  3. See error

Expected behavior
the users should be provided option to choose either they want to perfectly clip the map features by AOI. May be a toggle on/off button to specify.

Screenshots
Image

Additional context
Had a talk with Kshitiz, he shared about https://api-prod.raw-data.hotosm.org/v1/redoc#tag/Extract/operation/get_osm_current_snapshot_as_file_snapshot__post
Image

if you use usestwithin = True , it will return only the features which are inside the geometry
it is false by default so will give you the features that are intersected in that area , this will work for buildings but for roads might not make sense because it will return the roads that are exactly inside polygon

@manjitapandey manjitapandey added the bug Something isn't working label Nov 21, 2024
@spwoodcock
Copy link
Member

spwoodcock commented Nov 21, 2024

Thanks @manjitapandey 🙏

Setting useStWithin: False was intentional on our behalf, because otherwise geometries that are only partly in the AOI are left out.
Say a building that is 90% in the AOI, just has 10% outside, then this will be omitted.

I'm surprised we are only just encountering this issue, as we have had this set for months. The geometries we are seeing are far outside the extent of the AOI, so I'm really surprised by this.
@kshitijrajsharma do you know anything that might have changed to make this the case? It might have just been sheer luck until now, who knows!

Anyway, if this is the only solution, then we might have to do it.
We could set useStWithin as always True if needed.

For the future, as part of the revised project creation workflow, add extra configurability:

  • For polygon-based data collection (buildings, etc), we can set useStWithin as True.
  • For polyline-based data collection (roads, etc), we can set useStWithin as False (as roads we want to map will definitely extend outside the AOI)

@kshitijrajsharma
Copy link
Member

@spwoodcock Thanks for the mention !
I haven't done any development on raw-data-api since past couple of months !
Output doesn't seem to be usual on the screenshot , it doesn't look like a filter problem , is it possible you are not simplifying the geometry ?
For testing I just made a request to rawdataapi with star shape in Pokhara

Pasting here the request body :

{"outputType":"geojson","uuid":true,"bindZip":true,"includeStats":false,"useStWithin":false,"centroid":false,"includeUserMetadata":false,"geometryType":["polygon"],"filters":{"tags":{"all_geometry":{"join_or":{"building":[]}}},"attributes":{"all_geometry":["name",""]}},"geometry":{"type":"Feature","properties":{},"geometry":{"type":"Polygon","coordinates":[[[83.968077,28.220881],[83.982067,28.209763],[83.963614,28.198341],[83.98344,28.203863],[83.978806,28.191987],[83.993139,28.204241],[84.016056,28.197963],[83.992109,28.209309],[84.002237,28.221713],[83.987989,28.213923],[83.968077,28.220881]]]}}}

And this is the output which looks perfectly fine to me

Image

Attaching the export zip file as well

RawExport_geojson_uid_b5d52672-15d7-4c1d-9c4e-a8fd04f610ad.zip

@kshitijrajsharma
Copy link
Member

kshitijrajsharma commented Nov 21, 2024

By taking closer look at screenshot :

Issue could be this : if you are passing multipolygons directly to rawdataapi , or polygons with hole inside , raw data api will try to fix the polygons to get a bounding polygon. Try passing valid polygons or atleast polygon without holes or geometry problem free ! May be use shapely union to find the outer boundary of the input area and making a request !

@kshitijrajsharma
Copy link
Member

I will also share the logic how raw data API tries to fix those geometry problems :
First it uses https://postgis.net/docs/ST_MakeValid.html to get the outer geometry of the overlapping features and fix general issue
Second : it runs https://postgis.net/docs/ST_Union.html to get the union of geometries ( if you have multiple distinct polygons )

I think the geometry pasted on the screenshot contains hole and small distinct polygons which is why raw data API tries to fix it by getting union and hence the extra area !

@spwoodcock
Copy link
Member

spwoodcock commented Nov 22, 2024

This is so helpful! Thanks so much Kshitij 🤩

I will dig into it tomorrow and see if we can find the cause.

Hopefully just converting multipolygon --> polygon first could fix the problem (if indeed multipolygons are being used).

The polygon may be wrapped in a FeatreCollection, so possibly that's related too.

@manjitapandey could you share the AOI geojson for debugging please? 🙏

@manjitapandey
Copy link
Contributor Author

These are the AOI I was using, ward 4 of Tokha municipality(without hole) and AOI of janapur (with hole)
Ward4_Boundary.zip
AOI_Splits (2).zip

@kshitijrajsharma
Copy link
Member

kshitijrajsharma commented Nov 22, 2024

Here is the export for your AOI

  • for the first : ward4 boundary ( Fun fact : This looks like a shape of dog even with small tail hahaaa) :
    Image

fmtm_test_tokha_geojson_uid_752543f7-0748-4da4-911a-0a9b191cdc32.zip
It looks fine to me

  • The second polygon has multiple issues , Image
    It has 41 polygons inside , It has holes and mostly inconsistent nodes , it also violates the polygon right hand rule Image

I think this is the exact issue , I recommend doing some geometry cleanup : Either letting user know their geometry has issues or getting the union out of geometry might work ( but this is slightly complex than I explained , polygons with holes are tricky ) this might also raise issues in other geometry operations too if you are doing any : probably splitting

Image

To test this theory :

I Tried fixing up the geometry
I tried to fix the right hand rule first , tried to get the union output it ( still left holes ) , I tried filling the holes with delete holes and then converted multiparts to single part to get the single polygon out of filled geom
Image

& Here is the output

Image

RawExport_geojson_uid_e9cccffa-1aa7-4ce8-b139-09214bdca977.zip

@spwoodcock
Copy link
Member

Thanks boss man! You definitely saved me some work, so I really appreciate it 😁

That's an interesting analysis for sure!

So ideally we should do a lot more preprocessing of the geom before passing to raw-data-api. Do you use ST_RemoveSmallParts to move the holes?

@kshitijrajsharma
Copy link
Member

kshitijrajsharma commented Nov 22, 2024

So ideally we should do a lot more preprocessing of the geom before passing to raw-data-api. Do you use ST_RemoveSmallParts to move the holes?

@spwoodcock Yes , Correct !!
Not not at them moment , I tried to use ST_RemoveSmallParts before as I mentioned the hole problem is little more complicated than I imagined it also don't fixes all the cases so instead I decided go with the union geometry !
Cheers Man ! and thanks @manjitapandey for the minute observation

@spwoodcock spwoodcock added enhancement New feature or request priority:low Backlog of tasks that will be addressed in time effort:medium Likely a day or two backend Related to backend code and removed bug Something isn't working labels Nov 22, 2024
@spwoodcock
Copy link
Member

spwoodcock commented Nov 22, 2024

@Sujanadh I assigned you if thats ok 🙏
(also @Anuj-Gupta4 could look at this if that works)
I'm going to be in Nepal so we can discuss, but realise I may not be as efficient as I would like with traveling etc.
This is lower priority than the XLSForm fixes though.

Summary

Problem

  • During project creation, users may pass in geojson geometries that are not only Polygons.
  • We have various functions in postgis_utils.py that are not actually PostGIS functions, some are shapely.
  • We already handle some cases, like grouping a multipolygon into a single polygon, and a geom being wrapped in GeometryCollection or FeatureCollection.
  • However we do not solve the problem of a Polygon geometry with holes included. See further details above in this thread.

Solution

  • Many of the geojson 'normalisation' functions we have in postgis_utils.py should actually use PostGIS where possible.
  • Ideally we can convert the function to handle (1) MultiPolygon --> Single Polygon, and also the (2) handling of Polygons with holes, by passing the GeoJSON to PostGIS, transforming, and returning the normalised geojson.
  • To facilitate, we could have a nice PostGIS function, say 'normalise_aoi_geojson', which does various things like ST_Union etc.
    • We can probably handle the GeometryCollection and Feature / FeatureCollection unwrapping in Python, as it's a bit of hassle in PostGIS.
    • The pass a single geometry to the PostGIS function, ensuring the final output Polygon is a single Polygon geometry, with no holes.

In the end we need to be able to flexibly handle user AOI input, normalise to Polygon with no holes, and pass that to raw-data-api for data extract generation.

@spwoodcock
Copy link
Member

Although the above is definitely nice to have, the actual error was something else for now!!

This is the issue in osm-rawdata: https://github.com/hotosm/osm-rawdata/blob/1cd905f4aeaa794a162210fb5f8d12da4932629d/osm_rawdata/postgres.py#L762

We shouldn't be doing a unary_union of geoms in a FeatureCollection

@Sujanadh
Copy link
Collaborator

Sujanadh commented Dec 2, 2024

The issue isn't due to unary_union, it does its work perfectly dissolving all inner polygons (but not holes) generating exterior ring as polygon.

Image

The actual issue is generated due to convex hull. The holes inside polygon also doesn't affect that much in generating osm-extracts, only if those holes doesn't contain any data inside or else they will be left out. In above case, those holes doesn't contain any data extracts within them so it won't create a problem. But it's best to dissolve those holes to avoid any possible discrepancies.

Why convex hull is creating problem?
Convex hull creates polygon that is the smallest convex shape that completely contains the original polygon. "Convex" means there are no inward dents; every interior angle is less than or equal to 180°. That means the convex hull only captures the outer boundary without any inward dips.

Result of convex hull:
Image

Original AOI:
Image

@spwoodcock
Copy link
Member

Excellent breakdown Sujan 🙌

Thanks for digging into this more deeply!

Hopefully an easy fix then 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend code effort:medium Likely a day or two enhancement New feature or request priority:low Backlog of tasks that will be addressed in time testing:ready Ready for testing
Projects
Development

No branches or pull requests

4 participants