search

Project-OSRM / osrm-backend

Open Source Routing Machine - C++ backend
http://map.project-osrm.org
BSD 2-Clause "Simplified" License
6.41k stars 3.39k forks source link

maxspeed on road can lead to OSRM exceeding configured vehicle maxspeeds #6979

Open woodpeck opened 4 months ago

woodpeck commented 4 months ago

Issue

When working with the "truck-soft" profile I noticed that even when I changed the profile to have a truck go really slow on motorways, this would often not have any impact and the average speed in the computed result would be higher than my configured maximum.

I found out that this bug exists in the standard car.lua as well, it's just that it doesn't become apparent because speed limits tend to be below the configured car values. However, if some joker were to tag some unlimited motorways with a maxspeed of 9999999 it would quickly show implausible results.

In my opinion the root of the bug is https://github.com/Project-OSRM/osrm-backend/blob/master/profiles/lib/way_handlers.lua#L440-L446 where a maxspeed on the road unconditionally overwrites anything that was there before.

There is a line in car.lua that pulls in the limit function from lib/maxspeed.lua (https://github.com/Project-OSRM/osrm-backend/blob/master/profiles/car.lua#L11) and it appears that at some point this limit function was supposed to cap the maxspeed at the smaller of two values, however this function is then never used in car.lua. I have a hunch that somehow this functionality has been lost in some refactoring.

Steps to reproduce

Set all road class based speeds in car.lua to 10 (https://github.com/Project-OSRM/osrm-backend/blob/master/profiles/car.lua#L142-L157) and notice how you're still getting long-distance routes with average speeds of far above 50.

Fix

I have fixed the problem for me by changing the aforementioned lines in way_handlers.lua to

  if forward and forward > 0 then
    result.forward_speed = math.min(forward * profile.speed_reduction, result.forward_speed)
  end

  if backward and backward > 0 then
    result.backward_speed = math.min(backward * profile.speed_reduction, result.backward_speed)
  end

but it would probably be more elegant to fix the problem by putting the limit function to its intended use.

Related issues

This bug might also be the source of issue #5684 and part of the answer to #6872.

jcoupey commented 4 months ago

I did a few tests using the debug_way.lua script and I can definitely reproduce this behavior. Setting a very low speed for e.g. highway=primary is only accounted for on primary roads without a max tag. On the rest (that is most of them), the applied speed is directly derived from the maxspeed* values, even if way higher than the default speed provided in the profile.

@woodpeck I think your change does express the intended behavior for WayHandlers.maxspeed, would be great to submit it as a PR.