Modify

Opened 3 years ago

Last modified 3 years ago

#20057 assigned enhancement

Improve handling of preference `validator.UnconnectedWays.way_way_distance`

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone:
Component: Core validator Version: latest
Keywords: Cc: Klumbumbus, skyper, francois.lacombe

Description (last modified by GerdP)

Description for original ticket "Remove or improve test "Unconnected natural lands and landuses":
I fear I don't understand what this test is about. It collects ways whith keys landuse or natural but it will ignore all unconnected ways with key landuse and most of those with natural as they return true for method concernsArea()
So far I only found the tag natural=ridge which produces a useful warning.
Possible candidates like natural=tree_row or natural=cliff are explicitely excluded in the code. No idea why.
Typos like natural=woter will also produce a warning because concernsArea() returns false.

If preference validator.UnconnectedWays.way_way_distance is changed to a value > 0 (e.g. 5) this test will also produce "Other" messages about two landuse/natural areas which are very close (< 5m) but not connected. No idea where this is relevant.

Attachments (4)

20057-bad.osm (15.6 KB ) - added by GerdP 3 years ago.
special case with missleading Connected way end node near other way (2) messages
20057-beta.patch (9.0 KB ) - added by GerdP 3 years ago.
20057-highway.osm (6.8 KB ) - added by GerdP 3 years ago.
20057-highway-2.osm (3.1 KB ) - added by GerdP 3 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 by GerdP, 3 years ago

Cc: Klumbumbus skyper added
Owner: changed from team to GerdP
Status: newassigned

comment:2 by GerdP, 3 years ago

No idea where this is relevant.

Ah, it can be used to find "unmapped" areas between other areas.
Example: https://www.openstreetmap.org/way/101185578 and https://www.openstreetmap.org/way/12249128
Or nodes which might be merged, like node 2245347926 and node 105292216

by GerdP, 3 years ago

Attachment: 20057-bad.osm added

special case with missleading Connected way end node near other way (2) messages

comment:3 by GerdP, 3 years ago

My findings so far:
When I change the preference validator.UnconnectedWays.way_way_distance from 0 to e.g. 1 (meter) I see some new messages like
Connected way end node near other way
Example: https://www.openstreetmap.org/node/2245347794
I really wonder if anybody understands what that means and what makes it special compared to nearby node
https://www.openstreetmap.org/node/2245347793
which produces a different message Way node near other way
My current understanding is that the first node is used by multiple ways, the second is only used by one. In both cases the node is a candidate for action J Join node to way or N Move node onto way

I wondered if we should simply suggest on of these actions in the message? Something like
Consider to connect node with way
until I found the case in attachment:20057-bad.osm
You don't understand the message until you zoom in to find a culvert with a length of 4 centimeters (!)
This is probably a candidate for a new warning

comment:4 by skyper, 3 years ago

Nice findings. Wonder how a way with width=0.04 m can be called a track.

+1 for a new warning. Think linear features like the second example should be treated differently than areas in the first example.

  1. I do not find a layer in the linear example. Why do I not get a crossing way warning?
    • Think it is a problem with downloaded area as adding a node to the track north of the waterway inside downloaded area with show the expected warning. Think this example should always be warned about and not only inside downloaded area. -> new ticket?
  2. The major problem, I see, is the length of the way. How about a test about length of linear ways in general and having a variable validator.way_min_length to set the minimum allowed value instead of validator.UnconnectedWays.way_way_distance.

comment:5 by GerdP, 3 years ago

In fact the message from UnconnectedWays is wrong as the way is connected via the short culvert. I've just found a fix for that.
I see a Crossing highway/waterway warning without changing anything. Not sure why you get a different result.

comment:6 by GerdP, 3 years ago

Reg. short linear ways: I think it difficult to produce a general warning. At least highway=steps are often very short and still useful. I guess there are tunnel=culvert with lengths < 1m in real world, e.g. between barrier=ditch

comment:7 by skyper, 3 years ago

Strange, now I get the crossing way warning, too.

We have this test with default set to 0.0 or is it 1e7. validator.way_min_length could have the same default value. Additionally, another ignore list could be used. Short way length or Way length under …?

Version 0, edited 3 years ago by skyper (next)

comment:8 by GerdP, 3 years ago

I think this can be done in mapcss using waylength(), right?

by GerdP, 3 years ago

Attachment: 20057-beta.patch added

comment:9 by GerdP, 3 years ago

attachment:20057-beta.patch (well more work in progress than beta)

  • fixes the bug shown in attachment:20057-bad.osm and adds a unit test for it
  • changes the messages produced by the test so that the name is used as a group, e.g. "Unconnected Highways" or "Unconnected Waterways"
