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

Pass additional variables to turn_function #2822

Closed
wants to merge 1 commit into from
Closed

Pass additional variables to turn_function #2822

wants to merge 1 commit into from

Conversation

danpat
Copy link
Member

@danpat danpat commented Aug 30, 2016

This PR is to address #1735, and hopefully make turn penalties better by considering the approach/exit speeds for each turn. As pointed out on #1735, doing a u-turn on a 70mph (112km/h) road should be super expensive. On the car profile, currently we only penalize this turn by 3.3 seconds, and most 60km/h 90 degree turns are penalized by ~0.8 to ~1.2 (left vs right, depending on the bias).

This turn penalty calculation only occurs during osrm-extract, this PR will not include a mechanism to adjust the turn penalty if traffic data updates are used in osrm-contract.

My plan is to update the turn_function for the car.lua profile only. Other profiles are probably a bit less affected by this, although @emiltin, any thoughts you have on how this might apply to the bike turn penalty would be appreciated.

Tasklist

  • Test coverage
  • Do some test driving to get some better values for the car profile
  • Do some test riding?
  • review
  • adjust for for comments

References

http://nacto.org/docs/usdg/effect_of_radius_of_curvature_for_right_turning_vehicles_wolfe.pdf
http://onlinepubs.trb.org/onlinepubs/conferences/2011/RSS/3/Wolfermann,A.pdf
http://www.istiee.org/te/papers/N55/ET_2013_55_1_Mehar.pdf

Other useful factors

#592

@TheMarex
Copy link
Member

Few notes here:

  • the current implementation breaks the API and would require a major version bump. (not sure how lua feels about optional arguments but that could be an answer)
  • keep in mind [Not ready] Weight edges #1614 modfies the signature of turn_function drastically

@danpat
Copy link
Member Author

danpat commented Aug 30, 2016

@TheMarex AFAIK, Lua has no concept of function overloading, so the following works:

$ lua5.1
Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> function test(a)
>>   print "hello"
>> end
> test(1,2)
hello

The additional arguments passed by luabind to the function should just be ignored by older profiles that don't implement them. This should only require be a backward-compatible minor version bump.

// segment_length is in metres, weight is in deciseconds, this converts those
// to km/h
const double approach_speed = approach_segment_length / (approach_road_segments.back().weight/10) * 3.6; // km/h
const double exit_speed = exit_segment_length / (exit_road_segments.front().weight/10) * 3.6; // km/h
Copy link
Member

Choose a reason for hiding this comment

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

clang-format-3.8 all the things

@MoKob MoKob changed the title [not ready] Pass approach/exit speed values to turn_function. Pass approach/exit speed values to turn_function. Sep 2, 2016
@danpat danpat changed the title Pass approach/exit speed values to turn_function. [not ready] Pass additional variables to turn_function Sep 3, 2016
@emiltin
Copy link
Contributor

emiltin commented Sep 6, 2016

I think turn penalties for bikes could be improved quite a bit. however i think the type of road your turning from/to might be more important that the approach/exit speeds. The typical example is that turning left form a big road into a small road at a intersection without traffic lights should incur a much more significant penalty that turning left from a small road into a small road, since you often will have to wait for a gap in car traffic before you can turn.

Similarly, the penalty for doing a u-turn depends more on the type of road that the speed of the bike traffic, since doing a u-turn typically implies crossing the road.

This is why is suggested in #592 that the way type are passed to the penalty function. An alternative is to only depend a real-world measurements, but of course that requires sufficient data.

Turn penalties for bikes could perhaps be improved a bit by using approach/exit speeds, but I don't think it's worth it.

@MoKob MoKob mentioned this pull request Sep 26, 2016
3 tasks
@danpat
Copy link
Member Author

danpat commented Sep 29, 2016

Some further discussion on this one has happened in other forums. Some points that were raised:

  • it might be useful to apply no penalty when the turn instruction is "continue" - implying that the travellers is staying on the same road and likely does not need to slow down, even if the road curves at the intersection.
  • does the turn happen in across non-stopping traffic, and if so, how many lanes - this would be useful for both cars and bikes

@TheMarex TheMarex changed the title [not ready] Pass additional variables to turn_function Pass additional variables to turn_function Oct 3, 2016
@MoKob
Copy link

MoKob commented Oct 12, 2016

I've pulled this into #2912. Closing here in favour of a structure based approach (instead of passing single parameters to the function).

@MoKob MoKob closed this Oct 12, 2016
@MoKob MoKob deleted the fix/1735 branch October 12, 2016 11:18
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.

5 participants