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

sum_network_routes() + sfNework returns error when 'start' and 'end' are the same node #444

Closed
jmlondon opened this issue Nov 24, 2020 · 4 comments

Comments

@jmlondon
Copy link

Hello

There seems to be a rather unique situation where sum_network_routes() returns an Error when sln is an sfNetwork object and the start and end node are the same. This is not the case when sln is a SpatialLinesNetwork object.

It seems the preferred response would be an sf LINESTRING with the same start/end coords and length NA.

In my situation, this came up as I was calculating shortest path based on results from find_network_nodes() and the closest node to the given coordinates turned out to be the same.

The following reprex should demonstrate the issue.

library(sf)
#> Linking to GEOS 3.8.1, GDAL 3.1.1, PROJ 6.3.1
library(stplanr)

data("route_network")

# shortest path calculation with SpatialLinesNetwork
suppressWarnings(
sln <- SpatialLinesNetwork(route_network)
)
short_path <- sum_network_routes(sln, start = 1, end = 50)
short_path
#> class       : SpatialLinesDataFrame 
#> features    : 1 
#> extent      : -1.550139, -1.514462, 53.82007, 53.82685  (xmin, xmax, ymin, ymax)
#> crs         : +proj=longlat +ellps=WGS84 +towgs84=0,0,0,0,0,0,0 +no_defs 
#> variables   : 3
#> names       : ID,       sum_length, pathfound 
#> value       :  1, 2.88349109307858,         1
short_path_same <- sum_network_routes(sln, start = 10, end = 10)
short_path_same
#> class       : SpatialLinesDataFrame 
#> features    : 1 
#> extent      : -1.539033, -1.539033, 53.82491, 53.82491  (xmin, xmax, ymin, ymax)
#> crs         : +proj=longlat +ellps=WGS84 +towgs84=0,0,0,0,0,0,0 +no_defs 
#> variables   : 3
#> names       : ID, sum_length, pathfound 
#> value       :  1,         NA,         0

# shortest path calculation with sfNetwork
sln_sf <- SpatialLinesNetwork(route_network_sf)
short_path <- sum_network_routes(sln_sf, start = 1, end = 50)
short_path
#> Simple feature collection with 1 feature and 3 fields
#> geometry type:  LINESTRING
#> dimension:      XY
#> bbox:           xmin: -1.550139 ymin: 53.82007 xmax: -1.514462 ymax: 53.82685
#> geographic CRS: WGS 84
#> # A tibble: 1 x 4
#>                                              geometry    ID sum_length pathfound
#>                                      <LINESTRING [°]> <dbl>      <dbl> <lgl>    
#> 1 (-1.550139 53.82495, -1.549209 53.82496, -1.548204…     1      2883. TRUE
short_path_same <- sum_network_routes(sln_sf, start = 10, end = 10)
#> Error in linecoords[, "L1"]: no 'dimnames' attribute for array

# desired output should be an sf LINESTRING with the same start, end coords, length NA
sf::st_as_sf(short_path_same)
#> Simple feature collection with 1 feature and 3 fields
#> geometry type:  LINESTRING
#> dimension:      XY
#> bbox:           xmin: -1.539033 ymin: 53.82491 xmax: -1.539033 ymax: 53.82491
#> CRS:            +proj=longlat +ellps=WGS84 +towgs84=0,0,0,0,0,0,0 +no_defs
#>   ID sum_length pathfound                       geometry
#> 1  1         NA     FALSE LINESTRING (-1.539033 53.82...

Created on 2020-11-24 by the reprex package (v0.3.0)

@agila5
Copy link
Collaborator

agila5 commented Nov 24, 2020

Hi @jmlondon and thank you for your message! IMO it's not extremely difficult to fix this problem, but I'm not sure about the best approach. My idea would be adding an if clause in sum_network_routes() that tests if the input nodes are identical (and combinations = FALSE), and, in that case, it would simply return the input data (restructured as an sfc object).

I'm not 100% sure if the output should be a POINT object (i.e. the common node) or a (degenerate) LINESTRING object (as you suggest). I would opt for the POINT object, but this is clearly open to discussion.

Btw: I briefly checked your R package, and it looks really good!

@jmlondon
Copy link
Author

@agila5,

I'm not sure what the best or most appropriate result would be either. I suggested the degenerate LINESTRING option because that was consistent with behavior when passing a SpatialLinesNetwork. I think there could also be some downstream benefits (or, at least, reduced headaches) in having sum_network_routes() always return a consistent geometry type (LINESTRING) ... especially when passing a vector of start and end values.

If you decide to go with POINT, I would suggest updating the behavior for SpatialLinesNetwork for consistency.

There is, also, an interesting philosophical (or semantic) question whether pathfound should be TRUE or FALSE in this situation. As a naive user, I was expecting it to still be TRUE. But, then again, is a path of length 0 still a path? hmmm

Regarding, {pathroutr}, thanks! I'm excited for the potential but it's still early days in development. I might be back here with questions or other issues as I try to scale it up to large and more realistic and more complex environments.

@Robinlovelace
Copy link
Member

Update on this @jmlondon yes I think this is a good idea. Many thanks for flagging the issue, LINESTRING consistently will definitely be more consistent.

Robinlovelace pushed a commit that referenced this issue Nov 28, 2020
* First ideas for #444

* try fix the scenario with degenerate and regular routes

* fix docs for sum_network_routes
@Robinlovelace
Copy link
Member

Great reproducible report @jmlondon and great fix @agila5.

Code shown below shows: it works 🎉

remotes::install_github("agila5/stplanr", "network_identical_nodes")

library(stplanr)
library(sf)
sln_sf <- SpatialLinesNetwork(route_network_sf)
line1 <- sum_network_routes(sln_sf, start = c(1, 2), end = c(9, 13))
line2 <- sum_network_routes(sln_sf, start = 1, end = 1) 
plot(sln_sf@sl$geometry)
plot(line1$geometry, add = TRUE, lwd = 5)
plot(line2$geometry, add = TRUE, lwd = 20)

image

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