Last edited 3 years ago by GerdP (previous) (diff)

in reply to:  8 comment:10 by skyper, 3 years ago

Replying to GerdP:

I think this can be done in mapcss using waylength(), right?

You are right. I did not understand how setting variables in mapcss validator rules works and how can I make those user configurable. Or does this only work with styles?

comment:11 by GerdP, 3 years ago

Did not think about variables. Anyway, please create a new ticket if you think this will not produce too many false positives.

comment:12 by GerdP, 3 years ago

If I got that right right the setting validator.UnconnectedWays.way_way_distance should be ignored when looking at power objects. I am not a power expert but I think it finds only false postives, e.g. nodes n3887078076 or n1734250675. My understanding is that it would be a bad idea to connect the power lines.

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

by GerdP, 3 years ago

Attachment: 20057-highway.osm added

by GerdP, 3 years ago

Attachment: 20057-highway-2.osm added

comment:13 by GerdP, 3 years ago

For highway it helps to find errors like n370755789 in attachment:20057-highway.osm but it's also likely to find false positives like ways under bridges (n286333584) because layer is only considered for unconnected end nodes so far. I'll try to fix that first.
It also finds a rather strange case of parallel ways: w575642793 w351460405 which is not detected by other tests, but it produces lots of warnings for the same problem.
Without the fixes implemented in attachment:20057-beta.patch it also finds the error in attachment:20057-highway-2.osm
There are quite a lot of those cases which I would call something like "impossible sharp angle for possibe travel route".
In other words: The test finds lots of real problem cases but the message texts are rather useless and severity is too low.

comment:14 by GerdP, 3 years ago

For railway it finds far too many false positives near railway=switch without the patch and still too many with it. Also some toys rails in gardens or parks. I looked at many of the cases and found no real problem, so - like with power this is of no use.

comment:15 by GerdP, 3 years ago

For waterway there are typically lots dubious findings near waterway=weir as well as waterway=river nodes which are close to the enclosing waterway=riverbank (or vice versa). Also many messages for crossing waterways which are also found by CrossingWays. So I think validator.UnconnectedWays.way_way_distance is also of no use with waterway.

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

comment:16 by GerdP, 3 years ago

Description: modified (diff)
Summary: Remove or improve test "Unconnected natural lands and landuses"Improve handling of preference `validator.UnconnectedWays.way_way_distance`

in reply to:  12 ; comment:17 by skyper, 3 years ago

Cc: francois.lacombe added

In general, I do not know if there is a policy how to handle results of different test. Are all test in core supposed to be one unit or should they all work on their own.

Does it make sense to treat power, high-/rail-/waterway all individually and exclude them from the general test?

Replying to GerdP:

If I got that right right the setting validator.UnconnectedWays.way_way_distance should be ignored when looking at power objects. I am not a power expert but I think it finds only false postives, e.g. nodes n3887078076 or n1734250675. My understanding is that it would be a bad idea to connect the power lines.

Think this depends on the tags of the pole/tower but your examples seem to be false positives. Wonder if one of the power lines should have a layer. At least the information which line crosses above the other is missing, right now.

in reply to:  17 comment:18 by GerdP, 3 years ago

Replying to skyper:

In general, I do not know if there is a policy how to handle results of different test. Are all test in core supposed to be one unit or should they all work on their own.

Does it make sense to treat power, high-/rail-/waterway all individually and exclude them from the general test?

We have various tests in core which all work on their own. The java source UnconnectedWays.java implements 5 independent tests which share a common code base:

UnconnectedHighways
UnconnectedRailways
UnconnectedWaterways
UnconnectedNaturalOrLanduse
UnconnectedPower

In the past, this was different, see r6515.

Replying to GerdP:

If I got that right right the setting validator.UnconnectedWays.way_way_distance should be ignored when looking at power objects. I am not a power expert but I think it finds only false postives, e.g. nodes n3887078076 or n1734250675. My understanding is that it would be a bad idea to connect the power lines.

Think this depends on the tags of the pole/tower but your examples seem to be false positives. Wonder if one of the power lines should have a layer. At least the information which line crosses above the other is missing, right now.

Maybe. I don't know if I ever saw power lines crossing like that in real world, but I don't look at them that often ;)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as assigned The owner will remain GerdP.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from GerdP to the specified user. Next status will be 'new'.
Next status will be 'needinfo'. The owner will be changed from GerdP to GerdP.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.

Add Comment


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