Namer for applying street names to nearby sidewalks

[leonardehrenfried]

Summary

Introduces a new implementation of EdgeNamer that applies the names of nearby streets to sidewalks. The actual algorithm is described in the Javadoc.

What does it look like?

Before

After

I've also included smaller clean ups and improvements to the (new) debug UI.

Unit tests

Added.

Documentation

Lots of Javadoc.

Bumping the serialization version id

The graph itself is not changed but the build config is. I'm not sure if this requires a bump.

cc @binh-dam-ibigroup

[codecov]

Codecov Report

Attention: Patch coverage is 72.09302% with 60 lines in your changes are missing coverage. Please review.

Project coverage is 67.93%. Comparing base (f4f1e39) to head (60c4ee9).
Report is 26 commits behind head on dev-2.x.

Files	Patch %	Lines
...builder/module/osm/naming/PortlandCustomNamer.java	0.00%	23 Missing ⚠️
...graph_builder/module/osm/naming/SidewalkNamer.java	83.96%	13 Missing and 4 partials ⚠️
...nner/inspector/vector/edge/EdgePropertyMapper.java	0.00%	9 Missing ⚠️
...anner/graph_builder/module/osm/StreetEdgePair.java	38.46%	5 Missing and 3 partials ⚠️
.../graph_builder/module/osm/WalkableAreaBuilder.java	90.00%	0 Missing and 1 partial ⚠️
...pplanner/graph_builder/services/osm/EdgeNamer.java	88.88%	1 Missing ⚠️
...entripplanner/openstreetmap/model/OSMWithTags.java	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5774      +/-   ##
=============================================
+ Coverage      67.90%   67.93%   +0.02%     
- Complexity     16556    16590      +34     
=============================================
  Files           1910     1912       +2     
  Lines          72436    72601     +165     
  Branches        7447     7449       +2     
=============================================
+ Hits           49190    49323     +133     
- Misses         20724    20750      +26     
- Partials        2522     2528       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[abyrd]

After today's meeting discussion, I looked through SphericalDistanceLibrary in more detail. I see the equirectangularProject and fastDistance methods use the same approach I described, but are expressed differently: they leave all the coordinates in radians (basically, on a globe with a radius of 1 unit) and then the various methods all have to multiply their final results by the radius of the Earth as a final step to get usable results in meters. I can see why this would be confusing to approach, and this also complicates the reusability of the projected geometries with straightforward JTS methods. I don't think there's any loss of efficiency to just project everything straight into meters. In fact it should be both slightly more efficient and easier to read.

I will propose a refactored version of these library functions that I expect will be much easier to work with, adapting the sidewalk namer to use the refactored library functions.

[abyrd]

Thanks for all the revisions.

Unfortunately during yesterday's meeting I hadn't yet seen certain changes, particularly the removal of DistanceOp. I was thinking of the state I saw at the time of this comment: #5774 (comment)

I see that DistanceOp is no longer being used, and that was the one place I thought distances were still being calculated on lon-lat coordinates. The current approach looks sound: buffering is performed in UTM projected coordinates with cached transforms; intersection operations are in WGS84 but between areas and linestrings so not projection-sensitive; subsequent relative length calculations are performed in WGS84 but using great-circle distances, which are more precise than needed but certainly correct.

The mix of coordinate systems and distance calculation approaches remains a little unusual, but it looks correct and well-documented, and you measured it to be reasonably performant.

For the future, if I want to emphasize certain ideas for consistency and long-term maintainability, the ball is in my court to write documentation and maybe new utility methods. I realize none of the historically accumulated geographic/geometric calculation ideas have been summarized for reuse, that was basically undocumented institutional knowledge at Conveyal.

[abyrd]

I will propose a refactored version of these library functions that I expect will be much easier to work with, adapting the sidewalk namer to use the refactored library functions.

I drafted some ideas for this, but seeing that DistanceOp is no longer used the changes are not necessary for this PR and are more a matter of style/taste than correctness. So I won't be proposing changes to this PR, but possible a separate documentation/refactor PR on SphericalDistanceLibrary.

[leonardehrenfried]

Having a reusable component that converts WGS84 geometries to "equirectangular" ones would indeed be great. I think in such a case we should be converting to a type that is a facade to the actual JTS geometry (credit to @t2gran) and therefore documents what kind of coordinate system we are dealing with. For example, the result of the distanceTo() method could be of type Distance. It should also prevent comparing a WGS84 geometry to a equirectangular one.

[leonardehrenfried]

Would it be possible to use the spatial index with such an equirectangular geometry?

[binh-dam-ibigroup]

Just a few minor formatting things, but otherwise it is pretty solid!

[binh-dam-ibigroup]

For future work in a subsequent PR: This would not accommodate unnamed slip lanes (see screenshot and OSM link), and the adjacent sidewalk would keep its default name.

See https://www.openstreetmap.org/way/1201583893 vs. https://www.openstreetmap.org/way/784160579

[leonardehrenfried]

Well, we already filter out nameless edges before adding candidates to the index. Is the sidewalk in question nameless?

[binh-dam-ibigroup]

Yes.

[abyrd]

Final changes look good. We agreed in the development meeting that although it would be nice to have our own concise, fast, typesafe geometry code that's beyond the scope of this PR. This is certainly clean and correct enough to merge as-is.