Modify

Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#8519 closed enhancement (fixed)

Validator error for keys suffixed with :lanes

Reported by: imagic Owned by: team
Priority: normal Milestone: 14.01
Component: Core validator Version:
Keywords: Cc:

Description

The validator should show an error in the following case:

  • count the number of the character | (vertical bar) in the value of keys which end with any of the following: :lanes, lanes:forward, lanes:backward, lanes:both_ways . Consider the value for each suffix separately.
  • If for any suffix the number of the character | is greater than zero, all keys with the same suffix must have the same number of | . Otherwise show an error: In case of keys suffixed with :lanes show "Number of lane dependent values inconsistent". For keys with different suffixes show "Number of lane dependent values inconsistent in XXX direction" whereas XXX should be the direction suffix like forward, backward or both_ways.

Examples:
turn:lanes=left|right + change:lanes=only_left|not_right|yes

Error: Number of lane dependent values inconsistent

width:lanes:forward=1|2|3 + psv:lanes:forward=no|designated

Error: Number of lane dependent values inconsistent in forward direction

change:lanes:forward=yes|no + turn:lanes:backward=left|right|left

Ok - no error (different suffixes)

turn:lanes:forward=left|right + change:lanes:forward=yes|no|yes + width:backward=1|2|3

Error: Number of lane dependent values inconsistent in forward direction

Attachments (0)

Change History (29)

comment:1 by imagic, 11 years ago

Ticket #8520 has been marked as a duplicate of this ticket.

comment:2 by aceman, 11 years ago

This would be great.
And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.
E.g. lanes=3 + lanes:forward=1 and lanes:backward=1 is an error. Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.

in reply to:  2 ; comment:3 by imagic, 11 years ago

Replying to aceman:

And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.

This is dangerous as the lanes key only count lanes for motorized traffic while all keys with the :lanes extension specify lane-dependent values for all kind of lanes, e.g. also bicycle lanes.

Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.

This is not an error! There might be a lane that is not for motorized traffic!

in reply to:  3 ; comment:4 by skyper, 11 years ago

Replying to imagic:

Replying to aceman:

And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.

This is dangerous as the lanes key only count lanes for motorized traffic while all keys with the :lanes extension specify lane-dependent values for all kind of lanes, e.g. also bicycle lanes.

Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.

This is not an error! There might be a lane that is not for motorized traffic!

Yes, the value "yes" is an problem but not that you have more values than the value of lanes:*=

in reply to:  4 comment:5 by imagic, 11 years ago

Replying to skyper:

Replying to imagic:

Replying to aceman:

And all the found values should also be compared to the value of "lanes" and "lanes:forward" and "lanes:backward", whether they all logically match.

This is dangerous as the lanes key only count lanes for motorized traffic while all keys with the :lanes extension specify lane-dependent values for all kind of lanes, e.g. also bicycle lanes.

Or lanes:forward=1 + turn:lanes:forward=yes|no is an error.

This is not an error! There might be a lane that is not for motorized traffic!

Yes, the value "yes" is an problem but not that you have more values than the value of lanes:*=

No. Once again: the key lanes count only lanes for motorised traffic. All keys with the :lanes suffix specify properties for all kind of lanes. If the road has (in forward direction) one lane for cars and one bicycle lane and the lane for cars is a left turn you would tag: lanes:forward=1 + turn:lanes:forward=left|none. Additional tags might be e.g. vehicle:lanes:forward=yes|no + bicycle:lanes=yes|designated. Please read the approved proposal of the :lanes-extension: http://wiki.openstreetmap.org/wiki/Proposed_features/lanes_General_Extension#The_issues_with_the_lanes_tag

comment:6 by aceman, 11 years ago

If there are other than "motorized" lanes on the road then they should be tagged in some way. E.g. lanes:bicycle=1 or maybe determined from the counting of lanes in bicycle:lanes=*. Then, the validator can take that into account and as I said check if they LOGICALLY match. If the algorithm must take cyclelanes into account, so be it. I would still consider "lanes:forward=1 + turn:lanes:forward=left|none" for a warning if these are the only tags on the way. If there is a cycleway in there, it is not tagged and invisible to any routing/rendering.

Anyway, I hope you agree that if there are MORE lanes= than number of lanes mentioned in e.g. turn:lanes then it is a problem?

in reply to:  6 ; comment:7 by imagic, 11 years ago

Replying to aceman:

