Modify

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#17914 closed defect (fixed)

End node near other way test does not work if highways are connected with another node

Reported by: adiatmad Owned by: team
Priority: normal Milestone: 20.05
Component: Core validator Version:
Keywords: template_report unconnected highway end node near other Cc:

Description (last modified by adiatmad)

What steps will reproduce the problem?

  1. I download my interest area.
  2. Check visually and found this case.
  3. Check using validator and not detected as warning

What is the expected result?

It should appear warning on JOSM because the road is not connecting well. Eventough this is small segment.

What happens instead?

I am wondering why adjacent disconnected road and offside road not shown as warning.

Please provide any additional information below. Attach a screenshot if possible.

Node 6479785832, 4263968339, and node 6485923883.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2019-07-09 01:01:06 +0200 (Tue, 09 Jul 2019)
Build-Date:2019-07-08 23:02:00
Revision:15237
Relative:URL: ^/trunk

Identification: JOSM/1.5 (15237 en) Windows 7 64-Bit
OS Build number: Windows 7 Professional (7601)
Memory Usage: 1239 MB / 1820 MB (263 MB allocated, but free)
Java version: 1.8.0_211-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
Dataset consistency test: No problems found

Plugins:
+ DirectUpload (35041)
+ apache-commons (34908)
+ buildings_tools (34982)
+ ejml (35049)
+ geojson (124)
+ geotools (34908)
+ imagery_offset_db (34908)
+ jaxb (35014)
+ jts (34908)
+ opendata (34997)
+ reverter (34999)
+ todo (30306)
+ turnrestrictions (34977)
+ utilsplugin2 (34977)

Tagging presets:
+ %<user.name>Profile%\Dropbox\HOT\Tool Setting Files\FB_presets.xml
+ %<user.name>Profile%\Dropbox\HOT\OSM_Indonesia_Presets_Ver.3.2.xml

Map paint styles:
- %<user.name>Profile%\Dropbox\HOT\Tool Setting Files\FB_rainbow_color_style.mapcss
+ %<user.name>Profile%\Dropbox\HOT\Tool Setting Files\FB_style.mapcss

Validator rules:
+ %<user.name>Profile%\Dropbox\HOT\Tool Setting Files\FB_rules_HOT.validator.mapcss

Last errors/warnings:
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out
- W: java.net.SocketTimeoutException: connect timed out

Attachments (9)

Capture.JPG (90.4 KB ) - added by adiatmad 5 years ago.
node 6485923883.jpeg (170.7 KB ) - added by adiatmad 5 years ago.
node4263968339.JPG (153.7 KB ) - added by adiatmad 5 years ago.
josm_17914.osm.bz2 (804 bytes ) - added by skyper 4 years ago.
sample file
josm_17914_unlocked.osm.bz2 (816 bytes ) - added by skyper 4 years ago.
unlocked example
17914.patch (6.8 KB ) - added by GerdP 4 years ago.
Work in progress
josm_17914_false_positive.osm (30.3 KB ) - added by skyper 4 years ago.
better keep the examples off the server
17914.2.patch (10.6 KB ) - added by GerdP 4 years ago.
josm_end_node_embankment_example.osm (47.4 KB ) - added by skyper 4 years ago.
real world example of embankment

Download all attachments as: .zip

Change History (64)

by adiatmad, 5 years ago

Attachment: Capture.JPG added

by adiatmad, 5 years ago

Attachment: node 6485923883.jpeg added

by adiatmad, 5 years ago

Attachment: node4263968339.JPG added

comment:1 by adiatmad, 5 years ago

Description: modified (diff)

comment:2 by Klumbumbus, 5 years ago

Component: CoreCore validator

comment:3 by Don-vip, 5 years ago

Keywords: regression unconnected highway added
Priority: normalmajor
Summary: Offside Road not Detected as WarningUnconnected highways test does not work

comment:4 by Don-vip, 5 years ago

Keywords: regression removed
Priority: majornormal
Summary: Unconnected highways test does not workUnconnected highways test does not work if highway are connected with another node

comment:5 by GerdP, 4 years ago

Resolution: invalid
Status: newclosed

The messages are probably produced by rules in FB_rules_HOT.validator.mapcss.

comment:6 by skyper, 4 years ago

The test did not work properly for some time and data did change meanwhile. Only n6479785832 is still a problem in my eyes, though, it is connected but the way`s end node is unconnected and close to another way. See josm_17914.osm.bz2.

by skyper, 4 years ago

Attachment: josm_17914.osm.bz2 added

sample file

comment:7 by GerdP, 4 years ago

Does that mean you have the file FB_rules_HOT.validator.mapcss?

comment:8 by GerdP, 4 years ago

Resolution: invalid
Status: closedreopened

Ah, sorry, now I understand. The ticket is about a missing error message, not a wrong one.

in reply to:  7 ; comment:9 by skyper, 4 years ago

Replying to GerdP:

Does that mean you have the file FB_rules_HOT.validator.mapcss?

No, I was talking about the internal validator tests "crossing without connection" and "end node not connected". Is it possible to overrule internal messages? It is possible to disable internal rules but we never ask about the settings.

comment:10 by skyper, 4 years ago

Keywords: end node near other added
Summary: Unconnected highways test does not work if highway are connected with another nodeEnd node near other way test does not work if highways are connected with another node

in reply to:  9 comment:11 by GerdP, 4 years ago

Replying to skyper:

Replying to GerdP:

Does that mean you have the file FB_rules_HOT.validator.mapcss?

No, I was talking about the internal validator tests "crossing without connection" and "end node not connected". Is it possible to overrule internal messages? It is possible to disable internal rules but we never ask about the settings.

Replying to skyper:

Replying to GerdP:

Does that mean you have the file FB_rules_HOT.validator.mapcss?

Is it possible to overrule internal messages?

No, don't think so. I just didn't understand the problem.

Reg. the sample data:
The test ignores node 6479785832 because it is close to node 6479822118 which is connected. This is a common scenario with footways ending near buildings. Maybe the test could be improved by checking if the last segment with the unconnected node
builds a sharp angle like here or is close to a right angle?

comment:12 by GerdP, 4 years ago

Special case with your file: The data is locked and therefore the test is skipped.

in reply to:  12 comment:13 by skyper, 4 years ago

Replying to GerdP:

Special case with your file: The data is locked and therefore the test is skipped.

Damn, forgot about it again. Why are test skipped with locked data ?

by skyper, 4 years ago

Attachment: josm_17914_unlocked.osm.bz2 added

unlocked example

comment:14 by GerdP, 4 years ago

Good question. The implementation of the locked status has some sideeffects. The test tries to get the active edit layer:

  ds = OsmDataManager.getInstance().getEditDataSet();

and this returns null because the layer is not editable. With null in this variable the test does nothing useful.
I don't know if this effect is intended. Does it make sense to run validator tests on a locked layer?

comment:15 by skyper, 4 years ago

Yes, for debugging purpose. I usually upload my examples with the strictest state possible but scripts might also produce files with state locked and why should validator treat them different?

comment:16 by skyper, 4 years ago

Are there reason for not running validator on locked layers?

comment:17 by GerdP, 4 years ago

I don't say that it is intended. Possible reason: You cannot correct the error in a locked layer. If that is the idea the validator should be disabled completely.

comment:18 by Klumbumbus, 4 years ago

Yes, the state "locked" means no upload, download or editing of that layer, see also wiki:/Help/Menu/OSMLayer#LayerState

comment:19 by skyper, 4 years ago

Ok, completely disabled is a different case than different results.

In my case, I want to show the problem but no edit, upload or download action is needed, e.g. locked

comment:20 by GerdP, 4 years ago

I'll have a closer look tomorrow. A potential problem with locked layers is the autofix function. Makes no sense to create fixes for locked layers, right? Or should we produce them to get the same result?

comment:21 by skyper, 4 years ago

I did created #19003.

by GerdP, 4 years ago

Attachment: 17914.patch added

Work in progress

comment:22 by GerdP, 4 years ago

Milestone: 20.04

See attached patch. I am still struggling with this test. In what situation is such a short segment (not) an error?
I've found a few problems with the existing test:
1) The test should ignore node/way segment pairs which are connected with a detour of 4 * minDist. The current algo accepts much longer detours because some parts where not taken into account.
2) To find the special case with the sharp angle I've added two more rules:

  • if direct distance between the segment and the unconnected nodes is less than 0.1 m
  • if the detour builds a sharp angle and direct distance * 4 > length of detour

A few edge cases which are now flagged :
n1969826537 (sharp angle is probably too sharp)
n6391919540 (false positive)
n6392050845 (I would not have mapped way 682543326)
n421827210 (highway=turning_circle missing?)

Last edited 4 years ago by skyper (previous) (diff)

in reply to:  22 comment:23 by skyper, 4 years ago

Replying to GerdP:

n1969826537 (sharp angle is probably too sharp)

Looks like a mapping fault. At least Imagery show a different angle

n6391919540 (false positive)

Yes, see below

n6392050845 (I would not have mapped way 682543326)

Yes, see below

n421827210 (highway=turning_circle missing?)

Mmh, I would add the highway=turning_circle to the intersecting node and delete the last node. Everything else show be rather mapped with osmwiki:Key:area:highway

In all cases I would ask about the physical separation or routing purpose of the ways, otherwise they are no false positives.

Mapping in such detail but only half of the information will always be a problem. In the first three cases the buildings could be connected either with an additional way/path or as is and a tag for the door/gate/entrance added to the connecting node. Which leads us to the discussion in #18763.

by skyper, 4 years ago

better keep the examples off the server

comment:24 by GerdP, 4 years ago

Question is what this test is about. I think the original idea was to find end nodes close to other highways where the mapper simply forgot to connect the roads, similar to the check done by "Crossing highways".
Now we would mark nodes which should either be removed or connected with other objects like buildings.
These are different problems and thus should produce different messages.

comment:25 by skyper, 4 years ago

The titles "end node near other (high)way" are still valid.
+1 for splitting the test

in reply to:  25 comment:26 by GerdP, 4 years ago

Replying to skyper:

The titles "end node near other (high)way" are still valid.
+1 for splitting the test

Not sure what you mean with splitting.
Today I tried to implement a check which would suppress the message when a plausible target is close to the endnode, e.g. a building or shop or similar. I gave up because this would also suppress cases where the connection is really missing
or the short stub is really an error.
I can produce a different message for the sharp angle. I wonder why this is not detected by the SharpAngles test. Will look at this tomorrow.

by GerdP, 4 years ago

Attachment: 17914.2.patch added

comment:27 by GerdP, 4 years ago

I've changed the additional rules so that nodes are not considered as connected via a detour if

  • direct distance is < 0.1m or
  • direct distance is <= 0.5m and there is a significant angle (detour length >= 1.5 * direct distance)

Besides that the error messagge is also suppressed if a way with waterway=* or a buildings is found between the node and the highway. The unpatched version only looks for ways with barrier=*.

With this patch the original problem is found and none of the examples in attachment josm_17914_false_positive.osm is flagged.

Reg. SharpAngles: This test only reports sharp angles within one way. In josm_17914_unlocked.osm.bz2 the sharp angle is between two different highways.

Version 0, edited 4 years ago by GerdP (next)

in reply to:  27 ; comment:28 by skyper, 4 years ago

Replying to GerdP:


Besides that the error message is also suppressed if a way with waterway=* or a buildings is found between the node and the highway. The unpatched version only looks for ways with barrier=*.

Nice, did not know about barrier but had similar already in mind.

With this patch the original problem is found and none of the examples in attachment josm_17914_false_positive.osm is flagged.

Perfect, thanks

Reg. SharpAngles: This test only reports sharp angles within one way. In josm_17914_unlocked.osm.bz2 the sharp angle is between two different highways.

Be careful, at least ignore highways with placement=transition or turn_restriction when looking at sharp angles between two highways. oneway=yes is probably another hint and intersections where eight or more ways meet in one node might be challenging.

in reply to:  28 comment:29 by GerdP, 4 years ago

Reg. SharpAngles: This test only reports sharp angles within one way. In josm_17914_unlocked.osm.bz2 the sharp angle is between two different highways.

Be careful, at least ignore highways with placement=transition or turn_restriction when looking at sharp angles between two highways. oneway=yes is probably another hint and intersections where eight or more ways meet in one node might be challenging.

No idea where this is relevant for unconnected highways? I don't plan to change SharpAngles Test.

comment:30 by skyper, 4 years ago

Sorry, I read it like you plan to change it. Do you still plan to have different warnings for the different cases (not connected at all vs. not connected at end node)?

comment:31 by GerdP, 4 years ago

The new patch doesn't add a new message. I don't understand

(not connected at all vs. not connected at end node)?

The current algo works like this:
1) Collect all ways with highway (or waterway or power etc)
2) Collect end nodes of those ways which are not connected to other collected ways and don't have tags or parent ways which mark the end, e.g. highway=turning_circle or entrance=* or parent way is a building)
3) For each segment of the collected ways, check if any of the unconnected nodes within the given distance if the node is not part of the same way and can be reached by travelling on the network segments without making a detour of 4 times the given dist. If no short detour is found, the program checks if a barrier is between the segment and the node.
The current algo doesn't search all possible connections and may fail to find the shortest one, but that is unlikely.
4) If a pair of segment and node is found it is stored in a hashmap. If another segment is found for the same node the closer one is kept.
5) For each node / segment pair in the hashmap a message is created.

A few more tests are done, e.g. the layer tag is checked and also handling of highway=platform is a bit special.

A way with no connection is nothing special here.

in reply to:  31 comment:32 by skyper, 4 years ago

Replying to GerdP:

The new patch doesn't add a new message. I don't understand

(not connected at all vs. not connected at end node)?

I was talking about your comment 32 hours ago:

Question is what this test is about. I think the original idea was to find end nodes close to other highways where the mapper simply forgot to connect the roads, similar to the check done by "Crossing highways".
Now we would mark nodes which should either be removed or connected with other objects like buildings.
These are different problems and thus should produce different messages.

comment:33 by GerdP, 4 years ago

I plan no further changes in this direction. Is likely to produce many false positives and as it requires some kind of AI to distinguish valid short ways from invalid ones.
If you think that we should flag all unconnected end nodes I can add an expert preference which could e.g. change the factor for the detour length from 4 to a smaller value. You can try that also with the patch by changing the value for DETOUR_FACTOR.

comment:34 by skyper, 4 years ago

Finding all unconnected end nodes should be possible with search.

comment:35 by GerdP, 4 years ago

Resolution: fixed
Status: reopenedclosed

In 16240/josm:

fix #17914: End node near other way test does not work if highways are connected with another node

  • correct calculation of detour length (old code sometimes accepted too long detours)
  • complain if direct distance is < 0.1m
  • complain if direct distance is <= 0.5m and there is a significant angle (detour length >= 1.5 * direct distance)
  • in addition to barriers check also for waterway or building when looking for objects between unconnected node and highway segment

comment:36 by simon04, 4 years ago

@GerdP: In the past we refrained from committing bigger changes immediately after a release to make hotfixes easier, see DevelopersGuide/Schedule.

comment:37 by GerdP, 4 years ago

OK, so let's hope we don't need a hotifx.

comment:38 by Don-vip, 4 years ago

Let's wait 2 or 3 days before new big changes.

in reply to:  35 comment:39 by skyper, 4 years ago

Replying to GerdP:

  • in addition to barriers check also for waterway or building when looking for objects between unconnected node and highway segment

Please, add man_made=embankment to the list of barriers. Thanks

Last edited 4 years ago by skyper (previous) (diff)

comment:40 by GerdP, 4 years ago

Do you have a real world example? Is it an embankment between the end node and the highway or is the highway on the embankment?

by skyper, 4 years ago

real world example of embankment

comment:42 by Klumbumbus, 4 years ago

Thats not the embankement. Thats the highway=track to the other tracks. Double click the validator message to select the objects.

Last edited 4 years ago by Klumbumbus (previous) (diff)

in reply to:  42 ; comment:43 by skyper, 4 years ago

Replying to GerdP:

Is it an embankment between the end node and the highway or is the highway on the embankment?

The highway is separated by embankments from the other highway. No highways with embankment=yes or connected with man_made=embankment

Replying to Klumbumbus:

Thats not the embankement. Thats the highway=track to the other tracks. Double click the validator message to select the objects.

Gerd implemented checks, to not warn if a barrier is between the node and the way. I want to count man_made=embankment as barrier in these kind of situations.

comment:44 by skyper, 4 years ago

natural=cliff is another candidate.

in reply to:  43 comment:45 by Klumbumbus, 4 years ago

Replying to skyper:

Gerd implemented checks, to not warn if a barrier is between the node and the way.

Ah, I didn't notice that.

comment:46 by GerdP, 4 years ago

It's easy to add more tags to the code:

  • src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java

     
    576576        }
    577577
    578578        private boolean isObstacle(Way w) {
    579             return w.hasKey("barrier", "waterway") || isBuilding(w);
     579            return w.hasKey("barrier", "waterway") || isBuilding(w) || w.hasTag("man_made", "embankment");
    580580        }
    581581
    582582    }

I wanted to wait for release of 20.05. Should I commit it now?

in reply to:  46 comment:47 by skyper, 4 years ago

Replying to GerdP:

It's easy to add more tags to the code:

I wanted to wait for release of 20.05. Should I commit it now?

If it does not change translation stuff and as this is only a minimal update within the release cycle, I think you can go for it.
So man_made=embankment, man_made=dyke, natural=arete, natural=cliff, natural=earth_bank, natural=gully, natural=ridge would be a list of barrier like tags.

comment:48 by GerdP, 4 years ago

I wonder if there is any tagged object which is not acceptable as a barrier.

in reply to:  48 comment:49 by skyper, 4 years ago

Replying to GerdP:

I wonder if there is any tagged object which is not acceptable as a barrier.

Real world object or OSM object?

In OSM all the boundaries, area:highway, landuse and natural come to my mind. Also amenities and shops if they cover more than only buildings. Additional, you will never know what people tag.

comment:50 by GerdP, 4 years ago

Yes, but it also seems impossible to list all tags which are known as barriers. I created the small list as it catches the typical reasons. My new idea is that a mapped object between the end node and the highway segment is a very good reason to suppress the warning. Even boundaries could be a reason, at least those with admin_level=2.
I don't expect lots of suppressed correct positives.

in reply to:  50 comment:51 by skyper, 4 years ago

Replying to GerdP:

Yes, but it also seems impossible to list all tags which are known as barriers. I created the small list as it catches the typical reasons.

We are almost done, just seven more tags. I do not expect to find much more.

My new idea is that a mapped object between the end node and the highway segment is a very good reason to suppress the warning. Even boundaries could be a reason, at least those with admin_level=2.
I don't expect lots of suppressed correct positives.

-1
Rather get some false positives than missing the important valid ones like major road connections crossing postal_code borders.

comment:52 by GerdP, 4 years ago

Last edited 4 years ago by GerdP (previous) (diff)

in reply to:  52 comment:53 by skyper, 4 years ago

Replying to GerdP:

Other candidates:
landuse=grasslanduse=flowerbed natural=tree_row

No, than I fear we spoil the test completely. Excluding barrier=kerb and barrier=line is already critical.

Maybe, forget all natural and only treat the two, man_made=embankment and man_made=dyke as barriers in all test, like the "highway crossing barrier" test.

In general, we should stay in sync and either warn about crossing ways or do not count them as barrier.

In the end, mirco-mapping needs to be used correct and the real problem are the missing noexit=yes on the end nodes.

comment:54 by Klumbumbus, 4 years ago

Milestone: 20.0420.05

Milestone renamed

comment:55 by GerdP, 4 years ago

In 16404/josm:

see #17914: End node near other way test does not work if highways are connected with another node

  • add man_made=embankment and man_made=dyke as barrier like obstacles

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.