Modify

Opened 12 months ago

Closed 3 months ago

Last modified 2 months 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 12 months ago.
node 6485923883.jpeg (170.7 KB) - added by adiatmad 12 months ago.
node4263968339.JPG (153.7 KB) - added by adiatmad 12 months ago.
josm_17914.osm.bz2 (804 bytes) - added by skyper 3 months ago.
sample file
josm_17914_unlocked.osm.bz2 (816 bytes) - added by skyper 3 months ago.
unlocked example
17914.patch (6.8 KB) - added by GerdP 3 months ago.
Work in progress
josm_17914_false_positive.osm (30.3 KB) - added by skyper 3 months ago.
better keep the examples off the server
17914.2.patch (10.6 KB) - added by GerdP 3 months ago.
josm_end_node_embankment_example.osm (47.4 KB) - added by skyper 2 months ago.
real world example of embankment

Download all attachments as: .zip

Change History (64)

Changed 12 months ago by adiatmad

Attachment: Capture.JPG added

Changed 12 months ago by adiatmad

Attachment: node 6485923883.jpeg added

Changed 12 months ago by adiatmad

Attachment: node4263968339.JPG added

comment:1 Changed 12 months ago by adiatmad

Description: modified (diff)

comment:2 Changed 12 months ago by Klumbumbus

Component: CoreCore validator

comment:3 Changed 12 months ago by Don-vip

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

comment:4 Changed 12 months ago by Don-vip

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 Changed 3 months ago by GerdP

Resolution: invalid
Status: newclosed

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

comment:6 Changed 3 months ago by skyper

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.

Changed 3 months ago by skyper

Attachment: josm_17914.osm.bz2 added

sample file

comment:7 Changed 3 months ago by GerdP

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

comment:8 Changed 3 months ago by GerdP

Resolution: invalid
Status: closedreopened

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

comment:9 in reply to:  7 ; Changed 3 months ago by 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.

comment:10 Changed 3 months ago by skyper

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

comment:11 in reply to:  9 Changed 3 months ago by GerdP

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 Changed 3 months ago by GerdP

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

comment:13 in reply to:  12 Changed 3 months ago by skyper

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 ?

Changed 3 months ago by skyper

Attachment: josm_17914_unlocked.osm.bz2 added

unlocked example

comment:14 Changed 3 months ago by GerdP

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 Changed 3 months ago by skyper

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 Changed 3 months ago by skyper

Are there reason for not running validator on locked layers?

comment:17 Changed 3 months ago by GerdP

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 Changed 3 months ago by Klumbumbus

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

comment:19 Changed 3 months ago by skyper

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 Changed 3 months ago by GerdP

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 Changed 3 months ago by skyper

I did created #19003.

Changed 3 months ago by GerdP

Attachment: 17914.patch added

Work in progress

comment:22 Changed 3 months ago by GerdP

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 3 months ago by skyper (previous) (diff)

comment:23 in reply to:  22 Changed 3 months ago by skyper

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.

Changed 3 months ago by skyper

better keep the examples off the server

comment:24 Changed 3 months ago by GerdP

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 Changed 3 months ago by skyper

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

comment:26 in reply to:  25 Changed 3 months ago by GerdP

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.

Changed 3 months ago by GerdP

Attachment: 17914.2.patch added

comment:27 Changed 3 months ago by GerdP

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.

Last edited 3 months ago by GerdP (previous) (diff)

comment:28 in reply to:  27 ; Changed 3 months ago by skyper

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.

comment:29 in reply to:  28 Changed 3 months ago by GerdP

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 Changed 3 months ago by skyper

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 Changed 3 months ago by GerdP

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.

comment:32 in reply to:  31 Changed 3 months ago by skyper

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 Changed 3 months ago by GerdP

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 Changed 3 months ago by skyper

Finding all unconnected end nodes should be possible with search.

comment:35 Changed 3 months ago by GerdP

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 Changed 3 months ago by simon04

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

comment:37 Changed 3 months ago by GerdP

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

comment:38 Changed 3 months ago by Don-vip

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

comment:39 in reply to:  35 Changed 2 months ago by skyper

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 2 months ago by skyper (previous) (diff)

comment:40 Changed 2 months ago by GerdP

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?

Changed 2 months ago by skyper

real world example of embankment

comment:42 Changed 2 months ago by Klumbumbus

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

Last edited 2 months ago by Klumbumbus (previous) (diff)

comment:43 in reply to:  42 ; Changed 2 months ago by skyper

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 Changed 2 months ago by skyper

natural=cliff is another candidate.

comment:45 in reply to:  43 Changed 2 months ago by Klumbumbus

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 Changed 2 months ago by GerdP

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?

comment:47 in reply to:  46 Changed 2 months ago by skyper

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 Changed 2 months ago by GerdP

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

comment:49 in reply to:  48 Changed 2 months ago by skyper

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 Changed 2 months ago by 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. 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.

comment:51 in reply to:  50 Changed 2 months ago by skyper

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 Changed 2 months ago by GerdP

Last edited 2 months ago by GerdP (previous) (diff)

comment:53 in reply to:  52 Changed 2 months ago by skyper

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 Changed 2 months ago by Klumbumbus

Milestone: 20.0420.05

Milestone renamed

comment:55 Changed 2 months ago by GerdP

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.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.