tlverse / hal9001

🤠 📿 The Highly Adaptive Lasso
https://tlverse.org/hal9001
GNU General Public License v3.0
49 stars 15 forks source link

lambda parameter ignored by predict.hal9001 #58

Closed prockenschaub closed 4 years ago

prockenschaub commented 4 years ago

Issue predict.hal9001 ignores the lambda parameter and always shows predictions for all lambdas specified during fitting. Having had a cursory look at the source code, lambda does not seem to be used at all in predict.hal9001 but is used in predict.lassi.

This is not clear from the documentation, and led to confusions in my case since it makes the lambda parameter redundant and diverges from glmnet's behaviour. There might be theoretical reasons behind this but it would good to remove this parameter in this case, or state this behaviour clearly in the manual if the parameter is needed for compatibility with predict.lassi.

Reproducible example

# Simulated data taken from glmnet help page ?glmnet::glmnet
x = matrix(rnorm(100 * 20), 100, 20)
y = sample(1:2, 100, replace = TRUE)

# Fit a elastic net and predict with different lambda 
net_fit = glmnet(x, y, family = "binomial", lambda = c(0.001, 0.01, 0.1))
predict(net_fit, head(x), type = "response")  # Use lambda used during fitting
predict(net_fit, head(x), s = c(0.1, 0.05), type = "response") # Use custom lambdas
# --> glmnet correctly shows the specified lambdas, interpolating or refitting for the previously unseen lambda

# Fit a highly adaptive lasso model and try to predict with different lambda
hal_fit = fit_hal(X = x, Y = y, family = "binomial", lambda = c(0.001, 0.01, 0.1), cv_select = FALSE, fit_type = "glmnet")
predict(hal_fit, new_data = head(x), type = "response") # fitted lambdas 
predict(hal_fit, new_data = head(x), lambda = c(0.1, 0.05), type = "response") # custom lambdas
# --> hal9001 ignores the lambda parameter provided to predict, showing the same result in each case
nhejazi commented 4 years ago

Thanks for bringing this to our attention. As your example demonstrates, the predict method of fit_hal returns predictions for all lambdas specified in the original fit. Since it's easy to subset the matrix of predictions to those corresponding to lambdas of interest, our lambda argument wouldn't add much useful functionality (even if it worked). Rather than have the predict method interpolate / re-fit for lambdas not originally specified in the fit_hal call, we'll just remove the lambda argument to the predict.hal9001 method and require that users specify all lambdas of interest in the original call to fit_hal. We'll resolve this in a PR shortly.

nhejazi commented 4 years ago

Resolved by #59