Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

select_parameters() outdated with respect to projpred #688

Open
fweber144 opened this issue Mar 31, 2022 · 4 comments
Open

select_parameters() outdated with respect to projpred #688

fweber144 opened this issue Mar 31, 2022 · 4 comments
Labels
Bug 🐛 Something isn't working Low priority 😴 This issue does not impact package functionality much

Comments

@fweber144
Copy link

It seems like select_parameters.stanreg() is outdated with respect to projpred:

  1. Line
    parameters <- row.names(projection$beta)
    uses element beta which doesn't exist in newer projpred versions anymore. I don't know when beta was removed exactly, but it doesn't exist in the current CRAN version 2.0.2 (from October 2020) anymore. To access the projected parameter draws in projpred, the as.matrix.projection() method can be used.
  2. Line
    projection <- projpred::project(selection, nv = projpred::suggest_size(selection), ...)
    uses argument nv which doesn't exist in newer projpred versions anymore (at least in the current CRAN version 2.0.2 from October 2020, it doesn't exist anymore). This argument was replaced by a new argument called nterms.

Apart from that, I noticed that parameters doesn't make use of the projected parameter draws: Lines

parameters <- row.names(projection$beta)
# Reconstruct formula
formula <- .reconstruct_formula(parameters, model)
# Update model
junk <- utils::capture.output(best <- suppressWarnings(stats::update(model, formula = formula, ...)))
best
show that only the names of the predictors from the selected submodel are used, not their projected draws. I don't know if this was done on purpose or if it was even necessary to make projpred-based selections fit into the parameters workflow, but in principle, it would be preferable to use the projected parameter draws instead of refitting the model with only the selected predictors. Refitting the model with only the selected predictors throws away uncertainty inherent to the reference model, thereby leading to improper post-selection inference. Using the projected parameter draws after selection is even one of the key steps of the projpred workflow, I would say.

@strengejacke strengejacke added the Bug 🐛 Something isn't working label Mar 31, 2022
@strengejacke
Copy link
Member

@DominiqueMakowski Currently, select_parameters() doesn't work anymore. I'm, not sure how it worked or how it is supposed to work now. Can you look at this?

@DominiqueMakowski
Copy link
Member

@fweber144 thanks for letting us know! Since you seem to have a pretty good understanding of it, would you be interested maybe in maybe opening a PR to make some fixes to select_parameters()?

@fweber144
Copy link
Author

Sorry, I currently don't have time for this. But I hope my comment above helps in fixing this.

strengejacke added a commit that referenced this issue Apr 4, 2022
@strengejacke strengejacke added the Low priority 😴 This issue does not impact package functionality much label Apr 4, 2022
@strengejacke
Copy link
Member

I moved the files for stanreg and brmsfit objects into the WIP folder for now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something isn't working Low priority 😴 This issue does not impact package functionality much
Projects
None yet
Development

No branches or pull requests

3 participants