Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move phantom node to the junction of the closest way #4192 [WIP] #4272

Closed
wants to merge 1 commit into from

Conversation

fijemax
Copy link
Contributor

@fijemax fijemax commented Jul 10, 2017

I am trying to develop the feature describe in this Issue #4192.
For the moment I tried two way :

  • After query, during the phantomNode construction, I build the phantomNode differently and return that.
  • Two successive requests, the first is to identify the better segment extremity and the second exactly on the extremity choose for request the better node.

For the moment both solution return a snaped point but the algorithm passes through the wrong arc (see the screenshot).

The code is extremely ugly, and i think the query should be made in static_rtree.hpp instead of geospatial_query.cpp.

Have you any idea to achieve this Issue ?

what i have :
capture d ecran de 2017-07-10 15-50-37

what i want :
capture d ecran de 2017-07-10 15-50-43

tmp
Signed-off-by: FILLAU Jean-Maxime <jean-maxime.fillau@mapotempo.com>
@fijemax fijemax changed the title Move phantom node to the junction of the closest way #4192 Move phantom node to the junction of the closest way #4192 [WIP] Jul 11, 2017
@TheMarex
Copy link
Member

Just stumbled over this PR again and I think it is not possible (at the phantom-node level) for how OSRM models streets currently. The RTree will always return the segment that is closed to your input coordinate, which would be your one-way street segment there. By just moving snapping to the end of that segment, you will still force OSRM to route into the oneway street.

Finding the nearest intersection would require an own data structure that is indexed by points (intersection coordinates) and would return the PhantomNodes of all adjacent street segments.

Do you think we can close this PR? I'm currently cleaning up the PR queue.

@frodrigo
Copy link
Member

Yes you can close it.
Nevertheless do you think it's do at an uppler level ? even if it's sub optimal. Even if it's not a nice solution and not making a PR with it, we looking to have this "uncommon" feature in our multipoints route optimizer.

@TheMarex
Copy link
Member

@frodrigo I don't think that's possible, just by the nature we index the data for location lookups.

Implementing this at least involves the following steps:

  1. Build a spatial index base on intersections (within or outside of OSRM) to snap a coordinate to an intersection coordinate
  2. Make the Rtree return all PhantomNodes for a given coordinate (there would be several street segments with distance 0 if you input the intersection coordinate)
  3. Adapt the query code to be able to deal with multiple results from the RTree query (all need to be inserted in the heaps).

(1) is straight forward but (2) and (3) require lager changes in the codebase.

@TheMarex TheMarex closed this Aug 30, 2017
@emiltin
Copy link
Contributor

emiltin commented Aug 31, 2017

here's an idea for an ugly workaround to force the route to start and end from intersections: compute the route as normal, then remove the part before the first turn and after the last turn..

@frodrigo
Copy link
Member

@emiltin is not so simple. The goal is to allow another computed route by having more possibilities at the junction than the phantom node position (think about oneway end).

@frodrigo
Copy link
Member

(there would be several street segments with distance 0 if you input the intersection coordinate)

@TheMarex but it's not already the case if the input point is exactly on a junction ? We test this case and it seems working.

@danpat
Copy link
Member

danpat commented Aug 31, 2017

@frodrigo The problem @TheMarex is describing with (2) is that currently, OSRM must start on an edge. If you supply a coordinate that is exactly an intersection, it will randomly snap to one of the connected roads.

The distance from the snap point to the end of the road will be 0m, but the problem is that there will be turn costs calculated to move from the end of the snapped road onto the other directly connected roads - the route leaving the snapped road doesn't have the penalty.

The values are usually quite small (6-8 seconds for the car profile), so they may not affect route calculation very much, but it's not technically perfect.

This problem already exists in OSRM. Take the following scenario:

screen shot 2017-08-31 at 10 28 35 am

The closest point on all three segments to the point is right at the intersection. The road that is snapped will depend on how the RTree is sorted.

One thing we should probably do is modify the PhantomNode GetForwardWeightPlusOffset and GetReverseWeightPlusOffset - if the offset puts the point at the end of the geometry, this means we snapped to the end of the line (i.e. the intersection), we should subtract any turn penalty for reaching other roads from that point. This would correct the problem that @TheMarex pointed out. If you snap even 1m back from the intersection, then you would pay the entire turn cost.

This doesn't solve the "only snap to intersections" problem, but it fixes the route calculation. If you could build an external index of intersections, and pre-snap your coordinates to those, then OSRM would do the right thing if you fed it intersection coordinates.

I've opened #4465 describing this. The main problem implementing this is that a lot our test cases are going to need to be checked/modified, as they depend on RTree snapping order right now (which is deterministic, but not in a thought-out way).

@frodrigo
Copy link
Member

frodrigo commented Sep 1, 2017

OK Thank you for your explanation.
I got one real case with this issue:
http://map.project-osrm.org/?z=17&center=44.843916%2C-0.612568&loc=44.842310%2C-0.614355&loc=44.842120%2C-0.614263&hl=en&alt=0

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants