Improve "perimeter" classification

[michaelkirk]

IMPROVES #67 (There are still some known issues, but I think we should start with this and see how much of a priority the remaining issues are / gather more test cases).

In LTNv1, neighborhoods were assembled by tracing an outline of roads. Apparently there were some problems with this - in particular around dealing with bridges/tunnels into a neighborhood.

So in LTNv2, we now have the user draw a polygon, and use topology checks against that polygon to determine if a road is interior/exterior/crossing.

Firstly, checking if a road is topologically inside, outside, or crossing a boundary isn't sufficient for determining when it's a "perimeter" road. A better metric discussed in #67 for defining perimeter was "if a road is parallel and near the boundary" — I've attempted to implement that metric in this PR.

Another big wrinkle: We were using Relate::is_within/is_intersects topology to check if a road is Interior/Exterior/Perimeter. However, due to floating point rounding, there are some perimeter roads which are partially/mostly/entirely outside of the neighborhood (albeit by only a very small distance). To address this I'm checking the perimeter relationship first. And only if that fails, checking on topology. So now, so long as a segment is near and parallel to the boundary, it will be considered a Perimeter - regardless of whether it is inside or outside the boundary.

Overall, it seems to give better results, but it is (mostly) a slower implementation (though not drastically so):

build neighbourhood: bristol_east
                        time:   [8.5696 ms 8.5968 ms 8.6279 ms]
                        change: [+20.926% +21.448% +21.956%] (p = 0.00 < 0.05)
build neighbourhood: bristol_west
                        time:   [12.717 ms 12.775 ms 12.844 ms]
                        change: [+6.8103% +7.9397% +9.0261%] (p = 0.00 < 0.05)
build neighbourhood: strasbourg
                        time:   [11.598 ms 11.925 ms 12.276 ms]
                        change: [-12.421% -8.9352% -5.1781%] (p = 0.00 < 0.05)

Note: I've removed the Crosses { ratio } enum variant and added a Perimeter. If there's a reason to keep Crosses { ratio } around at run time, I can restore it and we can have { Interior, Exterior, Perimeter, Crosses }

Some other avenues I explored but backed out of:

1. Do Relate topology checks to quickly filter out "external" roads, but use a buffered boundary polygon (via the geo_buffer crate) to be sure any nearby perimeter roads are within the boundary. This actually ended up being a bit slower, primarily because buffering the neighborhood polygon is expensive.

2. Use i_overlay for linestring trimming in line_in_polygon: This ended up being slower than your existing impl in clip_linestring_to_polygon. I suspect it might be more robust to use i_overlay, but without seeing an issue I'm hesitant to slow this down further.

The remaining case I know of that we don't handle well is when a substantial portion of the road crosses out of the boundary. The portion inside the boundary should be classified as Interior or Perimeter, but we don't have a good metric for it. I naively tried to split these Roads, but some of them exit and re-enter the polygon, which I believe should be thought of as two separate roads, but at this point all the RoadIds have already been assigned. Addressing that seemed like a bigger change, so I thought I'd start with a simpler metric and see how much of a problem it is in practice.

One way of addressing this would look like splitting all the roads by the neighborhood boundaries up front, before assigning RoadIDs, but I suspect that might be complicated because currently RoadId's are assigned before any neighborhoods are created (kind of guessing, but I think it's true?), so I'm going to leave it here for now. Let me know what you think?

[dabreegster]

Awesome! I'll run locally and look at some example cases, but I maybe do need your local geo to be pushed to a GH branch somewhere first.

One way of addressing this would look like splitting all the roads by the neighborhood boundaries up front, before assigning RoadIDs, but I suspect that might be complicated because currently RoadId's are assigned before any neighborhoods are created (kind of guessing, but I think it's true?), so I'm going to leave it here for now. Let me know what you think?

The user can draw neighbourhood boundaries at any time in the workflow right now, and even change existing ones later. Also, two boundaries are allowed to overlap right now. (Need to discuss any requirements upstream for preventing this.) We could maybe just cache per session the resulting Neighbourhood after a user opens it, but it might not be worth the effort, depending how many people tend to use in one session.

[dabreegster]

Style question: Any preference not to lift this to the top of this module?

[michaelkirk]

Done!

[michaelkirk]

Ok, I think I've addressed all your concerns (and rebased). Please take another lookg @dabreegster

https://github.com/a-b-street/ltn/pull/94/files/98bdaa92aa562d8eb18da420fd260736a67b12d5..334f94cd6cd336fc8c753670c313d23b0b6152f9 should be a diff since your last review.

[dabreegster]

Fantastic, thanks so much for the PR! I just spot-checked a few random places, and the perimeter road detection seems great everywhere, and the slight perf degradation isn't noticeable. Really really awesome work