nrenner / brouter-web

Web client for BRouter, a routing engine based on OpenStreetMap
https://brouter.de/brouter-web/
MIT License
374 stars 74 forks source link

Wrong reading of isunpaved in profile fastbike? #17

Closed MaartenDeen closed 9 years ago

MaartenDeen commented 9 years ago

I have this route. With profile fastbike this has a cost of 3217.

It traverses a few ways tagged as highway=track, tracktype=grade2 with no additional tags: ways 140423237, 277710748 and 140423241. If I read the profile of fastbike right, an unpaved grade2 has cost 10, paved (not unpaved) grade2 has cost 3 switch tracktype=grade2 switch isunpaved 10 3

When I edit the profile and change this line to switch tracktype=grade2 switch isunpaved 100 3, the route is the same and the cost also stays on 3217. If I do switch tracktype=grade2 switch isunpaved 10 3.1, the cost goes to 3286.

For some reason, the tracks are seen as paved? Or does the switch not work for some reason?

nrenner commented 9 years ago

It seems to me (if I get the notation right) that the isunpaved assignment is negating the whole statement with not: assign isunpaved not or surface= or ispaved or surface=fine_gravel surface=cobblestone

instead of just ispaved: assign isunpaved or surface= or not ispaved or surface=fine_gravel surface=cobblestone

@abrensch

MaartenDeen commented 9 years ago

That does solve it, but I don't really see why it should matter.

I need to reconstruct the logic: surface=cobblestone is false surface=fine_gravel is false ispaved is false surface= is false (I assume this means any surface key) Then the original version is isunpaved = not (or surface= (or ispaved (or surface=fine_gravel surface=cobblestone))) isunpaved = not (or false (or false (or false false))) isunpaved = not (or false (or false false)) isunpaved = not (or false false) isunpaved = not (false) isunpaved = true

In the corrected version: isunpaved = (or surface= (or (not ispaved) (or surface=fine_gravel surface=cobblestone))) isunpaved = (or false (or true (or false false))) isunpaved = (or false (or true false)) isunpaved = (or false true) isunpaved = true

Also: if surface= is true when any surface key is set, then surface=fine_gravel and surface=cobblestone are not necessary in the logic. Maybe it was meant as assign isunpaved or not surface= or ispaved or surface=fine_gravel surface=cobblestone Anyway, why should isunpaved not be just not ispaved?

nrenner commented 9 years ago

surface= is false (I assume this means any surface key)

I assumed surface= means "surface is not set", so it would be true.

From the http://brouter.de/brouter/profile_developers_guide.txt:

- if the tag is not set or the value is empty [...] The value is refered to as an empty string, e.g. access=

MaartenDeen commented 9 years ago

That would make sense yes. If you read it like that than the switch is explained.

Still, my comment: why should isunpaved not be just not ispaved? To do it like it is now means some combinations are neither paved nor unpaved and fall through to the default cost at the end of the switch.

MaartenDeen commented 9 years ago

Changing it like that gives errors in other places, but that might be due to changes I made recently. This route goes haywire when I change the isunpaved assignment. I changed most (all?) of the grade1 tracks in that area to surface=asphalt on april 7th. I assume those changes are in the online router?

nrenner commented 9 years ago

Still, my comment: why should isunpaved not be just not ispaved?

abrensch would need to comment on this, he is the author of the profiles.

I changed most (all?) of the grade1 tracks in that area to surface=asphalt on april 7th. I assume those changes are in the online router?

The routing data files at http://brouter.de/brouter/segments3/ are from April 15th, as well as the latest planet http://planet.openstreetmap.org/planet/ on which they should be based on, so your changes should be included.

You can check by switching to the "Data" tab in the web client where the tags are listed.

abrensch commented 9 years ago

Still, my comment: why should isunpaved not be just not ispaved? abrensch would need to comment on this, he is the author of the profiles

True, "isunpaved" is not just the inverse of "ispaved". The cases where isunpaved=ispaved=false are:

The second case is an important design principle of these profiles: "no news is good news". The level of detail in the tagging is not homogenous in OpenStreetMap, so if you e.g. require a grade1 to be tagged surface=paved, you will miss a lot of perfect grade1's that just don't habe the tag.