If there are other than "motorized" lanes on the road then they should be tagged in some way. E.g. lanes:bicycle=1
or maybe determined from the counting of lanes in bicycle:lanes=*.

You can not restrict yourself to bicycle lanes (neither lanes:bicycle nor bicycle:lanes). See below for the reason - bicycle lanes are just an example. BTW: bicycle:lanes does NOT give you the number of bicycle lanes! Only if you count the designated values, but I wouldn't rely on that as the tagging it some kind of a mess here (official? designated? yes?)

Then, the validator can take that into account and as I said check if they LOGICALLY match. If the algorithm must take cyclelanes into account, so be it.

What's there to "logically match"? All xxx:lanes-keys must have the same number of lane-dependent values. This number is the total number of lanes. That's all the information that you get from those keys about the number of lanes. The only thing you could verify is that the value of the lanes-key must not be larger than the number of the lane-dependent values (in the respective direction).

I would still consider "lanes:forward=1 + turn:lanes:forward=left|none" for a warning if these are the only tags on the way. If there is a cycleway in there, it is not tagged and invisible to any routing/rendering.

No. The validator does not have enough information to give a warning. Bicycle lanes were just an example. There could be other kinds of lanes that the validator doesn't know of and a warning would scare off mappers and possible prevent them from applying completely correct tags.

Anyway, I hope you agree that if there are MORE lanes= than number of lanes mentioned in e.g. turn:lanes then it is a problem?

Yes, that's clearly an error.

in reply to:  7 ; comment:8 by aceman, 11 years ago

Replying to imagic:

You can not restrict yourself to bicycle lanes (neither lanes:bicycle nor bicycle:lanes). See below for the reason - bicycle lanes are just an example. BTW: bicycle:lanes does NOT give you the number of bicycle lanes! Only if you count the designated values, but I wouldn't rely on that as the tagging it some kind of a mess here (official? designated? yes?)
What's there to "logically match"? All xxx:lanes-keys must have the same number of lane-dependent values. This number is the total number of lanes. That's all the information that you get from those keys about the number of lanes. The only thing you could verify is that the value of the lanes-key must not be larger than the number of the lane-dependent values (in the respective direction).

lanes <= count_of(*:lanes) (for each *) (1st check proposed in the ticket)
lanes = lanes:forward + lanes:backward (2nd check proposed by me)
lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)

I would still consider "lanes:forward=1 + turn:lanes:forward=left|none" for a warning if these are the only tags on the way. If there is a cycleway in there, it is not tagged and invisible to any routing/rendering.

No. The validator does not have enough information to give a warning. Bicycle lanes were just an example. There could be other kinds of lanes that the validator doesn't know of and a warning would scare off mappers and possible prevent them from applying completely correct tags.

What is the value of the turn:lanes (etc) having more lanes than are visible via 'lanes' ? Then you don't know which of the marked turn lanes belong to the motorized lanes and which do not.
I just say that for this tagging to have any value is to find out the number/order of non-motorized lanes from lanes:non-motorized and non-motorized:lanes. If there is no consensus yet on how to do that, then I agree, the check can't be done yet (the 3rd check of mine). But then the tagging is inadequate and needs to be sorted out in a proposal on wiki.

comment:9 by aceman, 11 years ago

I have found that the "Lanes and Road attributes" map style (available from inside JOSM preferences or at http://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes) already does warn of some inconsistencies of lanes and also can draw e.g. turn:lanes. I have just found it and it seems a big help when focusing on tagging lanes on roads.

comment:10 by simon04, 11 years ago

Resolution: fixed
Status: newclosed

In 6592/josm:

fix #8519 - Validator: validate :lanes values

comment:11 by simon04, 11 years ago

imagic: Thank you a lot for giving those 4 examples with expected result. Those have been converted 1:1 to unit tests source:trunk/test/unit/org/openstreetmap/josm/data/validation/tests/LanesTest.groovy

comment:12 by aceman, 11 years ago

OK, so this seems to check the consistency of all *:lanes tags. Should I file a new ticket for the other 2 checks from my comment 8?

I don't know what the 'change' key is, but it is good the check also processes unknown keys. The key name actually isn't important, only the number of its values.

in reply to:  12 comment:13 by simon04, 11 years ago

Resolution: fixed
Status: closedreopened

Replying to aceman:

OK, so this seems to check the consistency of all *:lanes tags. Should I file a new ticket for the other 2 checks from my comment 8?

No, let's reopen this one. I would like to hear imagic's opinion on the other tests. The long discussion here doesn't make it easier to determine what to implement and what not.

I don't know what the 'change' key is, but it is good the check also processes unknown keys. The key name actually isn't important, only the number of its values.

To which change key are you referring to?

in reply to:  8 ; comment:14 by imagic, 11 years ago

Replying to aceman:

lanes <= count_of(*:lanes) (for each *) (1st check proposed in the ticket)

This would make sense. You could also test for the suffixes :forward/:backward in this context, i.e.:

lanes <= count_of(*:lanes)
lanes:forward <= count_of(*:lanes:forward)
lanes:backward <= count_of(*:lanes:backward)

lanes = lanes:forward + lanes:backward (2nd check proposed by me)

Not a good idea, because there are some lanes, which are dedicated to both directions, i.e. center turning lanes. Have a look: http://www.fhwa.dot.gov/publications/research/safety/08042/images/image024.jpg
I proposed(!) the suffix :both_ways for those, but it is not really in use right now, therefore I would not test for it.

One could test for lanes >= lanes:forward + lanes:backward . I personally(!) would not do this, because I either tag (i) only lanes OR (ii) only lanes:forward and lanes:backward. I never tag lanes AND lanes:forward AND lanes:backward, but as this is just a personal preference this test would make sense.

lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)

