brouter
brouter copied to clipboard
Update the trekking profile
Here a new version for the trekking profile suggested by @EssBee59
It seems like the updated profile does no longer have a avoid_unsafe
parameter. The fix for the overrideParam
testcase should be a valid parameter (which produces a different result) instead of changing the file.
@zod
thanks for your review. The trekking profile has an avoid_unsafe
, but it is located a little bit down.
- override param in 'RoutingEngineTest' paramTrack comes out as paramTrack0 (first alternative, can't say why the first test was against paramTrack1, don't see a second call)
- override param in 'RouteServerTest' also works for me ('avoid_unsafe=0' brings a different results)
@zod I made new tests to get this. First I found no paramTrack0.gpx in my local repo. Must have dropped it But it is needed to build the paramTrack1.gpx Then I generated the route with old and new profile.
First one is the old situation - blue the unsafe route, red the safe result and green the test route from repo Second one is new - blue unsafe, red the safe route. As you can see the new safe route is near to the test route. That seems to be the reason that there is no paramTrack1.gpx as result
What to do: I would like to replace the old test route0 with the unsafe route. Both profiles produce it in the same way. What do you think?
This PR adds the following without comment:
-
railwaypenalty
(which will affect routing) -
barrierpenalty
(which will affect routing) -
uphillcost
penalty of 80 (which will affect routing) -
consider_traffic
logic that does not fit a trekking profile -
surface=compacted
toisunpaved
(which will affect routing) -
downhillcutoff
andupillcutoff
logic that avoids steep inclines -
avoid_path
we previously agreed on to not add: https://github.com/abrensch/brouter/pull/593 -
smoothnesspenalty
we previously agreed on to not add: https://github.com/abrensch/brouter/pull/58
removes the following without comment:
-
stick_to_cycleroutes
logic
changes the following without comment:
-
ignore_cycleroutes
fromfalse
totrue
-
tracktype
penalty weights
And introduces a new logic for subtracting costfactors that can result in the costfactor becoming less than 1, which not only has the potential to completely destroy routing, but also contradicts what is currently being discussed here: https://github.com/abrensch/brouter/issues/610.
If this is merged, I will end the collaboration.
This PR [ adds | removes | changes ] the following without comment
It's hardly the first time this has been criticized, yet nothing changed.
If this is merged, I will end the collaboration.
FWIW, I already decided some time ago to stop caring about and commenting on such PRs, so do not mistake silence for agreement with any such PR. I suspect other contributors quit or reduced their involvement out of frustration how things are run here, too.
Hello ! Yes, I suggested this profile version to replace the current "trekking", the PR is a kind of a misunderstanding with afischerdev. Anyway, (perhaps better so!) we have now first reactions to the proposed changes, so that a real discussion on the parameters could start?
My first goal was to change the default value for "ignore_cycleroute":
see https://github.com/abrensch/brouter/issues/622
the further changes are not so critical, but as example not considering surface=compacted in the same way as fine_gravel is for me a severe bug.
Yes, I accept the critic that no documentation was provided, thank Quelnix for your list!
I suggest to start (or better to continue) the discussion with https://github.com/abrensch/brouter/issues/622 Till now I am missing arguments to not change the default value?
I hope we can prepare together a solution to this first parameter and go on with the others.
Regards
@zod Added a new paramTrack file and went back to old test. But for me it is still more a test for the generation of a 2nd route then a test for a param change.
@quaelnix You're right, a detailed explanation would have been good. When I started I was still unsure whether I should create an issue or a PR. But I decided to do a PR because we already have enough issues about it that end without a result. But I didn't ask for any further information about it. Sorry.
From my point of view, it would be good if we have a person responsible for the profiles who would bring the issues together. That's why I had the feeling that no one felt responsible for it and nothing was happening. I think it was good that @EssBee59 had the courage to create a new version of the trekking profile.
With a maintainer we could also go back to the normal way, create an issue and a PR for a result.
If this is merged, I will end the collaboration.
Hard words, but understandable. I would miss you. We need a corrective, someone who will point out the critical points and encourage changes. But I know that is not always easy going.
With a maintainer we could also go back to the normal way, create an issue and a PR for a result.
Yes, with a maintainer defect #622 would probably not hang for weeks!!!
It had be better to start with that, but now I found time to document the changes suggested in this PR profile:
1-cycle_route
Not every part of a cycle-route is a "safe harbor": Constrains exist to create a cycle-route, as example
- it should start and end in the center of a town
- big groupes of cyclist are expected, so that it has to avoid narrow pathes or cycleways!
Statistics from Hesse-Germany show as example, that a big part of the OSM primary (10%) and secondary (20%) ways are used in these cycle-routes, most of them without any cycleway=lane!
In the trekking profile (with the default parameters) these ways get a lower cost as any other ways (track, cycleway etc...), so that the resulting routing is not safe and do not reflect what the Brouter can do!
2-surface=compacted
Fine_gravel ==> cost 1 compacted ==> cost 2
But compacted (quality just below asphalt!) is better than fine_gravel
The profile make a very limited usage of the surface tag ... assign ispaved = surface=paved|asphalt|concrete|paving_stones|sett ... assign isunpaved = not or surface= or ispaved surface=fine_gravel|cobblestone
3-uphillcost penalty
The current implementation in the trekking:
assign downhillcost = 60 # %downhillcost% | Cost for going downhill | number assign downhillcutoff = 1.5 # %downhillcutoff% | Gradients below this value in percents are not counted. | number assign uphillcost = 0 # %uphillcost% | Cost for going uphill | number assign uphillcutoff = 1.5 # %uphillcutoff% | Gradients below this value in percents are not counted. | number
assign downhillcost = if consider_elevation then downhillcost else 0 assign uphillcost = if consider_elevation then uphillcost else 0
With the default value "consider_elevation=true", uphillcost remains =0, and this is a very poor usage of the "elevation" fonction as supported by the Brouter.
example 2 routes are possible to the same destination:
Route A: 20 km with uphill factor of 7.5
- 100 km with a downhill factor of 1.5
Alternativ Route B: 125 Km with uphill=downhill=0
The user would certainly prefer "Route B" as he choosed "consider_elevation".... but Brouter will suggest "Route A" because no elevation costs are added during his calculation. (because uphillcost=0 and donwhillcutoff = 1.5)
4- downhillcutoff and upillcutoff logic that avoids steep inclines
Also when "consider_elevation" is changed to "false", bikers are not able to use ways with very steep inclines. To avoid such ways, a minor change is possible:
assign downhillcost_ = 60 # %downhillcost_% | Cost for going downhill | number assign downhillcutoff = switch consider_elevation 1.5 10 assign uphillcost_ = 80 # %uphillcost_% | Cost for going uphill | number assign uphillcutoff = switch consider_elevation 1.5 10
5- consider_traffic
Default value is false, so that the traffic do not impact the routing. The pseudo-tag "estimatd_traffic_class" is now available world-wide for primary, secondary and tertiary highways, it should be considered for biker routing. (impact can be changed with the parameter, current setting "very low traffic" can be discussed)
6- smoothnesspenalty
The surface tag is important, but the surface quality can very different between 2 highways with the same type and surface:
- surface is gravel (tracktype2), but very very bad quality ...or excellent
- surface of a cycleway is asphalt, but with smoothness=horrible ... and some monthes later after renovation the smoothness get "excellent"
When available, the smoothness is a good help!
Remark: When the tag is considered in the profile, then the users have the posibility by need to add or change its value in OSM!!! So, it impacts positively not only the own routing, but it is also a benefit for all OSM users.
7- railway and barrier penalties
OSM delivers detailed tags about railways and barriers, why not to add penalties when the cyclist have to pass them?
assign railwaypenalty switch railway=level_crossing 155 0
Note: each rail-track is tagged, so the penalty depends on the number of rail-tracks to be crossed-and probably on the number of trains crossing that place per hour/day.
assign barrierpenalty switch barrier= 0 switch barrier=block|bollard 140
Note: Barriers not only cost time, the risk of accidents increases.
Here a new version containing these changes: trekking_v11.brf.txt
But weaknesses remain (a.e. surface tag), and the maintenance remains difficult. Better would be a new, well structured, easy to maintain profile!
Regards
@zod Added a new paramTrack file and went back to old test. But for me it is still more a test for the generation of a 2nd route then a test for a param change.
I agree that the tests is far from ideal, but unfortunately I didn't find a better way to test if param overriding works. BRouter only creates the 2nd GPX file when the track differs from the existing track. If you would pass avoid_unsafe=0
in the overrideParam test it would fail.
@zod You don't have a better idea either. And if we test both aspects, it's ok too.
Yes, with a maintainer defect https://github.com/abrensch/brouter/issues/622 would probably not hang for weeks!!!
Your assumption of being smarter than the person who created the trekking profile leads to ill-conceived changes and I am simply not going to accept them. The logic you are trying to remove does exactly the right thing despite map errors.
I have already proven that the alleged routing error in the first example you gave in this issue ticket is caused by incorrect map data, and here is the proof that it is no different in the second example:
Source: https://www.google.de/maps/@49.9787262,8.5809106,3a,60.3y,274.65h,88.89t/data=!3m6!1e1!3m4!1s79wZe-swSnTwfujOZnYEbw!2e0!7i16384!8i8192?entry=ttu
@quaelnix,
thanks for the kind words! It takes too much time for me to read comments that are confusing or completely out of place. From now on I will simply ignore your posts.
@ other participants of this PR:
in the first example Quelnix suspects a mapping error: the cycle_route would not use the secondary hw as mapped, but the parallel path! (mapping error)
No proof is delivered for this! I rather think, the route was planed for groups of cyclists biking together "sunday mornings" where the traffic remains low. And the path is this case probably not prefered to the secondary hw. (avoid path for groups)
in the second example Quaelnix delivers a picture taken nearly 1 km outside the route, strange for a proof! Within the critical route part, no path exists parallel to the primary hw.
Is it possible, that 10 % of the primary and 20% of the secondary hw in Hess are related BY ERRORS to cycle_route?
Let us stay by facts. Please.
Routing is very individual and many answers are correct.
Change that appear to be critical:
- assign ignore_cycleroutes = false # %ignore_cycleroutes% | Set true for better elevation results | boolean
+ assign ignore_cycleroutes = true # %ignore_cycleroutes% | Set to false to favor the segments on cycleroutes | boolean
At first glance I would say it doesn't matter what the starting value is. But cycle routes can present a few problems for routing:
- as shown in #622, routes can run on roads even though there is a bike path. This doesn't have to be a mistake, routes can be designed for tourist reasons or for sporting reasons (I don't know if there are any rules for that, didn't find one).
- the route relations is not always adjusted if access=no, highway=construction or something similar is suddenly set on a way. There are some issues in that direction
- more personally: normally, when I see a cycle path is available, but the route continues on the road, I decide to use the cycleway. But 'intelligent' apps with re-routing push me back onto the road. Annoying and with the potential to miss the right descent.
@EssBee59
- Please tell us how did you find the errors on Hessen data?
- When I copy the new profile to BRouter-Web profile box and select
ignore_cycleroutes=false
, it didn't follow the route (using first sample of #622).
May be a feature 'follow cycleroute' could help for trekking profile that follows the direction of a route but uses the saver way. When I use the original profile with ignore_cycleroutes=false
and avoid_unsafe=true
, route doesn't change.
From now on I will simply ignore your posts.
We can reopen this pull request once you change your mind.
No proof is delivered for this!
Within the critical route part, no path exists parallel to the primary hw.
Within the "critical route part" the speed limit of primary highway is 50 km/h or less and your alternative looks like this:
Source: https://www.google.de/maps/@49.9880073,8.5751062,3a,75y,311.2h,80.22t/data=!3m6!1e1!3m4!1seaC8bC_-mAKskLhcHiBTNQ!2e0!7i16384!8i8192?entry=ttu
Here we are again.
@quaelnix I don't think it's a good action to just close. As your pictures show, action is needed.
We can
- enable
avoid_unsave
to work in the original profile - in @EssBee59 's suggestion recall to follow the routes
Or maybe someone else would like to contribute a completely new trekking profile from their pool.
I don't think it's a good action to just close.
None of the proposed changes is acceptable in its current form and as long as @EssBee59 is neither willing to properly test his changes nor to discuss them, this pull request is not going to happen. His behavior is completely unacceptable. Period.
enable avoid_unsave to work in the original profile
Yes, of course we can discuss if there are ways to improve the avoid_unsafe
switch. But this has nothing to do with this pull request. And by the way a highway=primary
section with maxspeed=40
, right of way, asphalt in pristine condition and basically no hgv traffic is not unsafe.
As your pictures show, action is needed.
Yes, someone needs to put the route relation were it actually belongs and properly tag the path that has not been touched in 10 years and make this path the combined foot and cycleway it actually is.
Hello afficherdev,
- Please tell us how did you find the errors on Hessen data?
The statistics were created using overpass turbo http://overpass-turbo.eu/
here some examples: overpass.txt
A further example...
http://brouter.de/brouter-web/#map=9/51.0552/13.7219/osm-mapnik-german_style&lonlats=8.76663,50.017366;13.725973,51.044692
I tested with the current profile, the current profile + ignore_Cycleway=true and with the trekking_v11 including the changes suggested above
Even with this relaxed query: https://overpass-turbo.eu/s/1zDU, the rate of false positives is extremly high:
- B3: https://www.openstreetmap.org/way/364301602 (
cycleway:both = no
) - Parallel cycleway: https://www.openstreetmap.org/way/131292244 (
bicycle=designated
)
-> Map is wrong.
- L 3095: https://www.openstreetmap.org/way/164331153 (no cycleway tag)
- Parallel cycleway: https://www.openstreetmap.org/way/34607627 (
bicycle=designated
)
-> Map is incomplete. Route relation again on the wrong highway ...
I guess we don't have to talk about how the situation will be in other countries, where the density of mappers is much much lower than in Germany and where the trekking profile is also supposed to produce artefact free routes that will not get you stuck on paths where you have to carry your bike ...
assign smoothnesspenalty =
switch smoothness=intermediate 0
switch smoothness=bad 0.5
switch smoothness=very_bad 1
switch smoothness=horrible 3
0
The result of this will be that you downgrade ways which are tagged as smoothness=bad
(of which almost all are perfectly fine for trekking bikes) and instead favor any of the ~ 97 % alternative ways that do not carry any smoothness tag.
This is plain stupid and results in a near 100 % failure rate:
- Example 1: https://brouter.de/brouter-web/#map=16/50.0010/8.6288/standard&lonlats=8.614228,50.005942;8.634147,49.997639
- Example 2: https://brouter.de/brouter-web/#map=15/49.9305/8.7438/standard&lonlats=8.748474,49.925224;8.745916,49.938303
- Example 3: https://brouter.de/brouter-web/#map=16/49.8941/8.6598/standard&lonlats=8.665535,49.897755;8.659773,49.892833
but as example not considering surface=compacted in the same way as fine_gravel is for me a severe bug.
Another example of a gross misunderstanding of how the trekking profile works. Adding compacted
to this exclusion list actually removes such ways from the list of unpaved ways, which throws off the tracktype cost logic and results in chosing:
-
tracktype=grade3; surface=compacted
overtracktype=grade2
: https://brouter.de/brouter-web/#map=16/49.8394/8.6762/standard&lonlats=8.672164,49.839311;8.681874,49.841174 -
tracktype=grade4; surface=compacted
overtracktype=grade2
: https://brouter.de/brouter-web/#map=17/49.84359/8.69299/standard&lonlats=8.69299,49.844662;8.69378,49.841263
@quaelnix
...as long as @EssBee59 is neither willing to properly test his changes nor to discuss them...
I think it's sentences like this that make @EssBee59 a little more reserved:
Your assumption of being smarter than the person who created the trekking profile leads to ill-conceived changes...
Anyway, thanks for your samples, good basis for a discussion.
- Map errors Yes, we don't have to talk about that. But we need a few considerations about how we get from the street to the cycleway. As remark only: In the first example I didn't find any route networks And in the second example, both have this information.
- smoothnesspenalty
Yes, I've seen the table for that and
bad
is good enough for trekking bike. - compacted
Yes, that breaks the
tracktype
. But alsocobblestone
as unpaved is not really understandable when I seesett
as parved. (present in both profiles)
I think it's sentences like this that make @EssBee59 a little more reserved:
His complete inability to follow arguments is not new and now it is enough. Not to mention this .txt mess, broken markdown formatting in basically every post and the aparrent refusal to open his own pull requests. Again, I am not going to accept this anymore.
Yes, we don't have to talk about that.
We need to talk about this because the design goal of the trekking profile was (and still is, in my opinion) to make it resistant to map errors.
In the first example I didn't find any route networks
What do you mean by that? The relation in question does carry a network=lcn
tag.
But also cobblestone as unpaved is not really understandable
cobblestone
is excluded from the list of unpaved roads!
@quaelnix
Yes, we don't have to talk about that.
We need to talk about this ...
This quote has other context, Original it was:
- Map errors Yes, we don't have to talk about that.
And first sample of your map error
post is:
And I've found: it is member of the route Radnetz Stadt Darmstadt
cobblestone is excluded from the list of unpaved roads!
Yes, my fail - short reading only. But then remark is the other way round, why cobblestone and fine_gravel are not in ispaved collection. Historical I guess. But I'm digressing from the topic.
And a last remark on compacted
. I found this
Best sort of ways below paving with asphalt, concrete, paving stones.
Seems to be better in surface hierarchy then fine gravel.
His complete inability to follow arguments is not new and now it is enough. Not to mention this .txt mess, broken markdown formatting in basically every post and the aparrent refusal to open his own pull requests. Again, I am not going to accept this anymore.
Not everyone is born with developer skills. And I think we have to accept collaborators who are not that equipped. And not to push them out. They often come from an interested user community and contribute here the discussion of the scene. Our discussions are not primarily user driven. That's okay, we can do important things of our own. But it also makes us forget the user needs we have. E.g. if you take the example route Frankfurt - Dresden, around 500 km, it takes a lot of time to calculate. And I am willing to make the technical transfer for @EssBee59 if needed. I haven't been able to do this so well in the past.
This quote has other context
Yes, sorry, my bad.
Historical I guess.
No, the (almost exclusive) purpose of these two variables is to feed the probablyGood
logic, that can be considered a whitelist of ways that are likely kind of decent.
If you start feeding this whitelist with things that might not be good, you're taking it ad absurdum and end up with the routing artefacts I showed above.
You simply cannot look at one line of a profile and say this looks incorrect, lets change it. Doing it like that will almost always fail horribly.
And I think we have to accept collaborators who are not that equipped.
To be honest, I don't know what to say anymore. If by now @EssBee59 is still not able to grasp the idea behind the cycleroute logic, then yes, maybe this is the end of the discussion. One last example (from: https://github.com/abrensch/brouter/issues/479):
- https://brouter.de/brouter-web/#map=14/70.0685/24.9132/standard,Waymarked_Trails-Cycling&lonlats=24.934874,70.058619;24.917142,70.080058
E.g. if you take the example route Frankfurt - Dresden, around 500 km, it takes a lot of time to calculate.
I agree that we should try our best to keep the performance of the trekking profile as high as possible.
I added a little test for the variables inside a profile.
And a minimal profile with the logic on our discussion: probablyGood
is defined as true also when we have no surface definition. Because some said this is a cycle way.
But if we have an undefined value (e.g bricks, is added in the next generation lookups.dat) isunpaved becomes true.
You could use this for short results:
$ gradlew brouter-expressions:test