Project-OSRM / osrm-backend

Open Source Routing Machine - C++ backend
http://map.project-osrm.org
BSD 2-Clause "Simplified" License
6.42k stars 3.4k forks source link

Strange things happening around only_straight_on restrictions #150

Closed risicle closed 12 years ago

risicle commented 12 years ago

Hi,

I'm having good results feeding tens of thousands of tiny routing requests to OSRM, but of course in doing so I'm bound to find some hiccups. I've been having problems getting strange routes and total_times back from OSRM, which of course screws up the algorithm I'm trying to design on top of the routing.

I did notice however that in all the places I was getting these odd results, there was an only_straight_on restriction on one of the relevant ways. In some places, the router seems to think it's impossible to get from one segment of a way to the next (and we're not talking about way-way junctions, this is within one way), taking a 500m route when the two points are ~15m apart. In other places, the router ignores the only_straight_on restriction entirely, taking an illegal route and returning a total_distance of 10 and a total_time of 1s (where the actual route it's chosen is more like 300m).

The 3 places I'm noticing it are in London, the relation centered around node 449387170, on way 1884029 and around way 4223965 (this one's harder to pinpoint). (But then again, who knows if said objects will still exist by the time you read this?)

I'm using OSRM revision b210b22975. You'll probably need me to be more specific about exact data snapshot versions - suffice to say it happens in the greater_london.osm.pbf from geofabrik from a few weeks ago, but I'll need a little more time for more specifics.

DennisOSRM commented 12 years ago

Thanks for the feedback. Will look into it.

risicle commented 12 years ago

Bingo. I've managed to nail this down to a small .osm file.

http://ris.dev.openstreetmap.org/data/osrm_issues/M4J4.osm.bz2

(github doesn't seem to support issue attachments)

Using 795d2b264cfc1a32486b8c7989dfbd51e78444e4 and the above data the request:

/viaroute?start=51.496516119,-0.453495176566&dest=51.4965586504,-0.453443292968&instructions=true&geometry=true&geomformat=cmp&output=json

sends me up around the roundabout but then short-circuits at the traffic lights. But more importantly it then tells me the total_distance is 10 and the total_time is 1.

emiltin commented 12 years ago

you can also creating a failing cucumber test. it makes it easy to investigate, and make it easy to later check for regresssion errors after the bug is fixed. see https://github.com/DennisOSRM/Project-OSRM/wiki/Cucumber-Test-Suite

you'll find some tests for left_turn_only in the file https://github.com/DennisOSRM/Project-OSRM/blob/master/features/restrictions.feature, but they currently seem to fail, see #148.

risicle commented 12 years ago

I've now run a lot more tests on 795d2b2 and though one of my issues (on embankment in westminster) is fixed, I find in general the total_times reported by this version to be far more erratic than the earlier b210b22. One 10m route will be coverable in 1s while the next 10m route (on the same way) will take 10s.

Edit: just to clarify, this is happening all over the place in 795d2b2, not just around only_straight_on restrictions.

emiltin commented 12 years ago

hi risicle, i can add that the tests in features/time.feature also indicates odd behaviour. you're welcome to add more tests.

one thing i found is that osrm will truncate the total length (not time..) to the nearest 10 m interval. this especially effect time estimas on very short routes.

emiltin commented 12 years ago

here's one of the tests:

@routing @time
Feature: Estimation of travel time
  Note:
  15km/h = 15000m/3600s = 150m/36s = 100m/24s
  5km/h = 5000m/3600s = 50m/36s = 100m/72s

  Background: Use specific speeds    # features/time.feature:7
    Given the speedprofile "bicycle" # features/step_definitions/data.rb:1
    And the speedprofile settings    # features/step_definitions/data.rb:5
      | primary | 15 |
      | footway | 5  |

  Scenario: Time of travel on combination of road types # features/time.feature:147
    Given the nodes                                     # features/step_definitions/data.rb:15
      | a | b | c | d | e |
    And the ways                                        # features/step_definitions/data.rb:32
      | nodes | highway |
      | abc   | primary |
      | cde   | footway |
    When I route I should get                           # features/step_definitions/routing.rb:190
      | from | to | route   | time |
      | b    | c  | abc     | 24s  | <--- expected
      | b    | c  | abc     | 25s  | <--- got
      | c    | e  | cde     | 72s  | <--- expected
      | c    | e  | cde     | 145s | <--- got
      | b    | d  | abc,cde | 144s | <--- expected
      | b    | d  | abc,cde | 97s  | <--- got
      | a    | e  | abc,cde | 192s | <--- expected
      | a    | e  | abc,cde | 191s | <--- got
      Tables were not identical (Cucumber::Ast::Table::Different)
emiltin commented 12 years ago

see updated distance tests: #154

DennisOSRM commented 12 years ago

I took the test file you uploaded earlier, ran it through the tool chain and it seems to work on my end fine. It contains four only_* restrictions that get parsed and applied:

[info Contractor/EdgeBasedGraphFactory.cpp:277] Edge-based graph skipped 4 turns, defined by 4 restrictions.

emiltin commented 12 years ago

i'm continuing the discussion about turn restrictions at #148

risicle commented 12 years ago

DennisOSRM: hm. I did a complete clear out of my build directory, re-cloned and rebuilt the data and it works now. Strange. Sorry I didn't try that eariler.

Interestingly OSRM (795d2b2) doesn't build first time for me. I think the dependencies must be slightly wrong. The first time it fails to link osrm-extract with the pbf protocols (because they haven't been built yet). The second time I run the same build command, the first thing it does is compile the protos, so everything else succeeds.

emiltin: Are there any plans to allow the use of actual .osm files for tests? I can't help but think bugs may arise that are only triggered when a complicated setup of maybe 50 ways is used.

DennisOSRM commented 12 years ago

Alright, great to hear that it does work now. I'll close this issue, but please continue the discussion here if needed.

emiltin commented 12 years ago

@risicle interestint point you bring up. you're right that bigger data sets might bring out other types of bugs.

when i initialy started on the test suite, i tried using real-world osm data, but i realized that small well-known datasets that doesn't change are easier to write, understand and debug.

using real-world osm data makes it harder to specify the decided outcome, first of all because the data is more complex, but also because it's continuosly changing. not updating the data is not attractive either because other people will reference the updated version, and you might be stuck with invalid osm data.

at this point, i think simple test data files are more effective in squashing bugs, but i'm sure more complex tests will be needed at some point.

btw, there is nothing stopping you from using the current cucumber setup to write tests with 50 or 100 ways :-)

risicle commented 12 years ago

@emiltin I don't see using "old osm data" as a problem. OSM data does become invalid very slowly, and a test failing because of outdated mapping practices should be fairly easy to spot.

"btw, there is nothing stopping you from using the current cucumber setup to write tests with 50 or 100 ways :-)"

Patience & sanity.

DennisOSRM commented 12 years ago

Using old data is fine from my side as well. Should be covered by the OSM license. I only have one concern and that is that the data files should be stripped to minimal size.