a-b-street / a-b-street/osm2streets
Simplify the early stages of creating a `StreetNetwork` from OSM data
- 主要言語
- Jupyter Notebook
- スター
- 155
- フォーク
- 14
- PR マージ指標
- 30日以内にマージされた PR はありません
説明
The start of main `osm_to_street_network` function is a complicated tangle of stateful modifications of the `StreetNetwork` with plenty of comments hinting at dependencies between seemingly unrelated steps. The delegation of responsibilities and relationship between `StreetNetwork::blank`, `extract_osm`, `split_up_roads` and `clip_map` is not clear, with mutations happening to the `StreetNetwork` all over the place.
https://github.com/a-b-street/osm2streets/blob/3d64f99d400b654367f40d86bee8678ade037e81/streets_reader/src/lib.rs#L102-L115
I think we should improve the public APIs of `StreetNetwork` and `OsmExtract` so that they help explain the ordering constraints and delegation of responsibilities. I'll share my ideas about that here, which requires a diversion into clipping and the `StreetNetwork` to OSM mapping.
## Clipping the Map
Clipping is an interesting problem because the interpretation of the map features is influenced by nearby features, that's the whole reason osm2streets needs to exist. I think that `StreetNetwork` ought to treat its features fundamentally as areas, not centerpoints and centerlines. (That's why I'm advocating for geometry calculations to move up earlier in the process.) If that is the case, then precisely clipping a `StreetNetwork` has to involve cutting `Road`s and `Intersection`s in half. This is not something that we should support at the representation level, because the representation still needs to use points and centerlines as reference for the distinct features (intersections and roads) that make up the areas.
In fact, it is an unavoidable problem that we can't accurately represent anything that is close to the edge of the raw data we have. We should acknowledge that fact and build it into our understanding and the API. There should be a buffer border of some distance around the area that we have data for that we consider to be nonsense output. Probably about half the width of a very wide road.
When doing the OSM to Streets conversion, I think we should trim the raw OSM to the boundary polygon in `OsmExtract`, before adding it into `StreetNetwork`. The trimming operation is better defined in the domain of the OSM data itself, because the OSM data is points and lines. There should be no clipping operation done on `StreetsNetwork` representation data, instead we could clip the end-result geometry precisely to the inside edge of the buffer boundary, or output that boundary and let the consumer do it.
## Mapping Between `StreetNetwork` and OSM
A major usecase for `StreetNetwork` is empowering tools for authoring OSM data, so it will be useful to be able to get from some `StreetNetwork` feature back to OSM feature that defines it. From our recent discussions, it seems like it will be useful for transformations to have access back to the defining OSM too. That way they can use whatever weird properties of the OSM data they want, without us having to represent it first class within the `StreetNetwork` features.
The takeaway here is that `StreetNetwork` should store a full copy of the original OSM data (at least until the user wants to discard it) and maintain a mapping. Doing so makes it easier to reason about adding in additional details at later steps, like `use_barrier_nodes` and `use_crossing_nodes` does currently.
## The Refactor
Ok, enough waffling about motivations, my proposal is to use private methods and "constructor" methods to delineate the responsibilities, and use public methods to communicate what is safe to do without worrying about internal data dependencies.
1. Move the concept of a clip boundary into `OsmExtract`, with the invariant that it only stores data within the boundary. Don't be scared about discarding data just outside boundary, we already have no idea what features we can't see beyond the bounds of the raw data we do have, so lets be clear about our cut-off point.
2. Implement `OsmExtract::new(doc: osm::Document, boundary: Option>) -> Self` which doesn't depend on anything `StreetNetwork`. It can clip the raw OSM features as it reads them from `doc`.
3. Implement `StreetNetwork::from_osm(osm: OsmExtract, config: MapConfig) -> Self` which does `split_up_roads` internally, and whatever else is necessary to return a `StreetNetwork` that is ready to have its public API called.
4. Make `sort_roads` and `update_movements` private again, and work towards all transformations doing their work using the public `StreetNetwork` API, instead of direct mutation.
Because `StreetNetwork::insert_road` exists the implication is that it handles some internal data dependency so the caller doesn't have to. `split_up_roads` should be private to `StreetNetwork` because it wants to be more efficient than using `insert_road` and is willing to understand the data dependency in order to do it safely. Transformations should use the public API because they should be non-vital and safe. The only danger of skipping them or calling them in the wrong order should be the missed opportunity for an easy improvement.
The idea is that an interactive editor could use `StreetNetwork::new(bounds: GPSBounds, config: MapConfig)` to create a blank map for the user, and use `insert_intersection` and `insert_road` to build up a map based on user input. Then transformations would be buttons available to "tidy up" the map. They can be clicked in any order as many times as the user likes.
コントリビューションガイド
評価
この issue はまだ評価されていません。