Add "DIVIDE" command and new "maxslope" and "maxslopecost" parameters

[simdens]

To extend the functionality an additional "DIV" command and two additional parameters are defined. An example instance can be tested under Brouter-Web Dev Instance with fastbike-longdistance.brf profile.

Reasoning

"DIV" command: The profile can compute constants within the global context based on physical values like systemweight or nominal power.

"downhillmaxbuffercost" and "uphillmaxbuffercost" These parameters allow to increase the uphill or downhill penalty if the buffer exceeds the maximum buffer value. This makes it possible to increase the penalty for very step hills. i.e. for uphills which are too steep for the given gear ratio. See the fastbike-longdistance.brf profile for reference.

[simdens]

This merge request partially solves #617 as it introduces the possibility of two different elevation penalties depending on the slope of the road.

[quaelnix]

I have also missed a 'div' operator a few times myself, but I assumed it was left out on purpose, so I would like to get @abrensch's opinion on this.

As for the maxbuffercost logic, I'm not yet sure this is the right way to go, but the concept is interesting!

By looking at the way you designed your profile I was wondering if it maybe was enough to also allow the usage of the elevationbufferreduce command in the way context in order to solve https://github.com/abrensch/brouter/issues/617?

[simdens]

Yes, I assume the 'div' operator was left out on purpose for performance reasons. Nevertheless it was not possible to create a physics based profile without the operator. That's why I added it. At least on a server or normal PC, even very long routes are possible.

About the elevationbufferreduce: Can you please further elaborates how that can solve #617 or how that can implement a kind of a slope limit like in my profile?

[afischerdev]

I don't see a performance risk on the div command. But I see the exception problem.

When you add something like this to your profile, it will work without exception.

assign x
  max 1
  add div 1 2
  add div 2 1
  add div 3 0
  add div 0 3
  1

It just produces an infinity variable. But if you use this var you will run into an error. Sample

assign costfactor
  max 1
  add x
  costsum

The result in error log is: Error (linksProcessed=0 open paths: 0): from-position not mapped in existing datafile With a small error handling in BExpression you will get the 'real' result - for instance:

  private float div(float v1, float v2) {
    if (v2 == 0f) throw new IllegalArgumentException("div by zero");
    return v1 / v2;
  }

Result: ParseException .\profiles2\minimal.brf at line 39: div by zero

[quaelnix]

About the elevationbufferreduce: Can you please further elaborates how that can solve https://github.com/abrensch/brouter/issues/617 or how that can implement a kind of a slope limit like in my profile?

I thought about something along the lines of this:

assign elevationmaxbuffer     100
assign elevationbufferreduce  20
assign elevationpenaltybuffer 10

assign downhillcost   0
assign downhillcutoff 2
assign uphillcost     0
assign uphillcutoff   2

...

assign costfactor ...

assign uphillcostfactor   multiply 5 costfactor
assign downhillcostfactor multiply 5 costfactor

Here is a test profile if you want to play around with it. But it sadly does not work as well as I thought it would.

[simdens]

  private float div(float v1, float v2) {
    if (v2 == 0f) throw new IllegalArgumentException("div by zero");
    return v1 / v2;
  }

I have added that error handling. Thanks for suggesting!

assign elevationmaxbuffer     100
assign elevationbufferreduce  20
assign elevationpenaltybuffer 10

assign downhillcost   0
assign downhillcutoff 2
assign uphillcost     0
assign uphillcutoff   2

...

assign costfactor ...

assign uphillcostfactor   multiply 5 costfactor
assign downhillcostfactor multiply 5 costfactor

Very interesting approach. But I do not see any way to (soft) limit the maximum allowed slope of a route. At least to my understanding, one still needs the elevation height of the segment which is not directly available.

[quaelnix]

But I do not see any way to (soft) limit the maximum allowed slope of a route.

You do that by reducing the elevationbufferreduce to the desired maximum allowed slope.

[simdens]

But I do not see any way to (soft) limit the maximum allowed slope of a route.

You do that by reducing the elevationbufferreduce to the desired maximum allowed slope.

I already do that. But to actually limit to a maximum slope, the exceeding elevation must be penalized more heavily.

[quaelnix]

What I was trying to say is that in my test profile (which is designed to work on a regular BRouter instance without the contents of this pull request) you can also use elevationbufferreduce to soft limit the maximum allowed slope.

But to actually limit to a maximum slope, the exceeding elevation must be penalized more heavily.

My test profile does this by increasing the uphillcostfactor like so:

assign uphillcostfactor   multiply 5 costfactor

This merge request partially solves https://github.com/abrensch/brouter/issues/617 as it introduces the possibility of two different elevation penalties depending on the slope of the road.

I wonder if we could add a slope penalty based directly on the incline that we already calculate in StdPath? https://github.com/abrensch/brouter/blob/109782d36225e743138923aa9feecd702171c9bf/brouter-core/src/main/java/btools/router/StdPath.java#L216

[quaelnix]

I've been playing around with it a bit more and I have to say that your code works really well!

At least to my understanding, one still needs the elevation height of the segment which is not directly available.

To my understanding, you don't have to, because you can set elevationmaxbuffer to an arbitrarily large number.

---

I wonder if we could add a slope penalty based directly on the incline that we already calculate in StdPath?

Here's a little hot-rodded code snippet that illustrates the idea: https://github.com/quaelnix/brouter/commit/a00d992b95ea08c84e50414167634ce09fb8dc54

The syntax is straightforward. "maxupslope" and "maxdownslope" define the threshold values for the maximum permissible inclines and declines above which the additional penalties becomes effective. upslopecostfactor and downslopecostfactor optionally allow to adjust the amount of additional penalty and thus the "softness" of the limit:

assign maxupslope 9
assign maxdownslope -15
#assign upslopecostfactor multiply 10 costfactor
#assign downslopecostfactor multiply 10 costfactor

The remaining problem seems to be, that both of our ideas lack the 'fine-grained' part in order to truely solve #617.

[quaelnix]

@simdens, I spent some more time with this pull request and the more I think about it, the more I like it. It is simple, yet very powerful and gets us a fair bit closer to solving https://github.com/abrensch/brouter/issues/617, while sticking to existing concepts and the performance overhead should be reasonably small.

@afischerdev, what do you think about it?

[simdens]

Sorry. Currently on a business trip. Will write some more comments in a week.

[afischerdev]

Looks good to perfekt for me. My test was around here and the profile was reduced to the bare essentials

---context:global   # following code refers to global config

assign   validForBikes        1

#assign maxupslope 9
#assign maxdownslope -15

#assign maxSpeed 35

---context:way   # following code refers to way-tags

assign turncost 0
assign initialcost 0

assign costfactor
  switch not highway=   1
  switch not route=       1
  100000

---context:node  # following code refers to node tags

assign initialcost 0

Results - forward / revers - show what was expected. Blue the route without, red with activated params

Also speed check still works .

[afischerdev]

One more test: Start on a hill with all ways down less then maxdownslope brings a route down anyway.

[afischerdev]

And an addtional idea: bring out the slope values in json format like the times array:

Sample: Add an incline value to message class and add it like this

"slopes": [0.0,-0.4,-1.9,-2.2,-0.1,-1.7,-5.2,-5.9,-6.2,-6.6,-7.5,-7.8,-8.0,-7.4,-6.1,-6.2,-4.9,-7.1,-5.3,-6.7,-9.6]

[simdens]

I've been playing around with it a bit more and I have to say that your code works really well!

At least to my understanding, one still needs the elevation height of the segment which is not directly available.

To my understanding, you don't have to, because you can set elevationmaxbuffer to an arbitrarily large number.

I wonder if we could add a slope penalty based directly on the incline that we already calculate in StdPath?

Here's a little hot-rodded code snippet that illustrates the idea: quaelnix@a00d992

The syntax is straightforward. "maxupslope" and "maxdownslope" define the threshold values for the maximum permissible inclines and declines above which the additional penalties becomes effective. upslopecostfactor and downslopecostfactor optionally allow to adjust the amount of additional penalty and thus the "softness" of the limit:

assign maxupslope 9
assign maxdownslope -15
#assign upslopecostfactor multiply 10 costfactor
#assign downslopecostfactor multiply 10 costfactor

The remaining problem seems to be, that both of our ideas lack the 'fine-grained' part in order to truely solve #617.

I finally understand. Yes, that might be a way to linearly increase cost depending on slope. Simple and easy :-)

And an addtional idea: bring out the slope values in json format like the times array:

Sample: Add an incline value to message class and add it like this

"slopes": [0.0,-0.4,-1.9,-2.2,-0.1,-1.7,-5.2,-5.9,-6.2,-6.6,-7.5,-7.8,-8.0,-7.4,-6.1,-6.2,-4.9,-7.1,-5.3,-6.7,-9.6]

What do you mean with this suggestion? And what is the advantage of this?

[quaelnix]

@simdens, I think the idea is to enrich the json output (sent to e.g. BRouter-Web) with incline values, but I don't think this will work well, as the average incline of a way segment, which can be several kilometers long, is not very informative.

The only remaining issue I have with this pull request is that I dislike the nomenclature. maxbuffercost is misleading in my opinion, but at the same time, I do not have a better suggestion.

[sjakobi]

First off: Thanks a lot for working on a (partial) solution for #617, @simdens! :)

Regarding the new div command and the new parameters, I think it would be great to have some kind of documentation on what they do.

Based on the link to #617 I was under the impression that the div command would somehow make it possible to divide elevation cost definitions into segments for different slopes. ;) I had to look at the sample profile to figure out that it's just basic mathematical division.

(Minor bikeshed: Would it be better to avoid the abbreviation and spell out divide in analogy to the existing multiply function?)

For "downhillmaxbuffercost" and "uphillmaxbuffercost", I think it would be good to see some documentation that includes an annotated example. The documentation could be added to the wiki once the PR has been merged.

[simdens]

Agree, documentation is missing. I will include an updated profile_developers_guide.md to the pull request ASAP.

About the naming:

• "div": I have chosen the name according to the scheme for addition ("add") and subtraction ("sub"). But yes, multiplication ("multiply") does not use the short form. So at least for me it seems to be more consistent to go for "divide" instead of "div".
• "uphillmaxbuffercost" and "uphillmaxbuffercost": I don't like the names either. But every more precice name I came up was even longer. I thout about "uphillmaxbufferexceedcost" :see_no_evil: A bit more clear but even longer. Perhaps we start a little brainstorming (same for downhill parameter):
  • "uphillmaxbufferexceedcost"
  • "uphillbufferexceedcost" (currently my favorite)
  • "uphillbufexcost"

@quaelnix What are your thoughts about the naming? I'd like to agree first here before changing in the code.

[simdens]

@quaelnix About your commit 6a84490: I like the new clean code style! Well done! Nevertheless, I would prefer some more (ok, to be honest, a lot more) comments to make it easier to understand the code. I always find it hard to understand even my own code after some time and thus add a lot of comments everywhere in the code. Until now, that helped me a lot to understand the code faster when maintaining it.

[quaelnix]

So at least for me it seems to be more consistent to go for "divide" instead of "div".

I like it.

A bit more clear but even longer.

The problem is that both uphillcost and uphillmaxbuffercost penalize elevation that exceeds the elevationmaxbuffer:

• uphillcost is the costfactor for "excess elevation" below the elevationbufferreduce induced threshold
• uphillmaxbuffercost is the costfactor for "excess elevation" above the elevationbufferreduce induced threshold

The split point is not the size of the buffer, but the slope, which is implicitly defined by the bufferreduce logic.

I would prefer some more (ok, to be honest, a lot more) comments to make it easier to understand the code.

Ok, but wouldn't it make more sense in this case to make the variable names more meaningful instead?

excess = ehbd - rc.elevationmaxbuffer;          // elevation which excesses the maxbuffer threshold

->

buffer_excess = elevation_hysteresis_buffer_downhill - rc.elevationmaxbuffer; 

Additionally, when reaching elevationmaxbuffer, uphillcostfactor/ downhillcostfactor fully replaces costfactor in way cost calculation if those cost factors are defined.

This is true yet misleading. The elevationmaxbuffer does not affect the costfactor logic. Try:

• Global context: assign elevationmaxbuffer 1000
• Way context: assign uphillcostfactor 10

[quaelnix]

It would probably make more sense to introduce two new commands (uphillmaxslope and downhillmaxslope) in the way context instead of reusing the elevationbufferreduce command for something it was never intended for:

• uphillmaxbuffercost -> uphillmaxslopecost
• downhillmaxbuffercost -> downhillmaxslopecost

[quaelnix]

@simdens, here is a draft containing my suggested changes: https://github.com/abrensch/brouter/compare/master...quaelnix:brouter:draft2-slope-penalty#diff-59799a4a78f59692f35951f94cd8733f7e34718c2d60a6248685159f637517a4, please let me know what you think once you had time to take a look at it.

[afischerdev]

@quaelnix Thanks for the new version. I played a little bit around with it. But my results are less effective than in your first version. Again I add a list of the incline values to the output and can see that this still have values above/below the limits.

[quaelnix]

Those limits are soft limits and the level of softness depends on the actual cost values you set. If you add this:

assign uphillmaxslope 8
assign uphillmaxslopecost 800
assign downhillmaxslope 8
assign downhillmaxslopecost 800

Slopes of more than 8% are are going to be unlikely to occur on the route.

---

However there are two more things that need to be thought of:

