brouter icon indicating copy to clipboard operation
brouter copied to clipboard

Use cycleway:surface for routing

Open pkubowicz opened this issue 1 year ago • 15 comments

In https://github.com/nrenner/brouter-web/issues/438 I reported that a way with:

surface=paved
cycleway:surface=asphalt
footway:surface=paving_stones

is reported as 100% paved (instead of 100% asphalt).

The response I got was:

should be fixed (not only for asphalt, obviously) in either BRouter's profiles or in BRouter's tag aliasing. We want to not only count the tag in our statistics, but use it for routing too.

So I'm reporting here.

pkubowicz avatar Aug 30 '23 08:08 pkubowicz

As suggested by rkflx, a fix for the trekking profile could look like this:

- assign ispaved = surface=paved|asphalt|concrete|paving_stones|sett
+ assign ispaved = or surface=paved|asphalt|concrete|paving_stones|sett \
+                     cycleway:surface=paved|asphalt|concrete|paving_stones|sett

the same code could be used for the fastbike profile.

quaelnix avatar Aug 30 '23 14:08 quaelnix

I added the footway:surface to the next generation lookups.dat

afischerdev avatar Aug 30 '23 15:08 afischerdev

As suggested by rkflx, a fix for the trekking profile could look like this

FTR, that's not what I said. I said that this "triggers the correct behaviour" for that particular example, and that "not considering cycleway:surface [...] should be fixed". I did in no way describe how the actual fix would look like, so don't make it look like I "suggested a fix" that "could look like this".

Also, either properly @-mention my name, or link to the actual comment. Linking my name without actually notifying me makes people question your intentions.

in either BRouter's profiles or in BRouter's tag aliasing

I might be talking nonsense and should probably read the code, but is there a way in BRouter to automatically alias surface to surface|cycleway:surface for profiles setting validForBikes (and surface|footway:surface for validForFoot)?

This seems a bit more elegant and could ease profile maintenance compared to having to repeat the whole shebang everywhere each time.

(Ideally such aliasing should be reflected in the emitted messages too, i.e. when a profile has validForBikes and evaluates surface, the way's message should say cycleway:surface if that's what was used for routing.)

If that does not work yet or is too hard to implement, changing the profile as mentioned is probably the second best option.

rkflx avatar Aug 30 '23 18:08 rkflx

is there a way in BRouter to automatically alias surface to surface|cycleway:surface

I also have not looked at that part of the code yet, but maybe @afischerdev knows more.

This seems a bit more elegant and could ease profile maintenance

I prefer as little preprocessing as possible, but maybe in this case it indeed would make sense to make an exception?

changing the profile as mentioned is probably the second best option

Yes, it certainly looks tempting, but I wouldn't change it without regression testing.

quaelnix avatar Aug 31 '23 08:08 quaelnix

surface|cycleway:surface

To clarify what I mean by that (in case it wasn't obvious):

  • cycleway:surface should be used if surface is missing (and vice versa),
  • and the more specific cycleway:surface should have precedence over surface if both are specified.

Essentially provide what is called "tag normalization" in nrenner/brouter-web#460 (also note fixes in nrenner/brouter-web#550 and nrenner/brouter-web#553).

not only for asphalt, obviously

And probably not only for surface, but for smoothness too.


That being said, I just discovered another complication with the idea of automatic aliasing:

We'd not only have to look at validForBikes, but would need to differentiate between a profile wanting to route over the (car) lane part of the highway or the cycleway part in case of non-mandatory cycleways. I guess we should leave that to profiles, after all, or only apply the aliasing if cycleway:surface is unambiguous, e.g. if cycleway=* is unset or for highway=path|track|cycleway. Sigh.


changing the profile

I wouldn't change it without regression testing.

What particular regressions do you have in mind?

  • Something like surface=<accurate data> cycleway:surface=<incorrect data>? I doubt this is common, and in any case should be fixed directly in OSM.
  • Or the case of non-mandatory cycleways? Indeed, here we might want to add another condition depending on the profile's needs (see above). In that sense, the current profile code is not entirely accurate currently, either, since we might classify an asphalt road with a mandatory gravel cycleway as paved, which it is not in reality.

Apart from that I don't see any room for trade-offs or subjective preferences here, as it would be merely about evaluating more accurately what has been mapped.

rkflx avatar Aug 31 '23 13:08 rkflx

What particular regressions do you have in mind?

Basically the ones you mentioned above:

  • That, for example, a road bike profile might prefer to route over the (car) lane part.
  • That, for example, cycleway:surface=paved shouldn't overwrite surface=asphalt.

Plus the ones we probably did not think about it yet.

I just discovered another complication with the idea of automatic aliasing

We would also have to properly consider the oneway logic.

quaelnix avatar Aug 31 '23 15:08 quaelnix

I also have not looked at that part of the code yet, but maybe @afischerdev knows more.

I'm sorry, no. I'll do have a look later on - but will take a few weeks. And we will need the lookups update #458. Seeing the weight of the surface value, I could think of ordering the surface values in the lookups.dat file.

afischerdev avatar Aug 31 '23 16:08 afischerdev

cycleway:surface=paved shouldn't overwrite surface=asphalt

I disagree. When the cycleway is mandatory and a patchwork of different paving styles, paved would be much more accurate than asphalt, even though the latter might seem more specific at first glance.


Thinking more about this, perhaps we could define aliasing a bit differently (| meaning "fallback if not existing" as detailed previously, and _ illustrating the tag being an alias):

  • cycling_surface as an alias for cycleway:surface|surface (or possibly even more complex conditions / subtags)
  • walking_surface as an alias for footway:surface|surface (or possibly even more complex conditions / subtags)
  • surface stays as-is (or could be renamed to driving_surface)

That way profiles could simply choose what they require (e.g. it would be explicit whether the car lane or a cycleway is meant), while still being less verbose (no repetitions and no extra conditions necessary). Maybe we could even move aliasing to rd5 creation time that way, instead of having to process it dynamically during routing.

Note that it might make sense to be even more generic here, e.g. define a whole class of aliases like A_X for A:X|X with A ∈ {cycleway, footway, …} and X ∈ {surface, smoothness, …}. Are there any further tags for you could think of?

AFAICT from (finally) reading the code, in lookups.dat we currently only support aliasing like this: tag;001 value1 value2 value3 (where value1 will become an alias for value{1,2,3} in the expression tag=value1). This is different from aliasing tags themselves as discussed here. However, there is also generatePseudoTags which we might be able to extend.

OTOH, it might be too much work for too little gain…

rkflx avatar Aug 31 '23 22:08 rkflx

@rkflx

Does this mean all ways have a new cycleway_surface or footway_surface tag at the end - when they have a surface tag? Could be blow up the data source.

The more I look at it, the more I think: A simple surface has no place in cycleway:surface and/or footway:surface combination. I followed some ways that are visible in GMap Streetview like this. Visible is the traffic seperation but where is the asphalt?

There are more tags around:

cycleway:both:surface
cycleway:right:surface
cycleway:left:surface

sidewalk:surface
sidewalk:both:surface
sidewalk:right:surface
sidewalk:left:surface

afischerdev avatar Sep 01 '23 14:09 afischerdev

Does this mean all ways have a new cycleway_surface or footway_surface tag at the end - when they have a surface tag?

We could coalesce all specialized tags into a single tag per use case, so while there would be some duplication (which might be possible to avoid by deferring selected parts of the fallback algorithm to runtime), we could also drop other tags not needed anymore in such a scenario.

Could be blow up the data source.

Whether or not rd5 sizes will be affected is yet to be determined, it's not even clear where/when to do the aliasing yet. Also note that evaluating more complex expressions in profiles comes with quite a sizable performance penalty too, so there is always a space/time trade-off.


In summary, it does not seem trivial to answer a question like "Assuming a cyclist using mandatory cycleways and riding in direction X on way Y, what will the surface there look like?", since only looking at surface can be incomplete or even misleading, as we've seen.

On the contrary, it can be quite complicated. This brings us back to the original dilemma: Is it reasonable to expect each profile author (which includes advanced users too) to know about and evaluate all of these tag combinations and special conditions, or should we pre-process the complexity into an alias once? Or maybe choose a different approach and extend profile syntax to allow for | and/or * in the left side of tag=value expressions too? Perhaps we should just ask profile authors what they prefer.

Anyway, I'll stop my investigations here for now. Nevertheless I still believe we should probably somehow consider cycleway:surface for routing. I'll be curious though to watch with which constructive solutions you'll come up with.

rkflx avatar Sep 02 '23 06:09 rkflx

I spent an hour trying to find an example (using this overpass query: https://overpass-turbo.eu/s/1zIa) were:

- assign ispaved = surface=paved|asphalt|concrete|paving_stones|sett
+ assign ispaved = or surface=paved|asphalt|concrete|paving_stones|sett \
+                     cycleway:surface=paved|asphalt|concrete|paving_stones|sett

would affect the trekking profile routing and only found this one:

  • https://www.openstreetmap.org/way/1125846213#map=18/53.74589/9.65670
  • https://brouter.de/brouter-web/#map=18/53.74598/9.65651/standard,route-quality&lonlats=9.657122,53.745272;9.656339,53.747054
  • https://www.mapillary.com/app/?lat=53.745974954545&lng=9.6593258963636&z=17&pKey=487840566320545&focus=photo

Querying cycleway:surface in the trekking profile increases the calculation time by about 0.65%.


As a side note, the above overpass query has a near 100% success rate in finding map errors.

quaelnix avatar Sep 02 '23 12:09 quaelnix

I'll be curious though to watch with which constructive solutions you'll come up with.

Probably an unpopular opinion, but I would suggest to do nothing. No tag aliasing and no profile change.

quaelnix avatar Sep 02 '23 12:09 quaelnix

See discussion in https://github.com/abrensch/brouter/pull/641

EssBee59 avatar Nov 06 '23 08:11 EssBee59

See discussion in #641

Could you expand on how the linked PR relates to the issue at hand, since the tag mentioned in the issue's title (cycleway:surface) is nowhere to be found in that PR?

rkflx avatar Nov 06 '23 21:11 rkflx

Tank for the feedback, Sorry! I confused "cycleway" and "cycleroute"... ( https://github.com/abrensch/brouter/issues/622 in my mind)

EssBee59 avatar Nov 07 '23 08:11 EssBee59