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

New function — ms_union #170

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

atsyplenkov
Copy link

Hi guys,

whenever you have a moment, could you please take a look at this? I propose to add ms_union function family which is a wrapper to -union command.

Cheers

@ateucher
Copy link
Owner

ateucher commented Jan 8, 2024

Thanks for this @atsyplenkov. Does this have very different behaviour to sf::st_union()?

@atsyplenkov
Copy link
Author

atsyplenkov commented Jan 8, 2024

Hi @ateucher! As far as I understand, not really. However, it does provide a significant increase in speed. See a quick reprex below.

UPD: ms_union is even faster than terra::union

library(sf)
#> Linking to GEOS 3.11.2, GDAL 3.7.2, PROJ 9.3.0; sf_use_s2() is TRUE
library(rmapshaper)
library(terra)

file <- system.file("gpkg/nc.gpkg", package = "sf")
nc_sf <- read_sf(file)

nc_sf_buffer <- 
  st_buffer(nc_sf, 10^4)

# Convert to terra objects
nc_vect <- vect(nc_sf)

nc_buffer <- vect(nc_sf_buffer)

microbenchmark::microbenchmark(
  st_union = {
    
    st_union(nc_sf, nc_sf_buffer)
    
  },
  ms_union = {
    
    ms_union(nc_sf, nc_sf_buffer)
    
  },
  union = {
    
    terra::union(nc_vect, nc_buffer)
    
  },  
  times = 10L
)

#> Unit: milliseconds
#>      expr        min        lq       mean     median         uq        max
#>  st_union 16926.3117 17169.587 17528.1777 17483.3264 17780.6008 18183.9126
#>  ms_union   420.9673   440.738   460.7762   452.6254   477.9516   518.3586
#>  union 3465.1852 3492.5957 3562.6814 3537.1537 3599.4537 3726.8144
#>  neval cld
#>     10  a 
#>     10   b
#>     10   c

Created on 2024-01-09 with reprex v2.0.2

@ateucher
Copy link
Owner

That's pretty good! Thanks @atsyplenkov, I will probably take this PR, but I won't have time to work on the package for a little while, so I will look at it in detail then.

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

Successfully merging this pull request may close these issues.

2 participants