1. If and how to include the new costdividers into the definitlyWorseThan logic (might not be needed at all)
2. Whether or not we should automatically subtract the uphillcutoff / dowhillcutoff from the maxslope like this: https://github.com/simdens/brouter_profile/blob/master/fastbike-longdistance.brf#L103

[afischerdev]

@quaelnix I've tested new with the extra parameter uphillmaxslopecost anddownhillmaxslopecost and get same result as on my first test. Merge now?

[quaelnix]

@afischerdev, yes, I think it will make a lot of users very happy, and if we see the need for it, we can always expand it later.

We should probably use 'Squash and merge' to compress all the minor changes into a single commit that is properly attributed to @simdens who came up with this awesome idea.

[simdens]

Now had the time to test the current version. Looks good to me. Even though the downhillmaxslope and uphillmaxslope parameters appear a bit redundant to elevationbufferreduce. Can not think of any situation, were one sets elevationbufferreduce with a different value than the other two.

A small typo is still in profile_developers_guide.md: In the Operant list is still div instead of the correct divide command. I commited the fix to my dev branch.

[quaelnix]

The idea behind this was that you can now set the thresholds in the way context separately for uphill and downhill depending on the properties of the way. You can now create mountain bike profiles with like 4 lines of code where the profile automatically selects good (boring) paths for climbing up the hill and bad (fun) paths for racing down the hill.

[simdens]

Ah. Understand.

Sounds like a great idea. I have to think about the possibilities and may have to extend my profile :-D

[simdens]

If someone is interested about the kind of profiles possible with this change:

• Profile
• Brouter-Web with fastbike-longdistance profile

Thanks a lot for your work to make this possible.

[quaelnix]

I am still wondering if we should rename uphillmaxslopecost into something like uphillcostabovemaxslope and downhillmaxslopecost into something like downhillcostbelowmaxslope. The new parameter names would be truely ugly, but at the same time they would tell you exactly what they do. What do you think @simdens.

[quaelnix]

If someone is interested about the kind of profiles possible with this change:

Your profile contains some sexy shit! I'll definitely have to take a closer look at it!

PS: BRouter expects S_C_x to be the drag coefficient times reference area (m^2) times half air density (kg/m^3).

[simdens]

PS: BRouter expects S_C_x to be the drag coefficient times reference area (m^2) times half air density (kg/m^3).

Thanks for the hint! I've fixed it :-)

[simdens]

I am still wondering if we should rename uphillmaxslopecost into something like uphillcostabovemaxslope and downhillmaxslopecost into something like downhillcostbelowmaxslope. The new parameter names would be truely ugly, but at the same time they would tell you exactly what they do. What do you think @simdens.

I understand your concerns. But I don't like very long names for parameters and I do not have the creativity to find a better short one. To be honest, I'd go for the current parameter names.

[simdens]

With the changes from this merge request, for me it is not possible to change the downhillmaxslope variable in the way context:

Is there any other merge request I have to include into my brouter instance?

[quaelnix]

The downhillmaxslopecost variable will be readonly in the way context if you have already defined it in the global context.

[simdens]

:see_no_evil: yes. I just noticed in that second. Sorry for my stupid question and thanks for your very quick answer!

[quaelnix]

No worries, this also caught me in surprise a while ago.

[simdens]

Finally included the speed limitation to the "Long Distance" profile thanks to the configurable downhillmaxslope :-) Great idea, thanks!

[LupinSun]

Hi everyone, these "slope" parameters are already present in locus and Brouter, are they still under development or will they never be implemented, because they would be perfect for my needs.

[quaelnix]

@LupinSun, you can try them out at: https://brouter.de/brouter-test/

PS: The test area is currently limited to Germany and there are no pseudo tags on this server.

[LupinSun]

@LupinSun, you can try them out at: https://brouter.de/brouter-test/

PS: The test area is currently limited to Germany and there are no pseudo tags on this server.

Thank you, I will wait for the test to be extended to Italy. Where can I find news about developments and changelogs so that I can be aware of new features and future developments?

[afischerdev]

@LupinSun Sorry for the delay, I was away for a while. And you are right, we haven't a high rate in updates. I'll start with the process.

May be meanwhile the test for 1asec elevation #644 could stop and the test server could use standard data pool?

[quaelnix]

May be meanwhile the https://github.com/abrensch/brouter/pull/644#issuecomment-1978576954 for 1asec elevation https://github.com/abrensch/brouter/pull/644 could stop and the test server could use standard data pool?

Sounds reasonable to me.

[afischerdev]

@quaelnix Fine, thanks, I restarted the testserver with last lib and standard data pool