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

od2line() throws an error if only one origin-destination relation is selected #562

Open
yannikbuhl opened this issue May 16, 2024 · 5 comments

Comments

@yannikbuhl
Copy link

Hello,

I have recently been working with stplanr to create and plot desire lines. It worked fine almost all the times (thank you so much for this wonderful package!). However, in some edge cases I was tripping over an error. I have been able to trace it back to its origin and I am also able to suggest a solution. I will provide a minimal working example, too.

The error occurs if one only wants to create OD desire lines between two polygons/points. Its origins lie in od2line.sf(), more specifically in this chunk of code:

coords_o <- sf::st_coordinates(zones)[, 1:2]
origin_matches <- match(flow[[origin_code]], zones[[zone_code]])

od_matches_check(origin_matches, flow[[origin_code]])
origin_points <- coords_o[origin_matches, ]

Here, the object coords_o is of type "matrix" "array". When origin_matches only contains one single index, the matrix subsetting in the last row of the above code does not return a matrix/array, but a named numeric vector. The same applies for the code that deals with dest_points. One of the last actions of the function is to bind these two objects together:

odm <- cbind(origin_points, dest_points)

However, if both of them are named vectors, the result will not be a matrix with one row and four columns named X Y X Y, but a 2x2 dataframe. Subsequently, the code fails once the next line is executed, because the following function od_coords2line and its sub-functions access the object odm/odc by subsetting odc[, 3:4] when checking for NAs in one of the OD coordinates:

odsfc <- od_coords2line(odm, crs = sf::st_crs(zones), remove_duplicates = FALSE)

This leads to the following error: Error in odc[, 3:4] : subscript out of bounds.

What follows is my minimal reproducible example based on stplanrs vignette:

od_all <- pct::get_od()

centroids_all <- pct::get_centroids_ew() %>% sf::st_transform(4326)

london <- pct::pct_regions[1,]

centroids_london <- centroids_all[london, ]

od_london <- od_all %>% dplyr::filter(geo_code1 %in% centroids_london$msoa11cd,
                                      geo_code1 %in% centroids_london$msoa11cd)

od_london <- od_all[od_all$geo_code1 %in% centroids_london$msoa11cd &
                    od_all$geo_code2 %in% centroids_london$msoa11cd,]

desire_lines_london <- od2line(od_london[1,], centroids_london)

I think the easiest solution to the problem could be to use the argument drop = FALSE to preserve the matrix structure when subsetting the object coords_o to create the objects dest_points and origin_points, like so (thanks to @staudtlex for hinting at that):

origin_points <- coords_o[origin_matches, , drop = FALSE]
...
dest_points <- coords_o[dest_matches, , drop = FALSE]

Please let me know if you can reproduce the error and what you think of the solution. I have tested it and it worked in my case. I work on Windows and I am running R 4.4.0 but I have encountered this problem on R 4.1.0, too. If you wish, I can provide a PR to solve the problem.

@Robinlovelace
Copy link
Member

Hey @yannikbuhl thanks for the message and detailed example.

Quickfire follow-up question: have you tried od::od_to_sf() from the {od} package?

At some point I'd like to replace the current od2line() with a small wrapper to the more efficent implementation in the newer package: https://github.com/itsleeds/od

@yannikbuhl
Copy link
Author

Hi Robin, thanks for the quick response. I have not tried od::od_to_sf() yet but I definitely will, thanks for the suggestion. And I'll let you know if that solves the problem :)

@yannikbuhl
Copy link
Author

A quick test with the minimal reproducible example tells me that replacing od2line with od_to_sf does indeed work and return a LINESTRING geometry. I will test this when back at work with the data I'm working with but I'd be optimistic. Thanks so much for your suggestion!

desire_lines_london2 <- od::od_to_sf(od_london[1,], centroids_london)

@Robinlovelace
Copy link
Member

Robinlovelace commented May 16, 2024

Awesome, glad it worked. Do close this issue if fixed for you and wish me luck updating od2line() to build on the 'engine' of {od} as a backend!

@Robinlovelace
Copy link
Member

In fact may be useful to keep open to track updating od2line().

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

No branches or pull requests

2 participants