Repository navigation
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was assisted by Claude.
Summary
In the review of #588, @aloctavodia suggested that
get_refmodel.stanreg()andbrms::get_refmodel.brmsfit()could eventually accept acvrefbuilderargument. This PR adds the argumentscvfun = NULLandcvrefbuilder = NULLtoget_refmodel.stanreg(). A function in one of these arguments replaces the internal default. A value that is not a function and notNULLgives an error.Before this PR,
get_refmodel.stanreg()built its owncvfunandcvrefbuilderand passed them toinit_refmodel()together with.... Acvfunorcvrefbuilderin the call therefore stopped with an error, for exampleformal argument "cvrefbuilder" matched by multiple actual arguments. #588 documented a workaround for the custom latent-projection functions: replace elementcvrefbuilderof the reference model object afterget_refmodel(). For rstanarm reference models, argumentcvrefbuildernow replaces this workaround.The default
cvrefbuilderpasses...to the fold-wiseget_refmodel()calls. The new arguments are not in..., so the fold-wise reference model objects get the defaults. The fold-wise objects do not usecvfunorcvrefbuilder.Documentation
?init_refmodel: the entries ofcvfunandcvrefbuilderdescribe the defaults ofget_refmodel.stanreg().?extend_familyand the latent-projection vignette (section "K-fold CV with custom response-scale functions") now tell rstanarm users to pass the customcvrefbuilderas an argument.brms::get_refmodel.brmsfit()has acvfunargument but nocvrefbuilderargument yet, so for brms the text keeps the workaround. A brms PR that addscvrefbuilderfollows. After a brms release with it, a small projpred PR can update the brms text.Tests
test_refmodel.Rhas 3 new test blocks:run_cvfun()calls the givencvfunonce with the given folds.cv_varsel()withcv_method = "kfold"calls the givencvrefbuilderonce for each fold, in the order of the folds. The givencvfunreturns the full-data fit for each fold, so the test needs no refit.cvfun, the reference model object uses the defaultcvrefbuilder. The fold-wise reference model object does not get the givencvfun, and it takessigmafrom the fold-wise fit. With onlycvrefbuilder, the reference model object uses the defaultcvfun.cvfunorcvrefbuilderthat is not a function gives the error "Argumentcvfunmust be a function orNULL." (or the same error forcvrefbuilder).Without the change in
R/refmodel.R, 2 of the new blocks stop with the "matched by multiple actual arguments" error, and the third has 2 failures.The full test suite on master e1dd3bc with this PR (
NOT_CRAN=true) gave 57 269 expectations, 0 failures, 0 errors and 5 skips. Versions: R 4.5.3, testthat 3.3.2, rstanarm 2.32.2, brms 2.23.1, lme4 2.0.6. The 19 warnings come from CHOLMOD intest_div_minimizer.Rand from the generalized Pareto fit and Pareto k in the brmscvfitstest oftest_varsel.R. This PR does not change these two test files.