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

Strange things happening around only_straight_on restrictions #150

Closed
risicle opened this issue Feb 25, 2012 · 14 comments
Closed

Strange things happening around only_straight_on restrictions #150

risicle opened this issue Feb 25, 2012 · 14 comments

Comments

@risicle
Copy link
Contributor

risicle commented Feb 25, 2012

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 b210b22. 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
Copy link
Collaborator

Thanks for the feedback. Will look into it.

@risicle
Copy link
Contributor Author

risicle commented Feb 25, 2012

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 795d2b2 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
Copy link
Contributor

emiltin commented Feb 26, 2012

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
Copy link
Contributor Author

risicle commented Feb 26, 2012

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
Copy link
Contributor

emiltin commented Feb 27, 2012

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
Copy link
Contributor

emiltin commented Feb 27, 2012

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
Copy link
Contributor

emiltin commented Feb 27, 2012

see updated distance tests: #154

@DennisOSRM
Copy link
Collaborator

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
Copy link
Contributor

emiltin commented Feb 27, 2012

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

@risicle
Copy link
Contributor Author

risicle commented Feb 27, 2012

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
Copy link
Collaborator

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

@emiltin
Copy link
Contributor

emiltin commented Feb 27, 2012

@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
Copy link
Contributor Author

risicle commented Feb 28, 2012

@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
Copy link
Collaborator

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.

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

3 participants