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

Out of bounds error when nothing is reachable #201

Closed
mattwigway opened this issue Sep 24, 2021 · 4 comments
Closed

Out of bounds error when nothing is reachable #201

mattwigway opened this issue Sep 24, 2021 · 4 comments

Comments

@mattwigway
Copy link
Contributor

@c-voulgaris and I were just troubleshooting a travel-time matrix computation that was giving an out of bounds error here—saying that j was out of bounds for travel_times. It turned out the issue was that everything in this analysis was unreachable and unlinked (because the input data were in projected coordinates rather than lat-lon). I'm not exactly sure why this caused the out-of-bounds error, but maybe r5 short-circuits internally in this situation and returns an unexpected result. In any case, we should handle this case gracefully.

I'm also confused as to why the loop on the above-linked line starts from 3.

@mattwigway
Copy link
Contributor Author

mattwigway commented Sep 24, 2021

It might also make sense to check the projections of incoming sf objects and warn if they are not WGS84 (probably just warn, because they could be in e.g. NAD83, NAD83(HARN) etc which are different from WGS84 if you're doing geodetic surveys, but close enough for routing applications).

@mvpsaraiva
Copy link
Collaborator

Thanks @mattwigway. I've fixed the first part of this issue (the out of bounds error). Will leave this open to remember to handle the projection of incoming sf later.

I'm also confused as to why the loop on the above-linked line starts from 3.

We start in 3 here because we only need to fix travel times, and columns 1 and 2 are the id's of the origin and destination points.

@dhersz
Copy link
Member

dhersz commented Oct 20, 2021

Fixed that in eba0cb6.

Decided to raise an error instead of a warnings, just to be more strict the inputs. Using points with the wrong CRS might return very weird results, and I think it's better to ask the user to handle this situation before using the function to save them/us from possible headaches because of this.

Demonstrating usage (assert_points_input() handles this case internally):

path <- system.file("extdata/spo", package = "r5r")
points <- read.csv(file.path(path, "spo_hexgrid.csv"))

origins_sf <- destinations_sf <- sf::st_as_sf(
  points[1:10, ],
  coords = c("lon", "lat"),
  crs = 4674
)

r5r:::assert_points_input(origins_sf, "origins")
#> Loading required namespace: rJava
#> Error in r5r:::assert_points_input(origins_sf, "origins"): 'origins' CRS must be WGS 84 (EPSG 4326). Please use either sf::set_crs() to set it or sf::st_transform() to reproject it.

origins_sf <- sf::st_transform(origins_sf, 4326)
r5r:::assert_points_input(origins_sf, "origins")
#>                  id sfg_id point_id       lon       lat
#>  1: 89a8100c603ffff      1        1 -46.60797 -23.57110
#>  2: 89a8100c617ffff      2        2 -46.61108 -23.57249
#>  3: 89a8100c60fffff      3        3 -46.60788 -23.56803
#>  4: 89a8100c607ffff      4        4 -46.61099 -23.56942
#>  5: 89a8100c6abffff      5        5 -46.61409 -23.57082
#>  6: 89a8100c6a3ffff      6        6 -46.61719 -23.57221
#>  7: 89a8100c677ffff      7        7 -46.60779 -23.56497
#>  8: 89a8100c63bffff      8        8 -46.61090 -23.56636
#>  9: 89a8100c633ffff      9        9 -46.61400 -23.56775
#> 10: 89a8100c6afffff     10       10 -46.61710 -23.56914

@dhersz dhersz closed this as completed Oct 20, 2021
@mattwigway
Copy link
Contributor Author

Thanks!

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

3 participants