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

Rename uturns parameter #2213

Closed TheMarex closed 8 years ago

TheMarex commented 8 years ago

The uturns parameter is confusing (it does not govern actual uturn instructions but only uturns at vias). If a vehicle can turn in the middle of a street is determined by mode most of the time, as such having a query-time option is only useful in edge cases (namely allowing cars to turn in the middle of the street).

As such the new proposal is to drop uturns completely.

jcoupey commented 8 years ago

Does this mean not being able to allow uturns at vias anymore? I can see use-cases where this would be troubling.

Say you perform a table request you use to plan a trip through several locations. The overall duration is then obtained by summing up durations from the table. This will be shorter than the overall duration from a route query using the same points as vias, unless uturns are enabled at vias.

MoKob commented 8 years ago

Technically you are correct. But you have to consider that the u-turn itself might not be allowed or even feasible at that location. If you consider a large road with a lot of traffic, simply assuming a (in your case instant) u-turn to be a possible choice is not possible. Even if you would allow the u-turn, the turn itself is not free and also results in a different time than the time obtained from summing up the values returned by the table plugin.

The problem in your case is that you use a combination of tools in a way that is invalid.

If you want to enable u-turns, I believe it will still be possible to do on a per profile basis.

MoKob commented 8 years ago

After second though, this change will not allow for the same results. The parameter will disallow u-turns at via coordinates. Allowing them at the end of the street will not yield the same result.

jcoupey commented 8 years ago

You're right about the cost of the uturn itself and the cases where it is just impossible at some via. And my real issue here is that planning from the table can't ever account for this, as it depends on the next location.

All I'm reporting is that I've experienced significant differences between summing up durations from a table and the overall duration of a route without uturns. On the other hand, using uturns on the via has always enabled to make the two approaches very consistent.

I guess it's just that getting different results sounds a bit weird but from what has been said this is probably the only normal behaviour one should expect.

freenerd commented 8 years ago

I'm understanding what we want to do here this way:

In either case, u turns in dead end ways will always be allowed.

TheMarex commented 8 years ago

@jcoupey the problem is that durations for routes you get from the distance table is that they are not constrained by the direction of the street segments. The values represent: min(d(forward_a, forward_b), d(forward_a, reverse_b), d(reverse_a, forward_b), d(reverse_a, reverse_b))

If you add up two entries in the matrix, say (a, b) and (b, c) what you actually get is:

d'(a, b , c) = min(d(forward_a, forward_b), d(forward_a, reverse_b), d(reverse_a, forward_b), d(reverse_a, reverse_b)) + min(d(forward_b, forward_c), d(forward_b, reverse_c), d(reverse_b, forward_c), d(reverse_b, reverse_c))

This assumes that at b you can instantaneous change from forward_b to reverse_b and vice versa. The route plugin uses the additional constraint that the direction of the "middle" via need to match. So you get:

d(a, b, c) = min(d(foward_a, forward_b) + d(forward_b, forward_c), d(forward_a, reverse_b) + d(reverse_b, forward_c), d(foward_a, forward_b) + d(forward_b, reverse_c), d(forward_a, reverse_b) + d(reverse_b, reverse_c))

So what you would want is a distance table that has separate columns/rows for forward_x and reverse_x. This of course quadruples the size of the matrix, would however always lead the correct distance. Do you think that would be a worthwhile addition to the table plugin?

jcoupey commented 8 years ago

@TheMarex Thanks, a bit of internals help me better understand the details.

Hard to say at this point if a x4 sized forward/reverse matrix would be the way to go. This would give access to all the 'right' durations but might prove to be overly painful to deal with, each duration depending on the previous and next travel steps.

Actually I'm not convinced that picking between all the forward/reverse options would be that easy, e.g. when inserting a location c in an existing trip between a and b. If the current directions in use are forward_a->forward_b, one might want to pick the best option between forward_a->forward_c->forward_b and forward_a->reverse_c->forward_b. But this is only a greedy choice as whatever_previous->reverse_a->forward_c->forward_b or whatever_previous->reverse_a->reverse_c->forward_b might actually be shorter. Same for b and possibly with more previous or next steps involved.

TheMarex commented 8 years ago

@jcoupey I see, the problem is that forward_a and reverse_a are not independent nodes, so this would break some assumptions of the algorithm.

Also thinking about this more, in this case these would be different scenarios. If I imagine a logistics company were the waypoints are the entries to their unloading docks, its fair to assume that you can actually chose your direction freely after you dropped off your goods.

Then again there is the pizza delivery scenario that would not have this assumption, as you would just like to park you car besides the street and then drive on.

@freenerd actually these are very good scenario to illustrate the usage of this parameter I think. And it also makes sense to expose it.

freenerd commented 8 years ago

Looking at the use case @TheMarex outlined, it's not that clear anymore that the uturns parameter should be removed. To clean up confusion, it might be better to rename the parameter:

We are split here between keeping this parameter concise and explaining the complex concept with it.

freenerd commented 8 years ago

Let's go with waypoints_direction={same|all}.

jcoupey commented 8 years ago

:+1:

TheMarex commented 8 years ago

Waiting on https://github.com/Project-OSRM/osrm-backend/pull/2229 before tackling this

willwhite commented 8 years ago

Since there are only two values, using a boolean like maintain_waypoint_direction={true|false} or maintain_direction_at_waypoints={true|false} would be a bit more intuitive imo.

freenerd commented 8 years ago

In Friday @willwhite and me talked about using continue_straight_after_waypoints={true|false} with the reasoning that:

The only bad interpretation I see: After every waypoint the first instruction is "straight"

I hate naming.

freenerd commented 8 years ago

This is the hardest naming ever. We are converging towards continue_straight={true|false} now, since that is shorter. The bad interpretation seems okayishly uncommon.

emiltin commented 8 years ago

straight_after_waypoint?

danpat commented 8 years ago
TheMarex commented 8 years ago

This landed as continue_straight. sigh

freenerd commented 8 years ago

Most. Painful. Process.

willwhite commented 8 years ago

RIP train_mode

karenzshea commented 8 years ago

I stand by the name

danpat commented 8 years ago

Classic bikeshed.