osmlab / osmlint

An open source suite of js validators for OpenStreetMap data, to identify common geometry and metadata problems at scale.
ISC License
84 stars 10 forks source link

[Validator] Crossing buildings #24

Closed Rub21 closed 7 years ago

Rub21 commented 8 years ago

cc . @geohacker @maning

maning commented 8 years ago

Lets make this more generic to crossing polygons/closed ways. For example, for polygons/closed ways with the same kv pair (landuse=*, building=*, natural=*), check for overlaps.

Lets ignore multipolygons relations for now.

@geohacker @planemad @Rub21

planemad commented 8 years ago

Yes, the crossing linter should accept a list of keys: ["landuse", "building", "natural", "amenity"] and produce the overlapping objects.

@maning any particular scenario for which you think we need to compare tag values as well?

maning commented 8 years ago

Generally, same key should not cross. But there are edge cases, i.e. amenity=something within amenity=parking. Let's avoid catchall detectors. We can fine tune detectors as we encounter them.

Rub21 commented 8 years ago

@aaronlidman @geohacker @morganherlocker , Could you help me here ,https://github.com/osmlab/osmlint/blob/crossingbuildings/validators/crossingBuildings/map.js

so, i am trying to do validator for crossing buildings, i am using turf.intersect, but it throws error for monaco.mbtiles, and work for peru.mbtiles. we are using monaco.mbtiles for testing.

other problem is:

Perhaps, Could you recommend other ways to do this detection?

Rub21 commented 8 years ago

Per chat with @aaronlidman , here is the geojson of Monaco buildings

Rub21 commented 8 years ago

@maning a result from crossing buildings, https://gist.github.com/anonymous/42958ff767c141b20d98, to geenerate the result, it take around 40 min , I modified quiet bit, but it still throwing a error in some places . https://github.com/osmlab/osmlint/blob/crossingbuildings/validators/crossingBuildings/map.js

maning commented 8 years ago

@Rub21 Checked a few (12 buildings) from your sample data all are confirmed crossing buildings. Yay!

We need to formalize what tags should be included. I don't think it should be this way, properties":{"way1":169254869,"way2":169254828}. Lets make each error as one object similar to other validators/analyzers/linters/processors (can we decide on what to call them?) and add the other OSM tags and metadata. Related ticket: https://github.com/osmlab/osmlint/issues/30

@geohacker, can you :eyes: at the code? Intersection functions are always expensive, we may need some refactoring on this one.

geohacker commented 8 years ago

@Rub21 questions -

  1. This filter removes buildings that self intersect. https://github.com/osmlab/osmlint/blob/crossingbuildings/validators/crossingBuildings/map.js#L9-L16 Why?
  2. Why do you change the geometry type to LineString here? - https://github.com/osmlab/osmlint/blob/crossingbuildings/validators/crossingBuildings/map.js#L13-L14
  3. All objects in buildings are of type LineString, but turf.intersect takes two Polygons and returns a Polygon, undefined or MultiLineString. In which case I think this line will fail?

I think the reason why this is slow is because of the loop. For every building it loops through the entire set of buildings.

Ideas to consider - we don't have to compare every building to every other building in the set. We can limit this by a distance threshold. We are only interested in comparing buildings that are close enough:

  1. Check the distance between the buildings (their centroids) and if they are < ~10 meters apart then do the intersection. Assuming that finding distance is cheaper than calculating intersection.
  2. Slightly ambitious and not sure if this would work - calculate a distance matrix for all buildings in the tile and check for combinations that are within ~10m.
Rub21 commented 8 years ago

@geohacker :

This filter removes buildings that self intersect. https://github.com/osmlab/osmlint/blob/crossingbuildings/validators/crossingBuildings/map.js#L9-L16 Why?

I got an error : https://github.com/osmlab/osmlint/issues/29, for that I've considered avoid the self-intersect Polygons , maybe we can do this detection for other validator.

