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

Unify building parts with building #653

Closed
rmarianski opened this issue Mar 31, 2016 · 13 comments
Closed

Unify building parts with building #653

rmarianski opened this issue Mar 31, 2016 · 13 comments
Assignees
Milestone

Comments

@rmarianski
Copy link
Member

rmarianski commented Mar 31, 2016

In the buildings layer, it would be useful to know the building that all the parts were associated with. This can be done by seeing if the building:part overlaps a certain amount with any buildings in the
tile.

@nvkelso nvkelso added this to the v0.10.0 milestone Apr 1, 2016
@nvkelso
Copy link
Member

nvkelso commented Apr 1, 2016

This can be tracked directly with the building relation per building part. SF example: Ferry Building.

We added root_relation_id for transit stations in v0.9, let's reuse the property & extend the logic for buildings.

@rmarianski
Copy link
Member Author

The problem is, at least in nyc, the buildings and their parts aren't always linked with a relation. For example: http://www.openstreetmap.org/way/160967745

@zerebubuth
Copy link
Member

Yup, the logic required is more similar to #142. We might be able to use the "base" building outline as the "root ID", depends on whether we find that many building parts overlap multiple "base" buildings.

@nvkelso nvkelso modified the milestones: v1.0.0, v0.10.0 Apr 4, 2016
@nvkelso
Copy link
Member

nvkelso commented Jul 18, 2016

Related: #886

@nvkelso
Copy link
Member

nvkelso commented Jul 18, 2016

It's sounding like we should say root_id (and rename the transit root_relation_id to root_id).

@nvkelso
Copy link
Member

nvkelso commented Aug 25, 2016

@meetar I remember you were interested in this one? Can you please have a look at dev tiles to verify this one this week?

New source url setup to use is:

  • http://tile.dev.mapzen.com/vector/v1/all/{z}/{x}/{y}.mvt

@nvkelso nvkelso assigned meetar and nvkelso and unassigned rmarianski Aug 25, 2016
@meetar
Copy link
Contributor

meetar commented Aug 26, 2016

This is great news!

I see some things which make sense here, such as features with a building_part: yes tag plus a root_id tag pointing to the root feature.

But then I see some others which simply have an osm_relation: 1 tag instead, such as the Bank of England building at http://tangrams.github.io/explorer/#17.0/51.5143/-0.0890/osm_relation/true:

screen shot 2016-08-26 at 3 38 26 pm

In our tiles, the inner and outer shapes here seem to be split into two features, correlating to
https://www.openstreetmap.org/way/4959510 and https://www.openstreetmap.org/relation/553472, but with no root_id or other node which would associate them with each other.

Should this kind of situation have been fixed in the changes mentioned above, or is this a separate issue?

@rmarianski
Copy link
Member Author

So far it only unified building parts with the building they overlap with. Sounds like in addition to that logic, we'd want to also link buildings / parts to any relation that they are a part of? And I assume that we'd prefer to use any relation as the root id in ambiguous cases?

@zerebubuth
Copy link
Member

Looks like there's a whole mess of things going on with the BoE building:

  • Relation 553472 is a multipolygon relation, with building=yes, amenity=bank and osm2pgsql puts it in the polygons table.
  • Way 4959510 is an outer of the above relation, but since it has different tags from its relation, (building=public), osm2pgsql cautiously retains it. This means there's a second polygon in the polygons table with the same outline as relation 553472 but without the holes.
  • Relation 2917677 is also present and although it doesn't have any tags itself, osm2pgsql processes it to "steal" the tags from its outer (way 220644664), so it also ends up as a building.

I think we have two different issues here:

  1. Relation 553472 and way 4959510 are both included as buildings due to a combination of tagging and how osm2pgsql interprets that tagging.
  2. Relation 2917677 is an additional building on a similar footprint to relation 553472, which seems like it should be linked to the latter although it's not a building:part, and there's no relation linking them or containing them both (e.g: a "site" relation).

If this isn't a widespread problem, I'd be tempted to call it a data issue rather than introduce a special case to handle it. Although it would be nice to be able to present problems like this in a way that makes it easy for people to find and fix.

@meetar
Copy link
Contributor

meetar commented Aug 30, 2016

Ah ha – The tagging looked odd to me but I couldn't tell how, thanks for
unraveling it. I can do some more investigating to see how common that kind
of thing is, I believe I ran into a few others like it but I didn't keep
track… but I concur, adding ad hoc logic will give us an infinitely-long
rule set.

On Tue, Aug 30, 2016 at 7:37 AM, Matt Amos notifications@github.com wrote:

Looks like there's a whole mess of things going on with the BoE building:

I think we have two different issues here:

  1. Relation 553472 and way 4959510 are both included as buildings due
    to a combination of tagging and how osm2pgsql interprets that tagging.
  2. Relation 2917677 is an additional building on a similar footprint
    to relation 553472, which seems like it should be linked to the latter
    although it's not a building:part, and there's no relation linking
    them or containing them both (e.g: a "site" relation).

If this isn't a widespread problem, I'd be tempted to call it a data issue
rather than introduce a special case to handle it. Although it would be
nice to be able to present problems like this in a way that makes it easy
for people to find and fix.


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#653 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAcEwnX48rCGvAUZCrz9-UYy01Sz8Pjmks5qlBXvgaJpZM4H9NNU
.

@rmarianski
Copy link
Member Author

That all being said, if we had a relation with multiple buildings, would we want them all to be linked? As it stands now they will not be. The current logic will only link building parts with the largest overlapping building.

@nvkelso
Copy link
Member

nvkelso commented Sep 2, 2016

@rmarianski by relation with multiple buildings do you mean @zerebubuth's example Relation 553472 as a multi polygon relation? I think we should add logic for that case.

@nvkelso
Copy link
Member

nvkelso commented Sep 9, 2016

I've verified Rob's basic work, let's split off the Bank of England work into a new ticket (#1032).

screen shot 2016-09-08 at 23 13 35

screen shot 2016-09-08 at 23 13 48

screen shot 2016-09-08 at 23 14 11

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

No branches or pull requests

4 participants