Fix seeds/PRNG usage #412
Merged
Fix seeds/PRNG usage #412
Conversation
This file contains 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
use `NA` for seed arguments by default (to avoid that the PRNG state (`.Random.seed`) is reset after each call to a function which has a seed argument, which causes PRNG results to be less random than they should be).
`cv_varsel()` themselves. This concerns K-fold CV based on `cvfun`. The breaking change consists of the possibility of different results since the PRNG state when generating the CV folds would be a different one when using the new default of `seed = NA` in `cv_folds()` (so the folds could be composed differently). Using `seed = sample.int(.Machine$integer.max, 1)` in this special `cv_folds()` call should not be a problem as the PRNG state that `cv_folds()` resets to upon exit is never used again for generating a seed. (After this special `cv_folds()` call, the next PRNG-using code is `kmeans()`.) Nevertheless, this complicated construction can be avoided in the feature when introducing other breaking changes (e.g., in an upcoming UI overhaul).
because argument `seed` (or `.seed`) might consist of a lazily evaluated expression that makes use of the PRNG (so `seed` in `if (!is.na(seed))` advances the PRNG state).
did with the former default of `.seed = sample.int(.Machine$integer.max, 1)`, but I realized this only now.
…upt. Enhanced this now.
… perform better than the reference model.
`summary.vsel()` call to have the performance metrics table really match the plot.
…s changed due to the new PRNG state).
|
I forgot to add a short illustration of the issue and the fix for it: data("df_gaussian", package = "projpred")
df_gaussian <- df_gaussian[1:29, ]
dat <- data.frame(y = df_gaussian$y, df_gaussian$x)
library(rstanarm)
rfit <- stan_glm(y ~ X1 + X2 + X3 + X4 + X5,
data = dat,
chains = 1,
iter = 500,
seed = 1140350788,
refresh = 0)
### Use the installed version 2.5.0 (or load it via devtools::load_all()):
library(projpred)
###
### Load a version > 2.5.0 containing the fix for this seed issue (or
### attach it via `library(projpred)` if installed):
# devtools::load_all()
###
set.seed(467845213)
vs1 <- varsel(rfit,
nclusters = 23,
nclusters_pred = 25)
vs2 <- varsel(rfit,
nclusters = 23,
nclusters_pred = 25)
smmry1 <- summary(vs1)
smmry1 <- smmry1[setdiff(names(smmry1), "family")]
smmry2 <- summary(vs2)
smmry2 <- smmry2[setdiff(names(smmry2), "family")]
identical(smmry1, smmry2)
## --> `TRUE` with projpred 2.5.0, `FALSE` with a projpred version > 2.5.0
## containing the fix for this seed issue.
vs1 <- vs1[setdiff(names(vs1), c("refmodel", "search_path"))]
vs2 <- vs2[setdiff(names(vs2), c("refmodel", "search_path"))]
identical(vs1, vs2)
## --> `TRUE` with projpred 2.5.0, `FALSE` with a projpred version > 2.5.0
## containing the fix for this seed issue.
|
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 is a major fix for PRNG usage within projpred, see the
NEWS.mdentries added here. The commit messages provide more details.With this PR, we also make use of the fix in the vignettes: Now, we only need to set a seed once at the beginning (except for the comparison of latent and traditional projection, but this has nothing to do with the seeds issue fixed here).