Why do you change the geometry type to LineString here? - https://github.com/osmlab/osmlint/blob/crossingbuildings/validators/crossingBuildings/map.js#L13-L14

For polygons, it give us more errors than Linestring, and also give me rare polygon with just two coordinates, it can be sound strange but i saw it, and give many false positive as well. for that I preferred change to Linestrig, even in Linestring, we can get the exact intersection and it should be more easy to detect where is the overlap.

All objects in buildings are of type LineString, but turf.intersect takes two Polygons and returns a Polygon, undefined or MultiLineString. In which case I think this line will fail?

turf.intersect is working for Linestring as well.

Check the distance between the buildings (their centroids) and if they are < ~10 meters apart then do the intersection. Assuming that finding distance is cheaper than calculating intersection.

Yeah, maybe it can be work, but we have to compare the distance for entire set of buildings as well.

Slightly ambitious and not sure if this would work - calculate a distance matrix for all buildings in the tile and check for combinations that are within ~10m.

This sound interesting can you give me more context about this.

morganherlocker commented 8 years ago

Ideas to consider - we don't have to compare every building to every other building in the set. We can limit this by a distance threshold. We are only interested in comparing buildings that are close enough

I would consider a spatial index here like rbush. This will speed up processing dramatically and with full precision.

geohacker commented 8 years ago

For polygons, it give us more errors than Linestring, and also give me rare polygon with just two coordinates, it can be sound strange but i saw it, and give many false positive as well. for that I preferred change to Linestrig, even in Linestring, we can get the exact intersection and it should be more easy to detect where is the overlap.

@Rub21 Not sure if I understand this. Do you have an example? Outright it seems wrong me to change the geometry type of a feature.

Rub21 commented 8 years ago

@geohacker @batpad @maning : I just improved a bit this crossing building validator. https://github.com/osmlab/osmlint/commit/9d4435c30430c3d540478cf88366a42714693840 results:

I got some false positive. it is because the data in osm-qa-tiles at zoom 12 the shapes change a little bit.

Rub21 commented 8 years ago

rbush is amazing!! , now the crossing building validator is hundred times faster, it processing really fast the crossing buildings.

Next acction :

Thanks @morganherlocker for the advice.

maning commented 8 years ago

I'm seeing many false-positives.

Building properly connected

screen shot 2016-01-20 at 14 53 41

Very close, but no overlap (refer to the scalebar UL corner)

screen shot 2016-01-20 at 14 51 06

screen shot 2016-01-20 at 14 45 51

screen shot 2016-01-20 at 14 45 04

morganherlocker commented 8 years ago

I'm seeing many false-positives.

@maning Can you post some of the geometries so these can be made into test fixtures?

Rub21 commented 8 years ago

@morganherlocker one of the false-positive: https://gist.github.com/anonymous/f91b3f96800657dbf715

In osm-qa-tiles change the geometry and the validator detected it as error.

we can avoid those false-positive increasing the number of are (area > 0.4) to 1.2. but many other errors are avoided as well https://github.com/osmlab/osmlint/blob/crossingbuildings/validators/crossingBuildings/map.js#L32

maning commented 8 years ago

@Rub21 any status on this? I would like to run this for linting labuildings.

maning commented 8 years ago

@geohacker @Rub21 @arunasank Can we do this? Context here: https://github.com/osmlab/labuildings/issues/92#issuecomment-222731623

Rub21 commented 8 years ago

The problem here was with the mbtiles in zoom 12, this should be working better for mbtiles upper zoom 15. Close Here!

maning commented 7 years ago

@Rub21 Would difference work better than intersect?

cc/ @geohacker

Rub21 commented 7 years ago

@maning since we really need this validator for LA, I will figure out again to detect this issues, and thank you for your suggestion.

Rub21 commented 7 years ago

ready here https://github.com/osmlab/osmlint/pull/205!!!