Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misinterpretation of highway=pedestrian oneway=yes #4014

Open
osmuser63783 opened this issue Mar 8, 2023 · 12 comments
Open

Misinterpretation of highway=pedestrian oneway=yes #4014

osmuser63783 opened this issue Mar 8, 2023 · 12 comments
Assignees
Labels

Comments

@osmuser63783
Copy link

Valhalla won't route pedestrians against the flow of traffic in a pedestrian area (highway=pedestrian) that's been tagged oneway=yes. Or at least that's what I think is going on in this example.

Is this intentional? I think it's likely to be a misinterpretation of the oneway=yes tag which is only intended to apply to vehicles.

For comparison, OSRM and GraphHopper have no problems with sending pedestrians the "wrong way down a one way street."

@kevinkreiser
Copy link
Member

@gknisely just mentioned this to me in passing the other day. i only looked into very very briefly but things like this:

valhalla/lua/graph.lua

Lines 1182 to 1192 in 98fa752

if kv["pedestrian_backward"] == nil then
kv["pedestrian_backward"] = "false"
end
if ((kv["oneway"] == "yes" and kv["oneway:foot"] == "no") or kv["foot:backward"] == "yes") then
kv["pedestrian_backward"] = "true"
end
if kv["pedestrian_backward"] == "true" then
oneway_foot = oneway[kv["oneway:foot"]]
end

are pretty unintuitive. i need time to review the access logic pattern we have going there, the code is so large its hard to look at (especially without comments) and quickly make sense of it

@nilsnolde
Copy link
Member

Hm, not sure how to feel about this. It's clearly stated that oneway has to be ineffective for pedestrians, but clearly this way is only accessible for pedestrians! So what else would the oneway tag refer to if not to pedestrians? IMO this is a tagging error.

@osmuser63783
Copy link
Author

Streetview image of how it's signed. It's a pedestrian zone, except for certain vehicles and times, and it's got a one way sign which is clearly only meant to apply to vehicles.

The tagging may be incomplete (it's missing the conditional exemption) but I don't think the combination of highway=pedestrian and oneway=yes is wrong. I've had a quick look and found examples in all the major European cities I checked.

@nilsnolde
Copy link
Member

nilsnolde commented Mar 9, 2023

It's a pedestrian zone, except for certain vehicles and times,

Alright, I'm not exactly sure how best practices are in that case, but then it's missing some tagging IMO to indicate it's fine to use it conditionally or only as "destination", as you say. I can't confirm without a deeper look but I'd guess that Valhalla would respect that also for pedestrian zones and give vehicles access conditionally. It doesn't solve the oneway=yes tag issue, but now I see your point, if it's accessible to vehicles a oneway=yes should also only apply to vehicles, and pedestrian should only be one-way if it's oneway:foot=yes. And generally we could just avoid using oneway for pedestrian. So, the missing tagging really has nothing to do with it, but for my own understanding it'd have been more obvious:)

BTW, I don't see an indication on the Google Streetview image that the road is a one-way, right? It might be used like that, but doesn't seem to be signed like that, unless Scotland works a bit differently than Germany.

@osmuser63783
Copy link
Author

osmuser63783 commented Mar 9, 2023

The one way sign is the blue sign with a white arrow pointing up, it's on the wall to the right.

On the other end there will be a no entry sign. You can see one if you turn the camera by 180 degrees.

@gknisely
Copy link
Member

gknisely commented Mar 9, 2023

Seem like this should be tagged with the following:
highway=pedestrian
motor_vehicle:conditional=delivery @ (23:00-10:30)
oneway=yes

However, I do think there is a bug here as we shut off all access for vehicles if we encounter highway=pedestrian Also, we need to be able to handle cases where it is truly one-way exits as shown here Regardless, this is a bug for sure.

@nilsnolde
Copy link
Member

The one way sign is the blue sign with a white arrow pointing up

Ah interesting: see in Germany those arrows are before an intersection, in this case to indicate you're only allowed to go straight. But maybe they're also round and blue with a white arrow, not rectangular. I don't have a car and I don't care on a bicycle usually haha

handle cases where it is truly one-way exits as shown here

Should we? It's the same thing as right now with the example @osmuser63783 gave in terms of tagging. I'd argue to only ever respect oneway:foot, no matter the highway tag. Otherwise you'd have to check for every single access or other restriction which applies to cars, to evaluate whether oneway=yes does apply to pedestrians or it should only apply to motor_vehicles or whatever. But maybe we'll have to anyways, I'm sure this could be a fairly frequent "mapping error"/disambiguity, similar to the recent case where access roads for ferries were not tagged according to OSM best practices, but it was so frequent, we had to accommodate for it. I guess I just want to express that it bothers me, the whole access and one-way logic in graph.lua is complicated enough as it is.

@osmuser63783
Copy link
Author

osmuser63783 commented Mar 9, 2023

Yes, it seems like the true one way for pedestrians example would more accurately be tagged oneway:foot=yes, or foot:backward=no which was the favoured option when this was discussed in the community forum.

There are 26k cases of highway=pedestrian and 44k cases of highway=footway with oneway=* tags. The question is, how often are these meant to indicate actual one-way pedestrian access, so that it would be a problem to route pedestrians both ways regardless of access restrictions? I would think very rarely. Whereas if you assume that they are oneway unless specific other tags are present, then it's easy to make mistakes.

The access restriction could be unmapped, like in my example. But checking for access restrictions isn't enough. You would also need to check for bicycle=yes. highway=footway bicycle=yes oneway=yes is legitimate tagging for a footway=sidewalk where it's common that bikes are only allowed to go one way but pedestrians can of course walk both ways.

@nilsnolde
Copy link
Member

nilsnolde commented Mar 9, 2023

Uff haha yeah that all exists I guess. Again, I’m also in favor of never allowing oneway for pedestrians other than it being explicitly tagged appropriately. I think the wiki is also quite clear about it.

So, in case of a oneway on pedestrian-only ways (like your example), I guess we’d just assume that any other mode is oneway, whichever has access to that way, e.g. bike, but pedestrian is both ways. And then we’d consider @gknisely ‘s example to be wrongly tagged (ie it should have explicit „oneway:foot=true“)? That’d be preferred from my side and sounds a bit easier on the lua side (without looking in depth).

Opinions?

@gknisely gknisely self-assigned this Mar 10, 2023
@osmuser63783
Copy link
Author

This seems sensible to me.. though I'm just a user who stumbled upon a bug. The oneway tag would just be ignored altogether in pedestrian routing. Only oneway:foot, foot:backward and foot:forward should affect it.

@kadiwa4
Copy link

kadiwa4 commented Jan 26, 2024

Also applies to highway=path's for both bike and pedestrian use: example.

@osmuser63783
Copy link
Author

The above example has since been edited to footway. Here are some examples for path: 1, 2.

Community consensus seems to be, also for path, that oneway is only supposed to apply to bicycles: forum thread today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants