Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update lib part four - recalculation at end of track #498

Merged
merged 5 commits into from
Jan 20, 2023

Conversation

afischerdev
Copy link
Collaborator

The calculation for voice hints and speed profile is moved to the end of track generation.
This only effects tracks with more then two points.
And a recalculation is done for elevation data and track distances.

The output at the moment is not fully correct - the routines for voice hints will follow.
The main discussion is found in #441.

@afischerdev afischerdev temporarily deployed to BRouter January 16, 2023 10:37 — with GitHub Actions Inactive
@afischerdev afischerdev merged commit 2387513 into abrensch:master Jan 20, 2023
@quaelnix
Copy link
Collaborator

I can tell you from experience that if you continue to update the library in large and messy chunks, you are opening Pandora's box. The old rule "untested code is broken code" almost always applies.

This pull request broke at least two things:

  1. The ascent no longer increases monotonically with distance*
  2. The travel time computation can be off by at least up to 3 %

The bugs might not be related with each other, since the calculated travel time decreases even though the total ascent increases. While the total distance does not change.

*) If route A is a subset of route B then the total ascent of route B must be greater than or equal to the total ascent of route A. Example:

Route A Route B
Total ascent with 2387513 25 m 15 m
Total ascent with d081e5e 25 m 25 m

@quaelnix
Copy link
Collaborator

quaelnix commented Jan 24, 2023

The first bug is caused by now passing the deltas through the elevation filter:

if (ehb > 10.) {
ascend += ehb - 10.;
ehb = 10.;
} else if (ehb < 0.) {
ehb = 0.;
}

in reverse order.


The old (correct) behavior can be restored by iterating through the for loop in reverse:

diff --git a/brouter-core/src/main/java/btools/router/RoutingEngine.java b/brouter-core/src/main/java/btools/router/RoutingEngine.java
index d69a11d..05fc9b3 100644
--- a/brouter-core/src/main/java/btools/router/RoutingEngine.java
+++ b/brouter-core/src/main/java/btools/router/RoutingEngine.java
@@ -713,7 +713,7 @@ public class RoutingEngine extends Thread {
     short ele_end = Short.MIN_VALUE;
     double eleFactor = routingContext.inverseRouting ? 0.25 : -0.25;
 
-    for (i = 0; i < ourSize; i++) {
+    for (i = ourSize - 1; i > 0; i--) {
       OsmPathElement n = t.nodes.get(i);
       if (n.message == null) n.message = new MessageData();
       OsmPathElement nLast = null;

or by adjusting the elevation filter as follows:

diff --git a/brouter-core/src/main/java/btools/router/RoutingEngine.java b/brouter-core/src/main/java/btools/router/RoutingEngine.java
index d69a11d..241d4a1 100644
--- a/brouter-core/src/main/java/btools/router/RoutingEngine.java
+++ b/brouter-core/src/main/java/btools/router/RoutingEngine.java
@@ -772,16 +772,16 @@ public class RoutingEngine extends Thread {
         if (ele_last != Short.MIN_VALUE) {
           ehb = ehb + (ele_last - ele) * eleFactor;
         }
-        if (ehb > 10.) {
-          ascend += ehb - 10.;
-          ehb = 10.;
-        } else if (ehb < 0.) {
-          ehb = 0.;
+        if (ehb > 0) {
+          ascend += ehb;
+          ehb = 0;
+        } else if (ehb < -10) {
+          ehb = -10;
         }
       }
 
     }
-    ascend += ehb;
+    ascend += Math.max(0, ehb);
 
     t.ascend = (int) ascend;
     t.plainAscend = (int) ((ele_start - ele_end) * eleFactor + 0.5);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants