a-b-street / osm2lanes

A common library and set of test cases for transforming OSM tags to lane specifications
https://a-b-street.github.io/osm2lanes/
Apache License 2.0
34 stars 2 forks source link

Cutover A/B Street to using osm2lanes #71

Open dabreegster opened 2 years ago

dabreegster commented 2 years ago

osm2lanes is nearly at a point where I'd like to make A/B Street take a dependency on it and remove the original copy of the osm2lanes logic. Just writing down some notes about how to transition, not sure when I'll have time to tackle this.

The steps are probably: 1) Replace is_road, which initially decides which OSM ways to treat as roads in A/B Street 2) Replace get_lane_specs_ltr 3) Import a few maps and manually look for diffs -- I expect to find a few obvious ones, and iterate on osm2lanes or the way A/B Street calls it accordingly 4) After most issues seem fixed, import all maps and do the usual screenshot diff-based validation procedure

droogmic commented 2 years ago

Sounds good,

Replace is_road

This still needs to be implemented in osm2lanes right?

The rest seems relatively clear, cutovers are always challenging...

dabreegster commented 2 years ago

This still needs to be implemented in osm2lanes right?

I just tried the web viewer... plug in https://www.openstreetmap.org/way/39365402, a building, and it rewrote into a road with 2 lanes. So there's the answer. :) If the input doesn't represent something road-like, we should probably bail out cleanly.

is_road has lots of strange A/B Street logic again; we shouldn't preserve that in osm2lanes

dabreegster commented 2 years ago

FYI I started this at https://github.com/a-b-street/abstreet/tree/osm2lanes. https://github.com/a-b-street/abstreet/blob/osm2lanes/raw_map/src/lane_specs.rs has the conversion logic. So far, not bad!

Some of the biggest issues jumping out at me:

1) unimplemented: highway=construction, example https://www.openstreetmap.org/way/663639108 2) Very high width values. Some config based on highway={service, residential, primary} would go a long way 3) unsupported: cycleway:right=opposite_lane, example https://www.openstreetmap.org/way/6287847 (I see a separate cycleway tagged there too... who knows) 4) Loads of errors that occur in practice, like unsupported: more than one bus lanes scheme used, example https://www.openstreetmap.org/way/39817102

I've got enough other things to juggle to help out anytime soon, sorry. :\ But I am working on extracting the logic for making road/intersection polygons into a separate crate, adding unit tests, and taking other steps to make it usable outside of A/B Street

droogmic commented 2 years ago

unimplemented: highway=construction

do you need any metadata associated with this, or is type=construction alone enough?

https://www.openstreetmap.org/way/6287847

I think that is just bad tagging... https://wiki.openstreetmap.org/wiki/Key:cycleway:right?uselang=en and the cycleway is tagged separately... It should be cycleway:right=separate, and be ignored until we have a way of merging these ways.

https://www.openstreetmap.org/way/39817102

fascinating example, so many random tags just thrown together... I think this is where we can start using the Infer<> values to "build" a best guess, and throw warnings whenever there is something suspicious. The error for now should also have been unimplemented, not unsupported.

dabreegster commented 2 years ago

do you need any metadata associated with this, or is type=construction alone enough?

type = construction is nearly enough. I think using construction={residential,cycleway,primary, ...} is useful to feed into the width heuristics though.

I think that is just bad tagging...

100% agree... This is common though. A/B Street needs to do something for broken tagging besides crashing. My approach in the past has been to try to best-guess what the tagger meant, but I think that led to the super messy v1 of the tags_to_lanes implementation. I vote we stay strict in the new osm2lanes. In A/B Street, I can just transform broken roads into something obviously wrong, like a super skinny one-way one lane road, and display some kind of error. If these errors happen frequently in areas I personally need for other work, I'll pre-process tags on my end to "fix" them or attempt to fix upstream in OSM.

So... osm2lanes should stay strict.

I think this is where we can start using the Infer<> values to "build" a best guess

Same as above -- I have no idea what the mappers meant here. :D I'd honestly be happy just quasi-refusing to attempt to render this in A/B Street going forward, to encourage cleaning up the tagging. Handling the best guess in osm2lanes is an option, but 1) I would vote that complexity stays cleanly separated from the rest of the codebase, something like tags_to_lanes/repair/bus.rs 2) I wouldn't prioritize working on these inferences over handling more "correct" tagging

droogmic commented 2 years ago

Do you need an is_road clone,

or can you do: Highway::from_tags(&tags).is_supported()?

droogmic commented 2 years ago

my next 2 PRs, in no particular order:

dabreegster commented 2 years ago

I can use Highway::from_tags(&tags).is_supported(). I suspect I'll keep some of the weird filtering and transformation of is_road in the short-term, or move it just after this method.

droogmic commented 2 years ago

so construction is now in, where do we stand?

dabreegster commented 2 years ago

I will revisit after March 23, sorry :( Very high-stakes conference / deadline to finish the LTN work

droogmic commented 2 years ago

(reminder)

dabreegster commented 2 years ago

I did not get less busy yet. :(

But when I revive this, I'll use it as a forcing function to start some kind of locale-specific width config to roughly match areas I've spent a little time tuning before.

droogmic commented 2 years ago

Ah, width is missing. I am mainly using this to prioritise my efforts, because at this point the only thing I really see left to do is to iterate through tests cases trying to turn them all on.

dabreegster commented 2 years ago

Ah, yeah. Biggest missing thing from my POV is width. And if we wind up with a public API sort of like https://github.com/a-b-street/abstreet/blob/191f1ca265a51563995460f5375913ed30f7895a/raw_map/src/types.rs#L472 (but with locale, highway type, and lane type as input), even better, because then A/B Street's lane editor UI will just automagically make more sense in different places.

droogmic commented 2 years ago

I think the latest version will improve things, I haven't set up a dev environment for abstreet yet to check myself though, will do that ASAP

dabreegster commented 2 years ago

I'll update the draft PR with latest osm2lanes real quick. I don't think I can work on validation for a bit, though.