Very bad idea, because there is simply no tag like "lanes:non-motorized". If you want the number of all lanes, you have to count the values of any :lanes-key. It is a well known deficit of the lanes-key that there is no possibility to specify the count of all lanes.

Replying to simon04:

Replying to aceman:

I don't know what the 'change' key is

http://wiki.openstreetmap.org/wiki/Proposed_features/change
It is more or less defacto approved: http://taginfo.openstreetmap.org/search?q=change

in reply to:  14 ; comment:15 by aceman, 11 years ago

Replying to imagic:

Replying to aceman:

lanes <= count_of(*:lanes) (for each *) (1st check proposed in the ticket)

This would make sense. You could also test for the suffixes :forward/:backward in this context, i.e.:

lanes <= count_of(*:lanes)
lanes:forward <= count_of(*:lanes:forward)
lanes:backward <= count_of(*:lanes:backward)

Thanks. Yes, as you say "lanes" may not exists on a way, so only do the check that is applicable (for lanes:forward or lanes:backward).

lanes = lanes:forward + lanes:backward (2nd check proposed by me)

Not a good idea, because there are some lanes, which are dedicated to both directions, i.e. center turning lanes. Have a look: http://www.fhwa.dot.gov/publications/research/safety/08042/images/image024.jpg
I proposed(!) the suffix :both_ways for those, but it is not really in use right now, therefore I would not test for it.

One could test for lanes >= lanes:forward + lanes:backward . I personally(!) would not do this, because I either tag (i) only lanes OR (ii) only lanes:forward and lanes:backward. I never tag lanes AND lanes:forward AND lanes:backward, but as this is just a personal preference this test would make sense.

I always tag "lanes" and "lanes:forward" and "lanes:backward" explicitly if lanes:forward != (lanes / 2) AND lanes:backward != (lanes / 2). But I think I have seen objects with "lanes" and only "lanes:forward" specified, so that the reader must infer the number for lanes:backward.
But the wiki says for now:
If the lanes on a two way road are not distributed evenly between the driving directions, the keys lanes:forward=* and lanes:backward=* can be used in addition to the lanes tag.

So it looks like "lanes" should be there, regardless of existence of "lanes:forward" or "lanes:backward" .

So the check could be modified like this:
IF "lanes" exists then:

  1. IF both "lanes:forward" AND "lanes:backward" exist, check lanes = lanes:forward + lanes:backward
  2. IF only "lanes:forward" OR only "lanes:backward" exists, check "lanes >= lanes:forward" OR "lanes >= lanes:backward" (the one that exists).

Otherwise, no warning, if you wish so. Even though I would personally issue a warning if there is lanes:forward OR lanes:backward and no "lanes" as the wiki says that is not to spec.

lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)

Very bad idea, because there is simply no tag like "lanes:non-motorized". If you want the number of all lanes, you have to count the values of any :lanes-key. It is a well known deficit of the lanes-key that there is no possibility to specify the count of all lanes.

"lanes:non-motorized" was a shorthand for "lanes:foot"+"lanes:bicycle"+"lanes:horse"+etc. But of course if people regularly do not tag those then there is no check possible. I just said above I would expect a "lanes:bicycle" to be there as without it the information of the road is incomplete.

