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 turn penalty function to better fit some measured data. #2849

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

danpat
Copy link
Member

@danpat danpat commented Sep 3, 2016

This PR modifies the existing car.lua turn function to attempt to better model car turning penalties.

I went out driving and measured a bunch of turning movements:

screen shot 2016-09-02 at 5 21 03 pm

The key observations from the sample data were:

  1. We weren't penalizing 90 degree turns enough - typical values were from the 2-4 second range.
  2. There is not as much bias from left-to-right as we applied.
  3. Turns <45 degrees incurred relatively little penalty.
  4. Traffic-control mechisms (lights, stop signs) play a big part in the cost of turns.

Our existing turn function only accepts an angle value. This PR updates the turn function (and default parameters) for the car.lua profile to attempt to get a slighly better fit. In general, turns >90 degrees should have significantly higher penalties (2x or more), and shallower angle turns should have smaller penalties. There is a left/right bias still, shallow-angle left turns (in right-driving countries) do incur an extra cost.

As can be seen on the plot above, turns that traverse intersections with traffic control (lights and stop signs) incur a higher penalty than those that do not. At this stage we don't have that data available to the turn function, so this change is incomplete.

This PR is intended as a stepping stone towards #2822 that will pass additional data to the turn function.

Tasklist

  • review
  • merge

@daniel-j-h
Copy link
Member

Do we want to use the turn angles not only for penalties but also e.g. to prevent uturns at a road with high speed?

Can and should we use additional heuristics to influence the turn penalty, such as the road's width, lanes and other factors that may make a difference especially when the road is not modelled out in detail in the base map?

Can we add some Cucumber tests for this making sure the values make sense (+-) for certain turn angles? Mostly to catch regressions and to make sure this important part of the pipeline stays functional.

@danpat danpat force-pushed the sigmoid_turn_function branch from cc09f9b to 3a6c040 Compare September 6, 2016 15:59
@danpat
Copy link
Member Author

danpat commented Sep 6, 2016

@daniel-j-h This PR is just a quick-fix - #2822 is the place to include a lot of the extra parameters you mentioned, and it requires more than just a profile change.

Doing a CucumberJS test for just this function would add the requirement of having a Lua interpreter installed - something we don't currently require (just the runtime libs). Given the limited scope of this change, I'd like to hold off investing in testing infrastructure until #2822 lands.

@danpat danpat merged commit 1ab2b87 into master Sep 6, 2016
@danpat danpat deleted the sigmoid_turn_function branch September 6, 2016 16:07
@MoKob MoKob mentioned this pull request Sep 15, 2016
@@ -544,12 +544,13 @@ function way_function (way, result)
end

function turn_function (angle)
---- compute turn penalty as angle^2, with a left/right bias
-- Use a sigmoid function to return a penalty that maxes out at turn_penalty
-- over the space of 0-180 degrees. Values here were chosen by fitting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean this function expecting input from 0 - 180? How does that reconcile with this comment about how turn angles range from 0 - 360?

Also, this comment implies that the turn function is passed negative values (e.g. turn_function(-135)).

In short, what is the range of possible values that can be passed to turn_function()?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have two different notations here. OSRM internally operates on turn angles from 0 to 360 with 180 going straight.

Now in the profile, we use -180 as u-turn to the left, 0 as going straight, 180 as u-turn to the right.

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

Successfully merging this pull request may close these issues.

4 participants