-
Notifications
You must be signed in to change notification settings - Fork 96
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
layerCor problem with pearson correlation when NA values are present #1034
Comments
Thanks. I removed the NA's for each layer, instead of removing the cells that are NA in any of the two layers in a pair. I believe this is fixed now, and I get:
This could be make much more efficient by running this in one step in C++ |
Thanks for the fix. You mean using a cpp function to compute the correlation? I will look into that. |
I have now implemented pearson in C++; making it much faster, I think. |
Before I think it was a bit slower than directly using library(terra)
set.seed(1234)
n <- 10000
r1 <- rast(matrix(runif(n ^ 2), ncol = n))
r2 <- r1+rast(matrix(runif(n ^ 2,0,0.5), ncol = n))
system.time({c1<-cor(values(r1)[, 1], values(r2)[, 1], use = "complete.obs", method = "pearson")});c1
## utilisateur système écoulé
## 10.23 3.51 13.94
## [1] 0.8944202
system.time({c2<-layerCor(c(r1, r2), "pearson", na.rm = TRUE)$pearson});c2
## utilisateur système écoulé
## 6.84 3.70 10.68
## [,1] [,2]
## [1,] 1.0000000 0.2250657
## [2,] 0.2250657 1.0000000 |
Thanks so much for checking that. The mistake was for cases where the rasters are large and get processed in chunks. I blieve this is fixed now
It also works for your example. |
Hi!
While searching for a faster way to correlate two rasters, I found a bug in
layerCor
for pearson correlations whenNA
values are present. Here is an example:Or is it because there is an underlying filling/interpolation that is done which changes the values? The same correlation is found when there are no missing values or when the missing values are the same in both rasters. Sometimes, the correlation even goes over 1.
Thanks!
François
The text was updated successfully, but these errors were encountered: