-
Notifications
You must be signed in to change notification settings - Fork 15
"Impossible angles" comparator resulting in false positives #213
Comments
I think this is an interesting issue, with a fundamental issue being that within the current architecture, each "comparator" only receives the old and new version of the single feature modified / being checked. As you point out @vgeorge, for many comparators, it is useful to have some more "surrounding context" to be able to meaningfully make an assertion about the change. I know this is something that has come up / been discussed before, and any solution is potentially a bit tricky / has implications on the performance of the entire pipeline, since these comparators need to be run for every single edit in OSM, and adding any extra work / async activity in them must be measured very carefully. Currently, the approach that has been favoured has been a pre-processing step where the data that is sent to the comparators is "augmented" with additional context data that maybe useful in the comparison - for eg. - additional metadata about the user that made the edit. It does seem like it would be extremely useful to be able to add a property like This is a hard problem though :-) - I think the first step might be having an extremely responsive API that one could query to get all ways a node is a part of, and then hook that up to a pre-processing pipeline to lookup nodes against before the change data gets passed to In general, I think it might be nice to revisit the discussion of "additional context" that can be passed to the comparators - one starts hitting a serious wall on the kinds of assertions one can make about a change without having access to map data of the area immediately surrounding the change. One would ideally probably like to have access to perhaps the GeoJSON of the entire z15 (or so) tile that the feature being edited is on. Any attempt to do this, though, will have many infrastructure implications / would have to be thought through carefully, balancing practicality with the need to get additional data -- perhaps fetching this additional data only for a subset of edits, etc. This is a super interesting problem though, and I hope we can find a way to collectively think through / work on this a bit as being able to have some of this additional context within a compare function would drastically increase the usefulness of the assertions one can make about certain kinds of edits. |
Sanjay, thanks for the explanation. In this specific comparator ("impossible angles"), I think it might be more efficient to query context when an error was raised, because it is only needed when there is an impossible angle already identified. Otherwise, context is not needed. By the way, I'm looking again at the changeset that raised this issue and it seems that the connecting way is included on it:
Correct me if I'm wrong, but the connecting way is part of the changeset, so this issue could be resolved within the comparator without the need of context. |
I've been watching an area in OSMCha and received false positives of "impossible angles" comparator. Here is an example:
https://osmcha.mapbox.com/changesets/66565198
As @willemarcel stated at OSMCha/osmcha-frontend#168, the comparator do not consider the connecting way and returns the warning.
My suggestion here is to check if the vertex node is part of another way when a narrow angle is detected, and do not return the warning if that is the case.
The text was updated successfully, but these errors were encountered: