for bike? Shouldn’t we always use a faster speed for racing bike? Same for path (10 vs 8), service (14 vs. 12) and track (12 vs. 2). And probably some more I overlooked.
Also path is added to the pushing_section list for racingbike but has not the pushing section speed and uses 8km/h instead.
It seems we mix the priority into the speed. I would prefer a separation (especially when we have more options with custom routing and for priority with issue 2333). Do you see a possibility to make this more consistent?
As far as I remember the lower speed limit for residential and service are attempts to reduce the number of artefacts where the route leaves highway=primary in favour of a highway=residential or highway=service for just a short segment because of higher priority.
On highway=path one needs to be extremely cautious with a racing bike, especially as I’m assuming that you do not have any bell. So I believe it is not unrealistic to estimate a slower speed compared to an ordinary bike, also because usually a path without more information probably goes along with a bad surface for a race bike. By the way, a simple highway=path results in PUSHING_SECTION_SPEED in the test for both bike and raceingbike, despite the speed set with setHighwaySpeed.
For the highway=track I intentionally used the lowest possible speed, as it is usually tagged together with a tracktype and if not we assume the worst condition for a race bike you can imagine. It is simply no fun riding there if you don’t have any information about the surface/tracktype. But for tracktype=grade1 we do use 20 km/h.
Summing up from my point of view up the inconsistency is only an issue for highway=residential or highway=service case. Maybe this can be solved when having issue 2333 by preferring highway=residential one step less as compared to what we currently do and by reducing the speed for highway=service to 16 for the bike profile.
Thanks for the explanation - this makes sense. Then I’ll create an issue to increase the speed for racingbike for residential roads and reducing the priority one step (plus reducing speed for service roads for normal bike and removing the explicit speed change in racing bike) (?)
It is simply no fun riding there if you don’t have any information about the surface/tracktype
I would argue that “fun” should go into priority … so even if a road is very dangerous and/or unpleasant we should try to keep the speed assumption separate from this. But I understand that you probably (have to) ride more cautious/slower with a racing bike if the surface is ugly.
I found that several cases regarding highway=path are inconsistent (leading to strange differences for e.g. bike and mtb) and will submit a PR shortly to propose a solution.
For the highway=track problem I can imagine that we do not need to add code that makes it more complex and instead we should streamline the current code.
But before I can do this I have some questions as I didn’t find a solution yet (for these problems in my dev branch):
We have two similar code sections where we handle tracktype and surface and both use different definitions of “is designated for bike” (see priority parser and speed parser). Do you know if there was a reason for this difference?
Furthermore the handling of tracktype and surface for track and bridleway is different to that of the “pushing sections” (a term which I’m going to change to slowByDefaultHighways). Why don’t we increase the speed of the “pushing sections” for good surface tags too? Could we add track and bridleway to the “pushing sections“ and have one place for all of them that also handles the surface and not only the tracktype? Or is there a reason we don’t do this?
Yes, we should have better covered the priority/fun aspect you pointed out. But I think that small improvements such as this will always be possible!
I intentionally wanted to minimize the effects of this change localized to the bike profile only as I think that we already covered it for mountainbike and race bike sufficiently.
I guess that nobody checked for consistency of this yet. When you change it we’ll see if and what tests break and need to decide if the changes make sense.
I have now code that is simpler but causes a few changes that needs to be reviewed. For now this just touches the speed; other inconsistencies will be fixed later&separately.
Note in certain separate commits I moved a few tests around. Walking through these commits separately makes it probably more obvious what changed. Or probably just this comparison, which excludes the first commit.
Can you have a look at it? Or maybe you can come up with even less code?
Also from the perspective of a “conceptual simple” approach to avoid having too many differences and that we basically have only two things that influence speed: 1. bike allowed/designated and 2. surface/tracktype/smoothness and the `highway` value basically says which of the default should be used. (elevation is another big factor, but that influence comes outside of the parser)
I think in master we do not properly attribute this. E.g. “path” is added to pushing section although the slow speed (by default) is more because the surface is unpaved.
Also note that the speed should not be mixed with the priority. E.g. for mtb many bad surfaces have an unrealistic high speed. This does not make much sense and is probably just because we did this in the past to influence priority but this is no longer necessary.
Also you can see there is one big TODO NOW because not only bicycle=designated should boost the speed for e.g. track but also a bike route relation should do the same.
update, some examples:
highway=path & surface=paved is now 12 and was 4km/h reasoning: we assume highway=path allows bicycle usage by default
highway=track & bicycle=designated & segregated=no is now 18 and was 12km/h reasoning: same behaviour like highway=path or do we have to assume a bad surface even for bicycle=designated?
highway=track & tracktype=grade1 is now only 12 and was 18km/h reasoning: track has no explicit tag for bicycle usage
default for mtb with highway=track is now also 12 reasoning: mtb shouldn’t have much different speeds for bad tracks
TODO: for racingbike highway=track & bicycle=designated speed of this branch is too high as no surface is specified (18)
For sake of easier commenting I reverted the unit tests, made changes to get them pass based on the new code and commented on all the changed results. See here.
I only had a short glimpse at the code changes so far. While I like the introduction of the case statement I am skeptical on the overall result. I gave the comments based on my experience on the various bicycle types.
Here are comments on the update
highway=path & surface=paved is now 12 and was 4km/h reasoning: we assume highway=path allows bicycle usage by default
We need to consider that ways tagged as such are ways in the mountain/forest. I remember inappropriate usage of such sections through the mountains from the early days. Therefore I consider 12 as much too high, maybe we might increase to 6.
highway=track & bicycle=designated & segregated=no is now 18 and was 12km/h reasoning: same behaviour like highway=path or do we have to assume a bad surface even for bicycle=designated?
I think that we need to assume bad surface there even if bicycle=designated
highway=track & tracktype=grade1 is now only 12 and was 18km/h reasoning: track has no explicit tag for bicycle usage
If a bicycle is allowed there we should assume the same speed compared to a residential as the surface is appropriate.
default for mtb with highway=track is now also 12 reasoning: mtb shouldn’t have much different speeds for bad tracks
The former 18 was probably a bit too high, but with 12 I’m missing the advantage of wide tires.
TODO: for racingbike highway=track & bicycle=designated speed of this branch is too high as no surface is specified (18)
… so even if a road is very dangerous and/or unpleasant we should try to keep the speed assumption separate from this. But I understand that you probably (have to) ride more cautious/slower with a racing bike if the surface is ugly.
Yes, we should have better covered the priority/fun aspect you pointed out. But I think that small improvements such as this will always be possible!