Opened 6 years ago
Last modified 6 years ago
#17528 new enhancement
[WIP PATCH] Detect issues at intersections
Reported by: | taylor.smock | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core | Version: | |
Keywords: | Cc: |
Description
There are some issues that should be discovered near intersections.
For example, a short disconnect of road continuity at an intersection or a road has a node very close to another.
In the patch, I'm currently using 5.0 meters for the max distance for road continuity and nearby nodes (it ignores nearby nodes with tag information over 1.0 meters away from an intersection).
Attachments (14)
Change History (30)
by , 6 years ago
Attachment: | intersectionissues.patch added |
---|
by , 6 years ago
Attachment: | IntersectionIssues.osm added |
---|
Examples of possible issues found by this patch
by , 6 years ago
Attachment: | intersectionissues.2.patch added |
---|
Add checks to reduce false positives. Now looks for ways that are not flagged under the overlapping ways test and have an angle of less than 30 degrees.
comment:1 by , 6 years ago
I wonder how you can find errors produced by OverlappingWays, this test is executed after yours, isn't it?
comment:2 by , 6 years ago
Short answer: it isn't (currently) finding those errors. The logic is there, I just need to get it to run after the OverlappingWays test. I haven't seen a way to ensure ordering yet. I'm probably missing it somewhere.
I'm currently testing the overall code (it looks like I probably need to change the minimum angle for one of the tests).
by , 6 years ago
Attachment: | intersectionissues.3.patch added |
---|
Implement methods to ensure a test occurs after another test. The code that attempts to prevent duplication of overlapping ways is not currently working.
follow-up: 4 comment:3 by , 6 years ago
Don't know if it is relevant here, but we have a plugin called junctionchecking
. Sounds like something similar to me.
comment:4 by , 6 years ago
Replying to GerdP:
Don't know if it is relevant here, but we have a plugin called
junctionchecking
. Sounds like something similar to me.
Thanks -- I didn't know about that plugin. I'll look into it.
EDIT: It doesn't look like it, but it was crashing on me (see #17534), so it might not be working properly anymore.
by , 6 years ago
Attachment: | intersectionissues_v2.patch added |
---|
Ensure that tests have information on previously run tests
by , 6 years ago
Attachment: | intersectionissues_v3.patch added |
---|
Filter out items that are not relevant (_links, ways that connect to another short section of with the same ref/name)
comment:5 by , 6 years ago
I've tried v3 and I think there is a false positive for the junction at node 2202299509.
There are also a lot of warnings for the ways around this area=yes + highway=pedestrian: way 152259055
Not sure if those are intended? The overlapping ways test seems to exclude area=yes ways.
The "zoom to problem" doesn't help much in this case, it would be better to have the WaySegments as highligted objects.
There is also an issue when ways have different layers, see ways connected to https://www.openstreetmap.org/node/3749781035
Besides that please make sure
- that named constants are written in Upper Case, so maxDistanceNodeInformation should be MAX_DISTANCE_NODE_INFORMATION
- to use Logging() instead of System.out.printf() and e.printStackTrace();
The test is rather slow but that might be tuned later.
by , 6 years ago
Attachment: | intersectionissues_v4.patch added |
---|
Convert constants to upper case, highlight way segments instead of allowing the entire way to be highlighted, attempt to reduce the amount of false positives.
comment:6 by , 6 years ago
I've been trying to figure out how to reduce the false positives. I've taken a look at https://www.openstreetmap.org/node/152259055 and https://www.openstreetmap.org/way/152259055. In the new patch, I don't think either has false positives, but its a work in progress.
It shouldn't be excluding area=yes
ways on purpose, but it does exclude all ways that don't have a ref
or name
tag. I've only ever seen area=yes
on pedestrian ways, and most of those don't have names. I can look into it, but I use the ref
and name
tags to find ways that are relevant to each other.
The printf/e.printstacktrace has been corrected.
Known issues in the current patch:
In the test area (IntersectionIssues.osm), one of the "known bad" intersections isn't found unless it is selected. It is found when it is the only intersection in the test area.
by , 6 years ago
Attachment: | 17528_should_warn.osm added |
---|
comment:7 by , 6 years ago
Please check 17528_should_warn.osm. With v3 I got a (correct) warning, with v4 it is gone.
by , 6 years ago
Attachment: | intersectionissues_v5.patch added |
---|
Stop ignoring nearly overlapping way segments that have lengths longer than 5m and remove false positives where ways share multiple nodes
comment:8 by , 6 years ago
With v5 I see again more false warnings for places where a road splits, e.g. https://www.openstreetmap.org/node/2202299509
In this case maybe the oneway tags should be checked.
For this node https://www.openstreetmap.org/node/426511161 I think it might make sense to warn about something, maybe "sharp angle".
I don't know this place but it is unlikely that cyclist have to make such a sharp turn.
BTW: I've coded something like this test in for the mkgmap project. The programs warns about sharp angles and somehow fixes them because the Garmin routing algo doesn't like them at all. See https://github.com/openstreetmap/mkgmap/blob/master/src/uk/me/parabola/imgfmt/app/net/AngleChecker.java
No idea if it can help here.
by , 6 years ago
Attachment: | intersectionissues_v6.patch added |
---|
Ignore proposed highways, when looking for almost overlapping ways ignore oneways that have the same name/ref, rename the group from Amost overlapping highways
to Sharp angle
comment:9 by , 6 years ago
Somehow the highlighting still doesn't help. 17528_should_warn.osm. I think only the segments which form the sharp angle should be highlighted.
by , 6 years ago
Attachment: | intersectionissues_v7.patch added |
---|
Only highlight the sharp angles of the ways instead of everything touching the intersecting node (it does look much better)
comment:10 by , 6 years ago
Sorry for the late response. I thought I already commented....
It looks better but I still see false positives and I think it would be a good idea to have a unit test similar to MultipolygonTestTest
or RelationCheckerTest
.
comment:11 by , 6 years ago
I still see some false positives and some false negatives as well.
I think I'll see about doing something similar to https://github.com/KaartGroup/highwayNameModificationJOSMPlugin/blob/master/src/com/kaart/highwaynamemodification/GeometryCustom.java in order to get the distance from the node that is almost touching the other way to the other way.
So, TODO list before I work on this particular patch further:
Add additional functions toSee #17616.Geometry.java
to find the distance between different OSM primitives (I think some addons, namely PT Assistant would also benefit) so I can get the distance between the node and the way.Add tests for the above functions
RefactorGPXDistance.java
to use the newGeometry
functions
After that is done, refactor patch to:
- Take into account the distance of the node from the way
- Have fewer false positives
Don't worry about taking forever to comment -- I've been working on other stuff as well, which has given me an idea on how to approach #8082.
by , 6 years ago
Attachment: | intersectionissues_v8.patch added |
---|
Add tests along with some modifications to catch some edge cases -- dependent upon #17616, but can be switched back to using GPXDistance
methods if necessary.
comment:12 by , 6 years ago
I don't see any mention of the UnconnectedWays
test in the discussion. Doesn't this new test produce duplicate results? The descriptions look very similar.
comment:13 by , 6 years ago
It (currently) does duplicate some results. From what I understand, UnconnectedWays
only looks at nodes that are not connected to any other way, while the test that partially duplicates that particular test (currently called "Disconnected ways" in this patch) looks for intersections where the disconnected ways are separated by a (short) section of road.
comment:14 by , 6 years ago
OK. Do you want to work more on it to avoid duplication before we merge it?
comment:15 by , 6 years ago
Yes. There are still some issues I need to spend time working out.
I've been trying to make certain that the latest version is uploaded in case something permanent happens to me, but as you can tell, I haven't worked on the patch much lately. I've been trying to figure out what is going on with one of my plugins (highwayNameModification), and I haven't been able to figure out why it deadlocks for some length of time, errors, shows a dialog box, and then completes what it was deadlocking on.
Current issues:
1) Flags some roundabouts (I've got to investigate this) in the disconnected ways test (the test was supposed to find intersections where you could go straight across but was drawn in such that you couldn't do that). I think this would be an easy fix (I just have to add a check for a roundabout on the start/end nodes).
2) The nearly overlapping ways (currently called "sharp angle") needs to be redone to use the new methods in Geometry so that there are fewer false positives. I want to find roads that were almost overlapping but were not (the initial test just happened to be looking at sharp angles).
3) One of the tests works when the primitives are selected or it is the only issue in the area (it does work if all primitives are selected). I've got to track this down, but it is an intermittent bug that just won't reproduce itself when I'm stepping through with a debugger.
4) The test cases are currently failing, probably due to (3) -- some tests are getting one less than the expected errors except when I run it under a debugger. I should probably move those checks to a different method.
I should probably pull out the disconnected ways test and put it in the "Way end node near other highway" test, since they are very similar. This would allow me to remove the code that allows some ordering of the tests.
comment:16 by , 6 years ago
Summary: | [PATCH] Detect issues at intersections → [WIP PATCH] Detect issues at intersections |
---|
OK thank you :)
by , 6 years ago
Attachment: | intersectionissues_v9.patch added |
---|
Filter out roundabouts for the nearly connected roads check, refactor to reduce method complexities, fix failing tests (I need to use this for awhile to see if there are significant false positives)
Initial patch for finding some intersection issues