Typo in NA handling in comparePerChange() function

[carrieholt]

I think the brackets should be moved on the right side of these two lines https://github.com/SOLV-Code/MetricsCOSEWIC/blob/94d05ffe62475b0a77a61a2eb8e1b360e1d88c48/R/Module_comparePercChange.R#L39-L40

I think we want to na.skip when more than half the dataset are NAs, correct? The '/2' needs to be after the length() function. Currently the /2 is ignored. JAGS works with NAs, and Rstanarm deletes cases with NAs, which should be fine if indexing remains aligned with years with data. It should read:

if(sum(!is.na(du.df.sub[,2])) < length(du.df.sub[,2])/2 ){na.skip <- TRUE} # this results in NA outputs, but stops crashing if(sum(!is.na(du.df.sub[,2])) >= length(du.df.sub[,2])/2 ){na.skip <- FALSE}

However, I see that that this function also calls calcPerChangeSimple(), and in that function there is a line to ouput pchange = NA if there are any NAs in the input time-series, which is perhaps why this typo went unnoticed. I'll submit another issue for fixing calcPerChangeSimple().

[carrieholt]

I've added these 3 changes (from issues 65, 66, 67) in a branch "temp", with a few other changes to allow rstanarm to work with the updated benchmarks c(-30, -50, -70). Feel free to merge it, or extract what you want from it, or I can merge.

[SOLV-Code]

see notes on issue #66. I think the same questions apply here. but maybe in the Bayesian framework this gets handled through wider bounds on the estimate the more NAs you have?

[carrieholt]

Yes, the probability distribution should be wider with more NAs.