Cutover to osm2lanes #890

dabreegster · 2022-04-13T21:36:31Z

This is far from ready, but figured it'd be better to track progress here instead of a-b-street/osm2lanes#71. My workflow right now (which assumes the dev env is setup) is:

# Reimport a sample map
./import.sh --raw --map montlake
# Load the UI in a mode where ctrl+tab switches between old/new version of the map
./data/diff_changed_map.sh data/system/us/seattle/maps/montlake.bin

Lane width is the main difference that jumps out. Even a service=alley looks super wide. I'll start looking at ways of specifying different values in Locale.

CC @droogmic

droogmic · 2022-04-14T05:10:56Z

Thanks, I see the issue now. I need to look for some national road building standards to get a good defaults for these.

dabreegster

@droogmic, I decided to try finding some way to cutover to osm2lanes and patch around any "breaking" changes from A/B Street's perspective, so we could iterate faster. A few workarounds are here now, making my test Seattle map look close enough.

But I tried a map in the UK and hit a few new issues, which I'll work through next. If any of these merit issues or test cases in osm2lanes, I'll send a PR.

transform/tags_to_lanes/modes/non_motorized.rs retains A/B Street's hack of treating shared walking+biking spaces as a [shoulder, cycle lane, cycle lane, shoulder]. But https://www.openstreetmap.org/way/139974130 became just a sidewalk, because bicycle=dismount. I think osm2lanes is correct here; I just need to audit some of my maps to see why I added the dismount case originally. I was probably trying to make the cycling graph somewhere in Seattle be connected for an experiment which is hopefully not needed anymore.
Lots of ambiguous busways in practice, like https://www.openstreetmap.org/way/1027418221. I might try and add a hack around this to force some result. Until we have tooling to help mappers detect and fix these problems in bulk, it's not super reasonable to expect people to fix them everywhere. Or... I might just fix myself in the handful of maps I care about particularly now for work projects.
Tags({"abst:endpt_back": "true", "abst:endpt_fwd": "true", "abst:osm_way_id": "1004419293", "abst:parking_inferred": "true", "abst:sidewalks_inferred": "true", "highway": "tertiary", "name": "Hillside Road", "parking:lane:both": "no_parking", "sidewalk": "both", "sidewalk:right": "yes", "sidewalk:right:surface": "asphalt"}): unsupported: -- The error doesn't say what's unsupported? I'll add some osm2lanes test cases to figure out what's up

dabreegster · 2022-04-14T07:56:34Z

raw_map/src/lane_specs.rs

