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

Usage of negative turn penalties #3683

Closed
5 tasks
oxidase opened this issue Feb 10, 2017 · 2 comments
Closed
5 tasks

Usage of negative turn penalties #3683

oxidase opened this issue Feb 10, 2017 · 2 comments

Comments

@oxidase
Copy link
Contributor

oxidase commented Feb 10, 2017

Some tests in OSRM make turn penalties updates with negative value. This introduces incorrect behavior in snapping start points and may lead to quite non-obvious problems like #3647 and #3641.
Practically, negative turn penalties would make turns more preferable instead of going straight with 0 turn penalty. This can be achieved also by using a positive bias value and making all turn weight penalties positive. Duration values are not used as key values in heaps and can be negative.

  • clarify domain of turn penalties
  • review tests features/car/traffic_turn_penalties.feature and features/testbot/traffic_turn_penalties.feature
  • adjust updates
    if (turn_weight_penalty + new_weight < weight_min_value)
    {
    util::Log(logWARNING) << "turn penalty " << turn_weight_penalty << " for turn "
    << turn_index.from_id << ", " << turn_index.via_id << ", "
    << turn_index.to_id
    << " is too negative: clamping turn weight to "
    << weight_min_value;
    turn_weight_penalty = weight_min_value - new_weight;
    }
  • add non-negativity check of weights at
    auto segment_speed_lookup = CSVFilesParser<Segment, SpeedSource>(
    1, qi::ulong_long >> ',' >> qi::ulong_long, qi::uint_ >> -(',' >> qi::double_))(
    config.segment_speed_lookup_paths);
    auto turn_penalty_lookup = CSVFilesParser<Turn, PenaltySource>(
    1 + config.segment_speed_lookup_paths.size(),
    qi::ulong_long >> ',' >> qi::ulong_long >> ',' >> qi::ulong_long,
    qi::double_ >> -(',' >> qi::double_))(config.turn_penalty_lookup_paths);
  • add at
    const EdgeWeight to_weight = weight + edge_weight;
    assertion that to_weight >= 0

/cc @danpat

@danpat
Copy link
Member

danpat commented Feb 14, 2017

I think that we should allow for negative turn penalty values.

Consider a simple intersection where most of the traffic is not taking the straight maneuver - 25% of the traffic volume goes straight, 75% makes the right-hand turn:

screen shot 2017-02-13 at 4 09 07 pm

Let's say we have samples of all cars passing the "Average speed" measurement point. It's likely that the average speed is biased towards cars making the turning maneuver.

In this case, it would make sense for the 25% straight through traffic to receive a "negative turn penalty" - it's faster than average to proceed straight.

Of course, the validity of this idea depends on these biases existing. If the speed approaching the intersection is the average of the vehicles making the fastest maneuver, then all penalties can be positive for the same effect, and the fastest maneuver would have a penalty of 0.

Copy link

github-actions bot commented Jul 8, 2024

This issue seems to be stale. It will be closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Jul 8, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants