ropensci / stplanr

Sustainable transport planning with R
https://docs.ropensci.org/stplanr
Other
419 stars 66 forks source link

bug in geo_buffer #538

Closed wangzhao0217 closed 1 year ago

wangzhao0217 commented 1 year ago

Hi @Robinlovelace,

I have tried the method to define the CRS, but it still does not work: image image

so I have tried to use rnet_x_buffer = st_buffer(filtered_rnet_x, dist = dist, 'endcap=flat join=round'), and it seems not to work as well. image

Robinlovelace commented 1 year ago

Can you provide a reproducible example when you get a chance @wangzhao0217? Good idea to start with the data that we have in the NPT project, e.g. with the following:

https://github.com/ropensci/stplanr/blob/9661722564d550c24a62d7abcbb4ea8bff55435b/vignettes/merging-route-networks.Rmd#L179

Robinlovelace commented 1 year ago

Additional point/question: do we even need network simplification in these relatively remote places with low cycling potential? Thoughts @mem48 also welcome but my thinking is 'no'.

Robinlovelace commented 1 year ago

Can you try with the latest version of stplanr which you can install with remotes::install_dev("stplanr") @wangzhao0217 .

Any questions let me know.

Robinlovelace commented 1 year ago

Good morning @wangzhao0217 I've made some recent updates to the package that allow you to use geo_buffer() on projected data. So you can now do the rnet_merge() on projected data, e.g. EPSG:27700. I've done some tests here: https://github.com/ropensci/stplanr/blob/master/data-raw/test_geo_buffer.md

Does that fix the issue for you?

Robinlovelace commented 1 year ago

I think this can be closed now that geo_buffer() works with projected data. If there are any other issues, feel free to open @wangzhao0217, especially if you can illustrate them with reproducible example code like that shown in the link above.

wangzhao0217 commented 1 year ago

Hi Robin, The testing is not very smooth, thus I reopened the issue: I have created a markdown file (test geo_buffer with EPSG 27700.Rmd) and made the pr, please have a look.

if reading the sample data directly without doing st_transform, this will leave the CRS as EPSG 4326 Executing the code below will take an extremely long time to run :

rnet_x = sf::read_sf("data-raw/rnet_xp_clip.geojson")  # dim(rnet_x) 17934   2 "EPSG",4326
rnet_y = sf::read_sf("data-raw/sampled_rnet_yp.geojson") # dim(rnet_y) 1396   27 "EPSG",4326
rnet_merged = rnet_merge(rnet_x, rnet_y, dist = 20, segment_length = 10, funs = funs, max_angle_diff = 20, crs = "EPSG:27700") 

and encounter the error: Error in igraph::as.undirected(g) : At core/core/vector.pmt:443 : cannot reserve space for vector, Out of memory In addition: Warning messages: 1: attribute variables are assumed to be spatially constant throughout all geometries 2: In st_cast.sf(sf::st_cast(x, "MULTILINESTRING"), "LINESTRING") : repeating attributes for all sub-geometries for which they may not be constant

if conducting st_transform before rnet_merge, is the same takes an extremely long time to run:

rnet_x = st_transform(rnet_x, "EPSG:27700")
rnet_y = st_transform(rnet_y, "EPSG:27700")
brks = c(0, 100, 500, 1000, 5000,10000)
rnet_merged = rnet_merge(rnet_x, rnet_y, dist = 20, segment_length = 10, funs = funs, max_angle_diff = 20, crs = "EPSG:27700") 
Robinlovelace commented 1 year ago

I see the issue with rnet_merge() being slow but I don't think this is a bug with geo_buffer(). Will close this one and open another issue.