hootenanny
hootenanny copied to clipboard
Ref conflate doesn't handle situations where secondary road data has footway=crossing sharing nodes with roads
Found while working on #3895. Due to the crossing sharing a node with the road. I have no idea yet how to fix this. The crossings get snapped to the new intersection of the merge roads. See test-files/cases/reference/unifying/highway/sidewalk-3895
Have found other work in the meantime before starting this that I think is more important.
Did some cleanup of the railway schema, similar to what was done last week with the waterway schema, which should help runtime performance out.
Seeing some painfully slow processing with this data and collection relation matching, primarily for very large route and waterway relations...EdgeDistanceExtractor
is just too slow for them. Going to try to find a better performing extractor to use on large relations of those types.
Back on this.. Fortunately, there's already a test from #3895 with an example of this. I'm also noticing that secondary highway=crossing
nodes are unsnapped in the output, despite the fact that #3922 should have fixed that.
The orphaning of the crossing node actually happens before WayNodeCopier
from #3922 kicks in during the snapping of the ends. Seeing if I can preserve it in that function.
Fixed the crossing node issue with a HighwaySnapMerger
node copying during end snapping and an update to the SuperfluousNodeRemover
unallowed orphans list. lots of test failures, though...
Many of the test failures have been due to improvements so far and the output has been updated. A couple of the tests now have some strangely bad output that I'm looking at.
Actually, the bad output was pre-existing and looks to be caused by the collection relation merging. Very strangely, railway crossing nodes are added to some buildings and other roads. Some ids must be getting mixed up somewhere.
I think the bad output is caused by railway conflation. Still trying to verify. Even though I fixed the original problem with footways that I set out to fix, I'm going to go ahead and merge the snapped node tag fixes and then come back to these other two problems.
The bad output ended up being caused by WayNodeCopier
allowing copying of nodes from relations, which it shouldn't. Correcting that fixes the problem. Will merge changes so far and circle back around to the footway problem laster.
Actually have one POI regression test failing as a result of this...should have waited a little longer to merge. Number one suspect would be the changes to SuperflousNodeRemover
but haven't verified that yet. Since some of that regression testing input data gets artificially filtered down to POIs by custom scripts (orphaned POI nodes created that wouldn't ever exist in normal OSM), may have to add an option for this test only to not remove such nodes in the output.
Was able to fix the test by disabling the orphaned node removal by kvp for it only.
Worth noting that the Network alg doesn't do the bad footway snapping but it also fails to connect a portion of sidewalk at the intersection...so still worth fixing this in Unifying I think. I may be able to add in some custom handling to disconnect the node being snapped on the residential road to prevent the bad footway snapping. The problem is that the residential intersection will get the crossing node based on recent secondary info node retention changes when it really shouldn't. Need to think more about how to prevent that from happening.
Going to have to bail on this one for awhile. I came up with a partially working solution pushed to the 3901 branch but just can't come up with anything completely solid. Will leave the issue open for awhile in case I get any ideas in the future. While it would have been nice to finish this, for this particular data, the secondary is wholly superior to the reference, so probably a replacement would be done in this situation anyway.