pgRouting / osm2pgrouting

Import tool for OpenStreetMap data to pgRouting database
https://pgrouting.org
GNU General Public License v2.0
289 stars 111 forks source link

Are roundabout correctly parsed? #167

Open MarHoff opened 7 years ago

MarHoff commented 7 years ago

After using osm2pgrouting to parse data I've noticed that ways which are part of roundabout are incorrectly considered as two-way.

As stated in OSM wiki for the tag:junction=roundabout

  • The OSM ways of the roundabout itself must be drawn in the direction the traffic flows.
  • Tag the OSM way(s) of the roundabout with junction=roundabout.
  • oneway=yes is implied. Though it is not wrong to add it explicitly, it is not needed.

Unless I did some miss-configuration it look like all ways that are part of roundabout can lead to wrong calculations later on with pgrouting as roundabout awareness is lost in resulting network (when using mapconfig_for_cars.xml).

Even if resulting error is minimal in term of travel time/distance, when mapping result the result look pretty bad and can inspire doubt to the whole calculation method as viewers can pretty easily spot wrong paths in roundabouts.

cvvergara commented 7 years ago

hello, didn't know that. I'll see to it, I dont know how to consider it, a bug or feature request. but its labeled

MarHoff commented 7 years ago

Hi!

Here is an illustration of pgr_dijkstra() results based on my dataset over mapnik (we drive right).

As you will notice both paths respects one way restrictions. Access ramp to the roundabout are unaffected as well (marked as one way in the ways table).

But the red path is taking an ugly and illegal shortcut as ways of the roundabout are set to : one_way=0 and reverse_cost=cost bug_roundabout Can you reproduce on another OSM dataset? I guess it might be hard to spot because this heavily depend on queried route and roundabout shape itself.

cvvergara commented 7 years ago

hmm... I must say that I am not a visual person, I am a data person ;) can you give me the BBOX of the area shown so I can download that ultra petit area (can be a bit larger) so I can down load like this:

CITY="PETIT_AREA"
BBOX="7.097,50.6999,7.1778,50.7721"  #(s,w,n,e)
wget --progress=dot:mega -O "$CITY.osm" "http://www.overpass-api.de/api/xapi?*[bbox=${BBOX}][@meta]"

On the meantime, look for the rows of the roundabout on your edge table, and put a negative number in (cost or reverse_cost) and (cost_s or reverse_cost_s)

dkastl commented 7 years ago

oneway=yes is implied. Though it is not wrong to add it explicitly, it is not needed.

This was new to me. I assumed that by having oneway set on roundabouts there shouldn't be a problem. But as you reference the OSM wiki, my assumption seems to be wrong.

I'm wondering if this doesn't cause problems, as we then need to know if the traffic is left-sided or right-sided to apply the costs accordingly.

MarHoff commented 7 years ago

@cvvergara I confirm that with my implementation updating this roundabout edges so that reverse_cost=-cost solve the issue. Green path is unaffected while red path now avoid illegal turn.

roundabout_patched

This should be the correct extent for the view (WGS84) but I fear that the problem is pretty widely widespread. I've spotted multiple occurrences in my working area.

    <xmin>7.23983975890579</xmin>
    <ymin>47.8682489475</ymin>
    <xmax>7.25417104109422</xmax>
    <ymax>47.8814727525</ymax>

@dkastl Luckily the OSM wiki also state that "The OSM ways of the roundabout itself must be drawn in the direction the traffic flows." so we should be ok by simply setting one_way=1 for roundabout. If a contributor didn't respect the traffic way orientation as specified in the wiki, then this become an OSM issue resolved by updating the OSM database.

cvvergara commented 7 years ago

thanks, I tried it with the developing that I have and as far as can see it had no problems. I just merged my developing work for v2.3 into develop branch.

Please, can you give it a try, and verify these are correct? https://github.com/pgRouting/osm2pgrouting/blob/develop/src/osm_elements/Way.cpp#L161 https://github.com/pgRouting/osm2pgrouting/blob/develop/src/osm_elements/Way.cpp#L191

cvvergara commented 7 years ago

This is the merge information https://github.com/pgRouting/osm2pgrouting/pull/168

cvvergara commented 7 years ago

@MarHoff maybe I used the wrong query. something is wrong

cvvergara commented 7 years ago

(the something is wrong is with the code)

cvvergara commented 7 years ago

I think I fixed it with https://github.com/pgRouting/osm2pgrouting/pull/170 with this commit https://github.com/pgRouting/osm2pgrouting/pull/170/commits/4567733f382512997913463fbc73a9f104dc90ec

can you see if the problem of the junction gets fixed?

thanks

MarHoff commented 7 years ago

I'm sorry I wont be able to test on the real server until Monday... I don't have (maybe for the best) remote access to the database on which I noticed the bug :/

cvvergara commented 7 years ago

@MarHoff don't worry, a lot of work is still pending on v2.3

MarHoff commented 7 years ago

Houston we got a problem this is what showed up after loading the my OSM dataset under develop branch: houston This is the way table under 2.2. 2 2 I guess this mess come from the fact that I use a custom Dataset generated by http://overpass-api.de/ as I work near an international border and country extract are not relevant for me. Will try figure this mess out and double check. I'll post another issue if needed...

cvvergara commented 7 years ago

7.23983975890579

47.8682489475

7.25417104109422

47.8814727525

CITY="PETIT_AREA" BBOX="7.23,47.86,7.25,47.88" #(s,w,n,e) wget --progress=dot:mega -O "$CITY.osm" "http://www.overpass-api.de/api/xapi?*[bbox=${BBOX}][@meta]"

I use the overpass api like that

MarHoff commented 7 years ago

I used the interpreter api instead http://overpass-api.de/api/interpreter with this query file: (Warning huge area not the test area mentioned earlier)

<osm-script output="xml">
  <union into="_">
    <bbox-query e="7.868957519531249" into="_" n="48.372672242291294" s="47.39463076190644" w="6.767578125"/>
    <recurse from="_" into="_" type="up"/>
  </union>
  <print e="" from="_" geometry="skeleton" limit="" mode="meta" n="" order="id" s="" w=""/>
</osm-script>

This is the template recommended by overpass-turbo when exporting query as overpass-XML.

I tried with your template but got the following error message. I think it's due to the api/xapi limitation when querying large areas (my resulting file is 2.7GB with api/interpreter).

<?xml version="1.0" encoding="UTF-8"?>
<osm version="0.6" generator="Overpass API">
<note>The data included in this document is from www.openstreetmap.org. The data is made available under ODbL.</note>
<meta osm_base="2017-04-07T08:16:03Z"/>

<remark> runtime error: Query ran out of memory in "recurse" at line 11. It would need at least 517 MB of RAM to continue. </remark>

</osm>

And as for the roundabout in this mess it now look like Dijkstra want to use both path which is weird. So I'll need to decompose my custom wrapping function and see what happened. double_round

I'm sorry I don't have time to investigate right now but i will definitely follow up. I must confess I tested by pushing data into my production server (but compiled osm2pgrouting in a dev env) but this was a bad idea. I'll need to set up a cleaner test environment and maybe separate the two issues at stake.