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

Elevation Calculation Ignores Tunnel #685

Closed
1papaya opened this issue Mar 24, 2020 · 17 comments · Fixed by #750
Closed

Elevation Calculation Ignores Tunnel #685

1papaya opened this issue Mar 24, 2020 · 17 comments · Fixed by #750
Assignees
Labels
awaiting release ⏳ bug 🐞 Erroneous behavior of the backend
Milestone

Comments

@1papaya
Copy link

1papaya commented Mar 24, 2020

Here's what I did

Computed a car route which includes a tunnel that goes through a gigantic mountain:

http://maps.openrouteservice.org/directions?n1=27.950284&n2=-15.77744&n3=13&a=28.102968,-15.706137,27.981062,-15.780659&b=0&c=0&k1=en-US&k2=km


Here's what I got

The elevation profile returned includes the elevation being calculated as if the road goes up and over the mountain, like so:

ors-screenshot


Here's what I was expecting

The elevation profile ought keep the elevation constant while the car is going through the tunnel. Or, even interpolate the elevation on one side of the tunnel to the other, if the tunnel has an elevation change going through it.


Here's what I think could be improved

Great service!! Very well done!! :)

@nilsnolde
Copy link
Contributor

Yeah, that's unfortunately still true. Graphhopper fixed that a while ago:
graphhopper/graphhopper#798

Probably a part of the code we're not using for ORS. Any thoughts backend devs?

@nilsnolde
Copy link
Contributor

@aoles
Copy link
Member

aoles commented Mar 31, 2020

Actually, I don't see any problem here. In fact, the elevation profile for the the tunnel section consists of a straight light connecting both ends of the tunnel located at 611 and 363 meters, respectively.

image

@HendrikLeuschner
Copy link
Collaborator

@aoles You are underestimating the length of that tunnel. It does not start where the straight segment begins, but instead already in the bent part (see sattelite maps to check, https://www.google.com/maps/place/Gran+Canaria/@28.0331134,-15.7559019,2595m/data=!3m1!1e3!4m5!3m4!1s0xc40855504bf07c1:0x2ec916c8a5acdb16!8m2!3d27.9202202!4d-15.5474373 )
I think the complaint is a valid one. It is probably like this because the elevation profile does not take road category into account and instead just gets the data for each point along the route. No idea what GH have done about this.

@aoles
Copy link
Member

aoles commented Mar 31, 2020

Oh, right! Thanks for pointing this out @HendrikLeuschner.

@aoles
Copy link
Member

aoles commented Mar 31, 2020

I think the problem is that we actually don't use GH's code to do the interpolation.

@aoles aoles added bug 🐞 Erroneous behavior of the backend and removed investigate 🔍 labels Mar 31, 2020
@aoles aoles self-assigned this Mar 31, 2020
@1papaya
Copy link
Author

1papaya commented Apr 3, 2020

The tunnel is broken up into three segments. I think what's wrong is the elevation is sampled at the beginnings/ends of each of the segments as opposed to the beginning/end of the whole tunnel.

@aoles
Copy link
Member

aoles commented Apr 4, 2020

Thanks @1papaya for sharing your observations! Actually, the splitting into segments occurs only when traveling from South to North: 490956948 & 350064679. In the other direction (such as in your example) the tunnel is represented by a single way.

@aoles
Copy link
Member

aoles commented Apr 4, 2020

It turns out that the interpolation is not enabled because of a missing GH configuration parameter graph.encoded_values: road_environment. Without the interpolation the surface elevation is used:

image

The interpolated elevation profile:

image

@HendrikLeuschner
Copy link
Collaborator

Interesting. So we are just missing a parameter I guess? Should be easy to fix by @rabidllama or someone

@aoles
Copy link
Member

aoles commented Apr 6, 2020

I'm happy to fix it, but if someone else would like to take it from here I don't mind handing it over either. Just change the issue assignment, thanks!

@HendrikLeuschner
Copy link
Collaborator

Just meant that @rabidllama is probably the one to put it into our live servers. But he probably needs some input first.

@aoles
Copy link
Member

aoles commented Apr 6, 2020

Thanks @HendrikLeuschner !

As far as I can tell this is not just about adding a missing parameter to the configuration file but rather requires changes to the backend as well.

Correct me if I'm wrong, but any GraphHopper-specific configuration parameters need to be explicitly mapped from the app.config settings to the argument list passed to GH. Currently there doesn't seem to be any code to handle thegraph.encoded_values, at least I had to add a line to RoutingProfile in order to get this to work.

Besides, enabling the interpolation requires rebuilding the graphs, so it needs to go through a proper release cycle anyway.

@naalang
Copy link

naalang commented Apr 30, 2020

Also at bridges in the mountains the calculations are not correct.
If the bridge is nearly horizontal, the calculated elevation is based on the valley.

@aoles
Copy link
Member

aoles commented May 1, 2020

Thank you @naalang for the additional insight. Elevation for tunnels and bridges is currently not properly resolved because both need interpolation being turned on which is not the case currently. I'm sorry this has been an issue for so long now, somehow it went out of our radar after you reported it for the first time. I will make sure this is fixed ASAP.

@HendrikLeuschner
Copy link
Collaborator

Might also be due to the spatial uncertainty of the data, at least for bridges I can see this resulting in the valley's elevation being used.

@aoles
Copy link
Member

aoles commented May 4, 2020

The problem is not so prominent for bridges probably because most of the time they are single line segments without any intermediate nodes, so there is not really anything to interpolate.

But occasionally it might make quite a difference, for example in the case of the Millau Viaduct.
image

@takb takb added this to the 6.2 milestone Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting release ⏳ bug 🐞 Erroneous behavior of the backend
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants