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

Inefficient calculation of bivariate moran #28

Closed
JosiahParry opened this issue Aug 5, 2022 · 4 comments
Closed

Inefficient calculation of bivariate moran #28

JosiahParry opened this issue Aug 5, 2022 · 4 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@JosiahParry
Copy link
Owner

local_moran_bv_calc <- function(x, yj, wt) {

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)

@JosiahParry JosiahParry added the good first issue Good for newcomers label Aug 5, 2022
@opelolo
Copy link
Contributor

opelolo commented Aug 5, 2022

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

@opelolo
Copy link
Contributor

opelolo commented Aug 5, 2022

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

@JosiahParry
Copy link
Owner Author

@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
Copy link
Owner Author

Closed in #29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants