Bugfix to avoid NAs in Surv.SD_Cext()

[nkehrein]

Current versions of dplyr::lag() work differently and no longer interpret matrix arguments as intended. This results in NAs being introduced in Surv.SD_Cext(). The attached patch fixes that.

[pveber]

Thanks for the patch! @virgile-baudrot Would you have time to review this PR?

[virgile-baudrot]

Look like there is indeed a problem, but not sure this PR solve it.

In a more general approach, I think we should avoid dependency to dplyr: fonction and their argument change too often.

[nkehrein]

Ah yes, I think you are right. The method is vectorised and my patch would undo that. A quick fix for that would be:

lambda.prec = t(dplyr::lag(t(lambda),1))
lambda.prec[,1] = lambda[,1]

[virgile-baudrot]

Great, I was thinking about this kind of thing.

Contrary to what I did at this time, I now prefer to avoid dependency. So I did this function:

# Function with dplyr
library(dplyr)
lagMatrixCol_1 = function(lambda){
  lambda.prec = t(dplyr::lag(t(lambda),1))
  lambda.prec[,1] = lambda[,1]
  return(lambda.prec)
} 
# Function without dependency
lagMatrixCol_2 = function(lambda){
  cbind(lambda[,1], lambda[,1:(ncol(lambda)-1)])
} 

Both methods give the same results:

lambda = matrix(runif(1e5), nrow = 1e2)
# test equality
all(lagMatrixCol_1(lambda) == lagMatrixCol_2(lambda))

[1] TRUE

Second is faster:

# compare speed
library(microbenchmark)
microbenchmark(
  lagMatrixCol_2(lambda),
  lagMatrixCol_1(lambda)
)

Unit: microseconds
                   expr   min     lq     mean  median      uq    max neval
 lagMatrixCol_2(lambda) 319.8 388.95  740.363  417.35  518.25 9065.2   100
 lagMatrixCol_1(lambda) 823.4 968.20 1837.913 1087.75 1324.65 9975.3   100

[virgile-baudrot]

Test predict function on Propiconazole data set

With this new implementation:

With ODE solver: