osm2streets cutover

[dabreegster]

#71, about cutting A/B Street over to using osm2lanes, kind of died off. I wanted to start fresh here based on new goals and problems. Since the last update there, we successfully detangled the overall OSM import and geometry logic out of A/B Street into https://github.com/a-b-street/osm2streets. It has a standalone web UI, its own unit tests, etc and is no longer tied to A/B Street. A/B Street directly depends on it. That's the same thing we want to achieve with osm2lanes, but now the integration is a bit more focused -- osm2streets will depend on osm2lanes. A/B Street will depend on osm2streets, and osm2lanes is an implementation detail one layer in.

Goal

osm2streets is still using the original lane_specs parsing code. In the short-term (next few weeks), I want to start adding a lane type for shared walking/cycling paths, fix bugs like this and many of the others filed in this repo, and handle lanes with explicitly tagged widths. I do not want to invest further in the monolithic lane_specs.rs; I want to cutover to osm2lanes.

Problems

When I've tried to jump in and work on osm2lanes for some of these things, it's felt like the codebase is very complex, even though I've been reviewing all the changes.

Error handling / permissiveness

cargo run way 260912073 fails outright because sidewalk=none is deprecated. I think we need to rethink how strict we are, maybe using something like taginfo popularity to guide decisions. Any pushback against treating this as a warning but otherwise proceeding?

Alternatively, there can be an optional layer in osm2lanes that preprocesses tags and corrects common mistakes. Some examples in https://github.com/a-b-street/abstreet/blob/f0f13bc50aac28cf0f41d960602258ddfb61a566/raw_map/src/lane_specs.rs#L135 -- not the ones involving directions and such, or sidewalk inference.

Adding a test

osm2streets progress has happened quickly because of being able to iterate very quickly with tests. The table-driven tests in the original code were very quick to crank out:

            (
                "https://www.openstreetmap.org/way/49207928",
                vec!["cycleway:right=lane", "sidewalk=both"],
                DrivingSide::Left,
                "sddbs",
                "^^vvv",
            ),

The last time I attempted to add an osm2lanes test, I was a bit overwhelmed by having to figure out the locale and write out the full JSON (with separators or not?). Some ideas for making this experience easier:

1. Add something to the web app to export as a copy-pasteable test case template
2. Maybe make a tiny tool to transform the "sddb" + "^^vv" notation into full JSON, and also use that to copy into the yaml. It's easier to type.
3. Maybe split the test yaml by how much we want to assert. Most test cases don't need to specify separators or width in the output. We can start a different file where we include those in the expected output and enforce that.

If the drag-and-drop UI worked, we could also use that to quickly specify the expectation and copy it into the YAML.

Matching up lanes

osm2streets still uses the old LaneType enum. It'd be nice to use osm2lanes' representation directly at some point, but this is out of scope for now. One major change that has to happen in osm2streets and beyond is properly modelling Direction = Both or Direction = None. Until then, bidi lanes will get split into two directional lanes.

Some of that logic is https://github.com/a-b-street/abstreet/blob/f0f13bc50aac28cf0f41d960602258ddfb61a566/raw_map/src/lane_specs.rs#L238. Since it's non-trivial, I think it makes sense for osm2streets unit tests to also include the translated lane config in the expectation. I will probably add that as a preliminary step.

Rough plan

Because of the way work is going right now, I get the most done when I quickly crank out code in a few days. What I would work on is:

• fixing the above test issues, making it much easier to quickly iterate
• getting parity with the current implementation. Maybe that means turning errors into warnings, or adding an optional layer to "fix" common tagging issues that happen in the real world.
• a cutover
• then fixing reported bugs and starting on things like lane widths

Any feedback, @droogmic and @BudgieInWA? When I go rapid-fire mode and start this, do you want me to wait for reviews on PRs, or would y'all be OK leaving feedback whenever is convenient and I'll address it later? (If the former, I'll probably send bigger PRs to reduce communication latency)

[BudgieInWA]

As far as deprecated tags go, I think we should aim to support incorrect tags that are easy to support, like sidewalk=none. This need for preprocessing to normalise tags is one of the biggest benefits of the schemes we've been implementing, I think. If the raw tags are only used to generate schemes, then it becomes much easier to support a variety of tags for the same meaning.

Providing PR feedback after merge works fine for me.

[dabreegster]

A few PRs are out to simplify the test writing experience. I still find https://github.com/a-b-street/osm2lanes/blob/main/data/tests.yml and https://github.com/a-b-street/osm2lanes/blob/main/osm2lanes/src/test.rs confusing.

• How much of the expected output do we assert? There's custom logic to filter out separator lanes or not, expect warnings, don't care about things like road name/ref if they're not explicitly listed in the expectation, etc.
• Where do I add a new test? Right now I search for ### and browse through each other, then give up and just append to the end in the huge "To sort" pile!
• I can't easily filter for just one test case when running. Should we programatically generate test cases?

Nitpicks:

• The YAML format is picky about indention, and the current attempt to generate test YAML for copying doesn't exactly get things right. prettier doesn't always fix all the problems.
• We're inconsistent about the use of quotes in the yaml, and I haven't found a tool yet (prettier, yamlfix, yamlfixer) that consistently does something.

My only idea right now is to split the file into multiple, by the current category -- rural, mutli-lane trunk, mis-tagged, pedestrian, sidewalk, cycleways, bus lanes, combination, lifecycle, unsorted. I can list these files at a glance and more quickly pick one. It'll be a little more code to read data from all the files, but not much.

I also thought about splitting the files by how much detail they check -- aka, lane separators or not.