comment:16 by simon04, 11 years ago

In 6598/josm:

see #8519 - Lanes validator: add test for lanes <= count_of(*:lanes) and the like

comment:17 by aceman, 11 years ago

What is this?
void test6() {

lanes.check(TestUtils.createPrimitive("way lanes:forward=foo|bar turn:lanes:forward=foo+bar"))
assert lanes.errors.isEmpty()

}

The "lanes:forward=foo|bar" seems like an illegal value. lanes and lanes:forward an lanes:backward should be numeric values. There should be a warning if that is not the case :)

comment:18 by simon04, 11 years ago

To ensure that the test does not throw an exception when the integer conversion fails.

in reply to:  15 comment:19 by imagic, 11 years ago

Replying to aceman:

So the check could be modified like this:
IF "lanes" exists then:

  1. IF both "lanes:forward" AND "lanes:backward" exist, check lanes = lanes:forward + lanes:backward
  2. IF only "lanes:forward" OR only "lanes:backward" exists, check "lanes >= lanes:forward" OR "lanes >= lanes:backward" (the one that exists).

Ignoring the fact that there are lanes dedicated to both driving directions and therefore lanes=lanes:forward+lanes:backward may not be true everywhere isn't really helping. The specification of the lanes key is seriously broken: just have a look at the list of what should be included and what should be excluded when counting - this is far too difficult. lanes=lanes:forward+lanes:backward might be true for your and my region of the world but not for everywhere. Issuing a warning/error would be contra productive for JOSM and OSM, in my opinion.


lanes + lanes:non-motorized = count_of(*:lanes) (for each *) (3rd check proposed by me)

Very bad idea, because there is simply no tag like "lanes:non-motorized". If you want the number of all lanes, you have to count the values of any :lanes-key. It is a well known deficit of the lanes-key that there is no possibility to specify the count of all lanes.

"lanes:non-motorized" was a shorthand for "lanes:foot"+"lanes:bicycle"+"lanes:horse"+etc. But of course if people regularly do not tag those then there is no check possible. I just said above I would expect a "lanes:bicycle" to be there as without it the information of the road is incomplete.

The problem here is that "non-motorized" is an open space. You say it includes foot, bicycle, horse and "etc.". What is this "etc."? This may differ from country to country and maybe will change with time.

comment:20 by aceman, 11 years ago

Ah, I always forget there can be lanes in the middle, that are for no direction (blocked) or for both directions. There should be some tag to specify those too (like lanes:both_ways=1) so that the tags add up to "lanes". But if there isn't yet then yes, we can't check anything.

Do we at least agree on "lanes >= lanes:forward" AND "lanes >= lanes:backward" (for whichever key exists) ?

in reply to:  20 comment:21 by imagic, 11 years ago

Replying to aceman:

Do we at least agree on "lanes >= lanes:forward" AND "lanes >= lanes:backward" (for whichever key exists) ?

One could test for lanes>=lanes:forward+lanes:backward - if lanes:forward/lanes:backward is not set, assume zero. This should work.

comment:22 by simon04, 11 years ago

In 6606/josm:

see #8519 - Lanes validator: add test for lanes>=lanes:forward+lanes:backward

comment:23 by simon04, 11 years ago

Milestone: 14.01
Resolution: fixed
Status: reopenedclosed

Thank you!

Enough tests for now, let's not try to define more tests than actually needed at the moment.

in reply to:  23 comment:24 by imagic, 11 years ago

Replying to simon04:

Thank you!

You're the one doing the work - thanks to you. :-)

comment:25 by aceman, 11 years ago

Thanks for listening to us!
The pace of JOSM development is great these days :)

comment:26 by simon04, 11 years ago

In 6647/josm:

see #8519 - Lanes validator: correctly handle consecutive |s (e.g., yes|| defines 3 lanes)

in reply to:  26 comment:27 by skyper, 11 years ago

Replying to simon04:

In 6647/josm:

see #8519 - Lanes validator: correctly handle consecutive |s (e.g., yes|| defines 3 lanes)

Not sure if I did get it right but I think we only need to evaluate the number of | in these cases. Did you think about streets with more than 3 lanes in one direction ?

comment:28 by simon04, 11 years ago

Year, sure. Before r6647 there was a problem when counting yes|| and similar cases. Now, for the example yes||, 3 lanes are counted correctly. Higher numbers should work equally well.

comment:29 by simon04, 10 years ago

In 6834/josm:

see #8519 - Lanes validator: improve performance

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.