+            // Workaround common incorrect tagging
+            tags.checked_insert("sidewalk", "no").unwrap();
+        } else if k == "sidewalk" && v == "separate" && cfg.inferred_sidewalks {
+            // Make blind guesses


sidewalk=separate is unsupported (because it's something osm2lanes alone can't really handle...), so to match existing behavior, I'm doing some rewriting here

dabreegster · 2022-04-14T07:59:38Z

raw_map/src/lane_specs.rs

+                .flatten()
+                .collect::<Vec<_>>();
+
+            // No shoulders on unwalkable roads


I spotted shoulders on motorways. That's intentional, right? The definition of shoulder is fuzzy, I guess. If we mean it as a place for vehicles to pull over in an emergency, then it's reasonable to assume they exist on motorways. A/B Street currently treats shoulders as a space where technically pedestrians can walk, but it's dangerous / unpleasant to. In the US, it's generally illegal / extremely inadvisable to walk on the shoulder of most interstates. So I'm doing this post-processing here, to make sure pedestrians in the simulation don't find some clever shortcuts

dabreegster · 2022-04-14T08:00:51Z

raw_map/src/lane_specs.rs

-                if part == "designated" {
-                    back_side[idx].lt = LaneType::Bus;
-                }
+            // Use our own widths for the moment


I'd like to cutover ASAP and iterate on smaller issues. I realized the width issue is easy to workaround -- just override with the current A/B Street config. We can remove this later.

dabreegster · 2022-04-18T15:54:34Z

I made some more progress, continuing the strategy of fudging the input and output of osm2lanes to match current behavior a bit better. Some of the changes:

Found a bug in prior abst code where bus lanes on most roads had the wrong width value!
Some roads lose shoulders on one side, which is closer to reality. I'll just need to check carefully in places like North Seattle; shoulders were added previously to make sure the walking graph is reasonable
highway=footway, bicycle=dismount becomes just a walkable lane. I'm happy to "lose" a few of those previous bike trails; they were always a hack.
Patching around living streets, oneway=reversible, few other things
Bidirectional cycletracks are transformed into two separate lanes; the simulation layer further down the chain doesn't understand bidi lanes except for sidewalks
Attempting to hack around multiple bus schemas. Some attempts in Bristol, a map I need to be very careful about right now, have confusing results

I kind of want to wrap this up quickly instead of leaving it hanging, but it's really taking time to crunch through all the issues. :\

droogmic · 2022-04-18T16:22:38Z

| I kind of want to wrap this up quickly instead of leaving it hanging, but it's really taking time to crunch through all the issues. :\

I think osm2lanes is still a little bit immature to avoid having to deal with lots of issues.

dabreegster · 2022-04-25T17:27:05Z

I rebased against A/B Street changes and also updated osm2lanes. I left off looking at changes in Bristol particularly, as I have a work project centered there. ./import.sh --raw --map --city=gb/bristol
Main problem was... oh hey, it's gone, Bristol's actually looking better with osm2lanes now! Bus lanes at https://www.openstreetmap.org/way/987715676 showing up now.

droogmic · 2022-04-26T16:40:49Z

any opinion on what the next most problematic thing is for you?
cyclist=dismount, more bus stuff, or road widths?

dabreegster · 2022-04-26T16:48:14Z

Probably road widths.

But based on my quick diff yesterday, I bet we could do the cutover soon. I'll try to dedicate a few hours tomorrow to trying it and at least seeing what other problems come up. Once we get past this initial hump, subsequent improvements (removing overrides on the A/B Street side and trusting osm2lanes I/O more directly) can be done gradually. And osm2lanes can keep evolving at whatever speed; I'll re-import everything and repeat validation every so often.

dabreegster · 2022-04-27T15:38:12Z

https://www.openstreetmap.org/way/22772836
cycleway=opposite without oneway=yes oneway:bicycle=no

This is the most common error affecting a hi-pri map. I'll probably hack around it for the moment...

dabreegster · 2022-04-27T16:07:45Z

Ignore above.

@droogmic, I think I'm getting close -- I'm checking some last things, then I'll attempt a full reimport. Once I get that to a point where I'm happy, I'd prefer to upload the changed data and merge quickly to avoid hassle in my other work. (Branching code is easy, but not for data.) Are you interested in reviewing the code here? I don't expect it to be particularly understandable without lots of context.

Once this first cutover is done, iterating on smaller fixes will be much easier on my end, though I'll still probably batch the full reimports and validation on my end. Cleanup like:

removing is_road
doing less transformation of tags
doing less transformation of osm2lanes output (like trusting widths)
cleaning up all the places that insert an inferred sidewalk or parking tag (a weird old technical decision that's no longer valid and is mostly just confusing)

droogmic · 2022-04-27T17:48:01Z

Are you interested in reviewing the code here?

not particularly, unless you either, want me to understand it generally, understand the workarounds you are still doing, or want me to find defects

dabreegster · 2022-04-27T20:44:27Z

not particularly, unless you either, want me to understand it generally, understand the workarounds you are still doing, or want me to find defects

Alright, feel free to ignore this PR then.

I imported everything and managed to provoke a few crashes. I'll track down where when I get some time and file + fix bugs:

somewhere in london, newcastle, auckland: "capacity overflow" in osm2lanes::transform::tags_to_lanes::road::RoadBuilder::from -- I suspect something's trying to allocate a huge vector. If I had to blindly guess, there's a "-1" we're parsing as a usize
somewhere in seatle: unexpected speed 3025 mph', /home/dabreegster/.cargo/git/checkouts/osm2lanes-0a9401f46878d04a/df0e7bb/osm2lanes/src/transform/tags_to_lanes/separator/semantic.rs:27:18

droogmic · 2022-04-27T20:52:33Z

Is it hard for you to find the way ID associated with the failures?

droogmic · 2022-04-27T20:56:28Z

raw_map/src/lane_specs.rs

-                fwd_side.push(fwd(LaneType::Biking));
-            }
+        Err(err) => {
+            error!("osm2lanes broke on {:?}: {}", orig_tags, err);


I guess the tracing we are missing here is way_id

It's awkwardly embedded in the list of tags as abst:way_id. I'll drag it out and print a URL to make for faster debugging.

There were straight-up panics though, I'm adding some debugging to get examples

droogmic · 2022-05-01T20:46:26Z

convert_osm/src/extract.rs

+    // Hack for Geneva, which maps sidewalks as separate ways
+    if !tags.contains_key(osm::SIDEWALK)
+        && tags.is("oneway", "yes")
+        && name.city == CityName::new("ch", "geneva")
+    {
+        tags.insert(osm::SIDEWALK, "both");


I guess I should get some test cases from Geneva ;)

@ThomasDagier uncovered some gnarly bus tagging there, but otherwise, nothing too special here. Hacking around sidewalks is unfortunately common in this codebase.

dabreegster force-pushed the osm2lanes branch from 4f575d2 to 0bd7daf Compare April 14, 2022 07:55

dabreegster commented Apr 14, 2022

View reviewed changes

dabreegster force-pushed the osm2lanes branch from 0bd7daf to 7db9dbb Compare April 18, 2022 15:35

dabreegster force-pushed the osm2lanes branch from 7db9dbb to c232b84 Compare April 25, 2022 17:21

dabreegster force-pushed the osm2lanes branch from c232b84 to c2e8d77 Compare April 27, 2022 15:32

dabreegster force-pushed the osm2lanes branch from ed3b087 to 8a9893f Compare April 27, 2022 16:04

droogmic reviewed Apr 27, 2022

View reviewed changes

dabreegster force-pushed the osm2lanes branch 3 times, most recently from 5786339 to 27f067e Compare May 1, 2022 20:28

droogmic reviewed May 1, 2022

View reviewed changes

dabreegster force-pushed the osm2lanes branch from 27f067e to f46b7fd Compare May 4, 2022 09:07

dabreegster force-pushed the osm2lanes branch from f46b7fd to 5907a8e Compare May 22, 2022 16:11

Initially cutover to osm2lanes

f0f13bc

dabreegster force-pushed the osm2lanes branch from 5907a8e to f0f13bc Compare July 5, 2022 14:09

dabreegster mentioned this pull request Sep 4, 2022

Use osm2lanes, guarded by experiment flag a-b-street/osm2streets#85

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cutover to osm2lanes #890

Cutover to osm2lanes #890

dabreegster commented Apr 13, 2022 •

edited

Loading

droogmic commented Apr 14, 2022

dabreegster left a comment

dabreegster Apr 14, 2022

dabreegster Apr 14, 2022

dabreegster Apr 14, 2022

dabreegster commented Apr 18, 2022

droogmic commented Apr 18, 2022

dabreegster commented Apr 25, 2022

droogmic commented Apr 26, 2022

dabreegster commented Apr 26, 2022

dabreegster commented Apr 27, 2022

dabreegster commented Apr 27, 2022

droogmic commented Apr 27, 2022

dabreegster commented Apr 27, 2022

droogmic commented Apr 27, 2022

droogmic Apr 27, 2022

dabreegster Apr 27, 2022

droogmic May 1, 2022

dabreegster May 2, 2022

Cutover to osm2lanes #890

Are you sure you want to change the base?

Cutover to osm2lanes #890

Conversation

dabreegster commented Apr 13, 2022 • edited Loading

droogmic commented Apr 14, 2022

dabreegster left a comment

Choose a reason for hiding this comment

dabreegster Apr 14, 2022

Choose a reason for hiding this comment

dabreegster Apr 14, 2022

Choose a reason for hiding this comment

dabreegster Apr 14, 2022

Choose a reason for hiding this comment

dabreegster commented Apr 18, 2022

droogmic commented Apr 18, 2022

dabreegster commented Apr 25, 2022

droogmic commented Apr 26, 2022

dabreegster commented Apr 26, 2022

dabreegster commented Apr 27, 2022

dabreegster commented Apr 27, 2022

droogmic commented Apr 27, 2022

dabreegster commented Apr 27, 2022

droogmic commented Apr 27, 2022

droogmic Apr 27, 2022

Choose a reason for hiding this comment

dabreegster Apr 27, 2022

Choose a reason for hiding this comment

droogmic May 1, 2022

Choose a reason for hiding this comment

dabreegster May 2, 2022

Choose a reason for hiding this comment

dabreegster commented Apr 13, 2022 •

edited

Loading