mfasiolo / qgam

Additive quantile regression R package
http://mfasiolo.github.io/qgam/
30 stars 7 forks source link

undesirable effect of do.call #30

Closed hyanworkspace closed 5 years ago

hyanworkspace commented 5 years ago

Hi Matteo, after looking into the code, maybe the do.call("gam", list(parameters)) may have undesirable effects. Typically, the last element of a normal gam object is only the call object, but by do.call we find a list of function object, formula and the training dataset. Are these elements necessary for other functions ?

https://github.com/mfasiolo/qgam/blob/721bb830048e7dfc69f72c1377a25e650982ca7b/R/tuneLearn.R#L119

mfasiolo commented 5 years ago

Hi Hui,

thanks poiting this out, for sure this is expensive in terms of memory. I use do.call to avoid using the "..." which is sometimes problematic. Maybe the easiest solutions is doing:

library(mgcv)
set.seed(2) ## simulate some data... 
dat <- gamSim(1,n=400,dist="normal",scale=2)

form <- y~s(x0)+s(x1)+s(x2)+s(x3)

argGam <- list()

# Standard call 
b <- gam(form, data=dat)

# do.call 
b1 <- do.call("gam", args = c(list("formula" = quote(form), "data" = quote(dat)), argGam), quote = FALSE)

b$call    # gam(formula = form, data = dat)

b1$call  # gam(formula = form, data = dat)

The problem with this is that it will still evaluate the elements of argList (which is empty here). I'm thinking about how that could be avoided (but maybe it's not a problem in general).

mfasiolo commented 5 years ago

qgam version 1.3 implements this solution (i.e. using quote()), see a5326fadc51ba318b8a38bc468cecb4df1b92139

But maybe it would be good to come back to this and some point, with a better solution,