Function crossCorrelation - Change of code arrangement?

[MarHer90]

Hi!
First of all, thanks for the immediate change considering my first issue that I posted. Testing the change, i came across another issue that might be worthwhile solving.
Whilst testing the change, I provided the function with a prespecified weight matrix. I took me some time to realize that I had to set the dist.function argument to some arbitrary "none" to stop the function from altering the provided weight matrix.
As I was looking at the code, it seems as if this issue could be avoided since I think the dist.functions are only necessary if there is a new weight matrix to generate. So i changed the code arrangement to avoid an alteration of the prespecified weight matrix no matter the dist.function argument given. At least for the one example I tested it seemed to work fine.

This is the old arrangement (lines 109 - 136) of the line code

if( is.null(w) ) {
if( is.null(coords) ) stop("If no Wij matrix is provided, a coordinates matrix is required")
w <- sp::spDists( coords )
} else {
if(!class(w)[1] == "matrix") stop("Spatial weights must be in matrix form")
if(ncol(w) != length(x) | nrow(w) != length(x)) stop("Spatial weights matrix must be symmetrical and match x")
w[which(is.na(w))] <- 0
}
if( dist.function == "inv.power" ) {
message("Calculating spatial weights matrix using inverse power function")
w <- 1 / w
diag(w) <- 0
w <- w / sum(w)
} else if (dist.function == "neg.exponent") {
message("Calculating spatial weights matrix using negative exponent")
diag(w) <- NA
mu <- mean(w, na.rm=TRUE)
for(i in 1:nrow(w)) {
for(j in 1:nrow(w)) {
w[i,j] <- round(exp( (-2 * w[i,j]) / mu ),6)
}
}
diag(w) <- 0
} else if (dist.function == "none") {
message("Wij matrix is being left raw")
} else {
stop("Not a valid matrix option")
}

This would be the altered arrangement:

if( is.null(w) ) {
if( is.null(coords) ) stop("If no Wij matrix is provided, a coordinates matrix is required")
w <- sp::spDists( coords )
if( dist.function == "inv.power" ) {
message("Calculating spatial weights matrix using inverse power function")
w <- 1 / w
diag(w) <- 0
w <- w / sum(w)
} else if (dist.function == "neg.exponent") {
message("Calculating spatial weights matrix using negative exponent")
diag(w) <- NA
mu <- mean(w, na.rm=TRUE)
for(i in 1:nrow(w)) {
for(j in 1:nrow(w)) {
w[i,j] <- round(exp( (-2 * w[i,j]) / mu ),6)
}
}
diag(w) <- 0
} else if (dist.function == "none") {
message("Wij matrix is being left raw")
} else {
stop("Not a valid matrix option")
}
} else {
if(!class(w)[1] == "matrix") stop("Spatial weights must be in matrix form")
if(ncol(w) != length(x) | nrow(w) != length(x)) stop("Spatial weights matrix must be symmetrical and match x")
w[which(is.na(w))] <- 0
}

[jeffreyevans]

I actually just made this change and it was intentional. The reason behind this change is that somebody may pass a w matrix that already represents the desired Wij weights “or” a distance matrix representing an alternative distance hypothesis (eg., road verses straight line) that still needs to be converted to spatial weights thus, two potential operations on a specified `w` argument as denoted by the `dist.function` argument. I realized this with Chen’s example data where, the matrix represents railroad and not straight-line geographic distances. In this case there is a specified input matrix but, it still needs to be transformed into a spatial weights matrix. So, the arguments `cords`, `w` and `dist.function` work together is account for different potential inputs. 1. (if coords=x, w=NULL, dist.function= c("inv.power", "neg.exponent")) A distance matrix is derived using the data passed to coords then spatial weights derived using one of the dist.function options 2. (if cords=NULL, w=x, dist.function= c("inv.power", "neg.exponent")) It is expected that the distance matrix specified with w represent some form of distance then the spatial weights are derived using one of the dist.function options 3. (if cords=NULL, w=x, dist.function="none") It is assumed that the matrix passed to w already represents the spatial weights Now, that said I certainly admit that the code could be more optimized. The dist.function="none" was just a quick way to account for alternate distance hypotheses with the existing code. I really do not want to set the default as “none” because the function is fairly automated if a spatial object along with x and y are specified, which is a common usage. If you have any code suggestions that would account for this please feel free to forward them along and I will incorporate them. Best, Jeff Jeffrey S. Evans, Ph.D., | Senior Landscape Ecologist & Biometrician The Nature Conservancy | Protected Lands Science Visiting Professor | University of Wyoming | Zoology & Physiology Laramie, WY | ***@***.******@***.***> | (970) 672-6766<tel:(970)%20672-6766> From: MarHer90 ***@***.***> Sent: Wednesday, May 5, 2021 1:42 AM To: jeffreyevans/spatialEco ***@***.***> Cc: Subscribed ***@***.***> Subject: [jeffreyevans/spatialEco] Function crossCorrelation - Change of code arrangement? (#28) Hi! First of all, thanks for the immediate change considering my first issue that I posted. Testing the change, i came across another issue that might be worthwhile solving. Whilst testing the change, I provided the function with a prespecified weight matrix. I took me some time to realize that I had to set the dist.function argument to some arbitrary "none" to stop the function from altering the provided weight matrix. As I was looking at the code, it seems as if this issue could be avoided since I think the dist.functions are only necessary if there is a new weight matrix to generate. So i changed the code arrangement to avoid an alteration of the prespecified weight matrix no matter the dist.function argument given. At least for the one example I tested it seemed to work fine. This is the old arrangement (lines 109 - 136) of the line code if( is.null(w) ) { if( is.null(coords) ) stop("If no Wij matrix is provided, a coordinates matrix is required") w <- sp::spDists( coords ) } else { if(!class(w)[1] == "matrix") stop("Spatial weights must be in matrix form") if(ncol(w) != length(x) | nrow(w) != length(x)) stop("Spatial weights matrix must be symmetrical and match x") w[which(is.na(w))] <- 0 } if( dist.function == "inv.power" ) { message("Calculating spatial weights matrix using inverse power function") w <- 1 / w diag(w) <- 0 w <- w / sum(w) } else if (dist.function == "neg.exponent") { message("Calculating spatial weights matrix using negative exponent") diag(w) <- NA mu <- mean(w, na.rm=TRUE) for(i in 1:nrow(w)) { for(j in 1:nrow(w)) { w[i,j] <- round(exp( (-2 * w[i,j]) / mu ),6) } } diag(w) <- 0 } else if (dist.function == "none") { message("Wij matrix is being left raw") } else { stop("Not a valid matrix option") } This would be the altered arrangement: if( is.null(w) ) { if( is.null(coords) ) stop("If no Wij matrix is provided, a coordinates matrix is required") w <- sp::spDists( coords ) if( dist.function == "inv.power" ) { message("Calculating spatial weights matrix using inverse power function") w <- 1 / w diag(w) <- 0 w <- w / sum(w) } else if (dist.function == "neg.exponent") { message("Calculating spatial weights matrix using negative exponent") diag(w) <- NA mu <- mean(w, na.rm=TRUE) for(i in 1:nrow(w)) { for(j in 1:nrow(w)) { w[i,j] <- round(exp( (-2 * w[i,j]) / mu ),6) } } diag(w) <- 0 } else if (dist.function == "none") { message("Wij matrix is being left raw") } else { stop("Not a valid matrix option") } } else { if(!class(w)[1] == "matrix") stop("Spatial weights must be in matrix form") if(ncol(w) != length(x) | nrow(w) != length(x)) stop("Spatial weights matrix must be symmetrical and match x") w[which(is.na(w))] <- 0 } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#28>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACLKH7ZWJCGZINKFLVERHKDTMDZFJANCNFSM44EERNWQ>.

[MarHer90]

Thanks again for the quick reply!

Given your explanation, I see that the way the code works now allows for a more flexible usage of the function. My confusion seems to have come from reading the current source code in conjunction with the package vignette explanation hosted on CRAN, so I missed the change that has been made in the function description hosted on github. That said, shouldn't the "none" option also be added in the function code (line 118) for completedness? I'm not sure if that alters the function internals, which also brings me to my last point: I'm not that experienced in R coading (or coading in general), so my code is often a "quick and dirty" kind of thing. Therefore, i don't expect to provide any essential code improvements, but I will look into it and other functions as I proceed working with the package.

Best wishes,

Marco