JosiahParry / sfdep

A tidy interface for spatial dependence
https://sfdep.josiahparry.com/
GNU General Public License v3.0
124 stars 5 forks source link

Inefficient calculation of bivariate moran #28

Closed JosiahParry closed 2 years ago

JosiahParry commented 2 years ago

https://github.com/JosiahParry/sfdep/blob/45f8e539aba745358549497f70a335cf4f76f540/R/local-moran-bv-impl.R#L15

This calculation of the bivariate local moran is slow. It would be much more effecitve to calculate using x * st_lag(y, nb, wt). See benchmark below.

This can be replaced fairly simply. After changing local_moran_bv_calc() to include nb argument, local_moran_bv_impl() and local_moran_bv_perm_impl() will need to be modified to use the new calculation.

library(sfdep)

x <- guerry_nb$crime_pers
y <- guerry_nb$wealth
nb <- guerry_nb$nb
wt <- guerry_nb$wt

bench::mark(
  current = sfdep:::local_moran_bv_calc(x, find_xj(y, nb), wt),
  simpler = x * st_lag(y, nb, wt)
)
#> # A tibble: 2 × 6
#>   expression      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 current     107.5µs  115.6µs     8327.      24KB     16.7
#> 2 simpler      17.1µs   18.1µs    53472.     107KB     10.7

Created on 2022-08-05 by the reprex package (v2.0.1)

opelolo commented 2 years ago

Hi @JosiahParry please assign this to me. I'd love to give it a try. Thank you

opelolo commented 2 years ago

Claimed. I will work on it right away. Thank you

JosiahParry commented 2 years ago

@opelolo awesome!

For the function definition in line 15, I'd suggest the arguments to be x, y, nb, and wt. Then the body of the function can be x * st_lag(y, nb, wt). Then, update the instances where local_moran_bv_calc() is called

JosiahParry commented 2 years ago

Closed in https://github.com/JosiahParry/sfdep/pull/29