comment the redundant run_lasso in fit_itr, as it was overwritting lasso in caret algorithms

[yanisvdc]

For both cv = False and cv = True in main.R, lines 310 (sample-splitting) and 589 (k-fold cv) in the function fit_itr.

[jialul]

Hi @yanisvdc, thanks for bringing it up. The arguments cv = FALSE and cv = TRUE are both used for estimating the ITR, depending on whether to estimate ITR via sample-splitting or cross-validation. If you want to tune the hyperparameter via cross-validation, you can pass trControl = fitControl to the package. You can find a detailed example here. Hope it helps!

[yanisvdc]

Hi @jialul, it makes sense, however, my understanding is that, when doing:

fit_cv<-estimate_itr(treatment='A',form=U ~ A + W + L,data=data,algorithms='lasso',budget=0.5)

The results, line 238

fit_ml[[algorithms[i]]] <- caret_est$test
models[[algorithms[i]]] <- caret_est$train

will be overwritten by the results line 322, for the algorithm "lasso", as the name is the same between caret and our custom run_lasso.

fit_ml[["lasso"]] <- est$test
 models[["lasso"]] <- est$train

The same overwrite will happen between line 516 and line 589, which is why I suggested to comment out the custom run_lasso. We could also give the custom lasso a different name like "custom_lasso" instead of "lasso" to distinguish it from the caret "lasso".

I believe it would be fine to simply comment out in this case.

Please let me know if that clarifies the issue or if I missed something.

Thanks,
Yanis

[jialul]

Thanks for providing further info Yanis! I understand your question now. This issue has been resolved in the developer branch with this commit on Sep 24, but has not yet reflected on the CRAN version. The internal facing function is named as lasso_developer but no long lasso to avoid overwriting. We are currently implementing the multi-value treatment in the developer version, so these new updates are not pushed to CRAN yet.

For now, I think it's easier to use other lasso models from caret and pass trControl = fitControl to the package to work around this issue, or use the developer version directly. I'll resolve this thread once we updated the new version on CRAN. Thanks again for bringing it up!