UrbanAnalyst / dodgr

Distances on Directed Graphs in R
https://urbananalyst.github.io/dodgr/
127 stars 16 forks source link

paths through roundabouts do not obey turn restrictions #175

Closed deanmarchiori closed 2 years ago

deanmarchiori commented 2 years ago

Hi, Excellent package, thanks so much. Just filing an issue for some unexpected behaviour, although quite possible its user error.

Issue: When calling dodgr::dodgr_paths between two nodes returned from dodgr_streetnet_sc, such that a roundabout junction is transited, the path finds the shortest path, but seems to go 'the wrong way' around the roundabout. i.e. it doesn't seem to obey conventional turn restrictions.

In the example below, the red points highlight the vertices returned in the path. I have annotated to show start and end nodes.

On inspection of the data, I cannot see how turn restrictions are being parsed from the OSM data.

(Apologies of this is a limitation I am unaware of, or if I have done something silly!)

Attempted Fix:

image

Example:

library(dodgr)
library(sf)
library(mapview)
library(dplyr)

streetnet <- dodgr_streetnet_sc(c(150.88907, -34.4220507, 150.8985099, -34.4210507), expand = 1)

wt_streetnet <- dodgr::weight_streetnet(streetnet, 
                                        wt_profile = 'motorcar', 
                                        left_side = TRUE, 
                                        turn_penalty = TRUE, 
                                        keep_cols = "restriction")

path <- dodgr::dodgr_paths(wt_streetnet, from = "2386269053", to = "8634061489")

Visualising path:

vert <- dodgr::dodgr_vertices(graph = wt_streetnet) |> 
  st_as_sf(coords = c("x", "y"), crs = 4326)

path <- vert |> 
  filter(id %in% path$`2386269053`$`2386269053-8634061489`) 

mapview(vert) + 
  mapview(path, col.regions = 'red') 

Session Info:

R version 4.1.1 (2021-08-10)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19042)

Matrix products: default

locale:
[1] LC_COLLATE=English_Australia.1252  LC_CTYPE=English_Australia.1252   
[3] LC_MONETARY=English_Australia.1252 LC_NUMERIC=C                      
[5] LC_TIME=English_Australia.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] dplyr_1.0.7    mapview_2.10.0 sf_1.0-3       dodgr_0.2.12  
mpadge commented 2 years ago

G'day Dean, thanks for the kind words! And yep, you've found a bug. The whole roundabout thing has been simply overlooked until now. Watch here for implementation of a fix, which shouldn't take too long.

mpadge commented 2 years ago

That was a really good catch @deanmarchiori, thanks so much! You should now see something like this:

image

deanmarchiori commented 2 years ago

G'Day Mark,

Wow what a quick fix! Thanks heaps for that.

Dean.

mpadge commented 2 years ago

Thanks for finding that out - it was a pretty important bug to uncover, and something that i just hadn't explicitly thought about until you raised it.