Seeds #286
Merged
Seeds #286
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
downstream code on-the-fly. The approach implemented now changes the default of `seed` (and `.seed`) arguments from `NULL` to `sample.int(.Machine$integer.max, 1)`. For top-level functions which are only called by the user and not within projpred (should only be `cv_varsel()`, `project()`, and `proj_predict()`), this is not strictly necessary, but done for consistency with lower-level functions which are also called within projpred. The main advantage of generating seeds on-the-fly is that more seeds may be easily added and used in downstream code (which will be important for fixing issue stan-dev#268), without always having to add a new argument to top-level functions. Apart from that, this approach makes it possible that users set a seed once at the beginning of their script and then use the default `seed` (and `.seed`) arguments -- they will then get reproducible results which would not have been the case for the former implementation. However, due to the resetting of `.Random.seed`, this approach does not avoid yet that the same seed is re-used multiple times (which is probably bad practice from a theoretical point of view).
`.get_p_clust()` and in a new function for subsampling the parameter draws (`draws_subsample()`). This is only done for conceptual reasons (to point out where the seed is actually used).
`Error in qr.solve(decomposition, Q) : singular matrix 'a' in solve` in the K-fold CV for refmodel `rstanarm.glm.brnll.stdformul.without_wobs.without_offs`, perhaps due to unfortunate PRNG leading to linear separability).
reason is that this was probably done on purpose, so that the variable selection with all the data uses the same clustering (of the draws) as the variable selection(s) in `loo_varsel()` and `kfold_varsel()`.
the beginning of which a seed is set), don't reset the RNG state on exit. Consequently, at those places, a seed does not even need to be set. However, the latter is just a simplification. The important point is that the RNG state is not reset on exit which avoids that the same seed is re-used multiple times (which is probably bad practice from a theoretical point of view).
…bility (seed) test.
fweber144
added a commit
that referenced
this pull request
Apr 23, 2022
`predict.refmodel()`. In the future, it may be needed at other places, too. And with the new seed behavior of projpred (see PR #286), K-fold CV might actually be reproducible even if `brms_seed` is not set (I did not test this, though). So it's probably best to only vaguely hint at argument `brms_seed` in the vignette.
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.
With this PR, the seed supplied to top-level (user-facing) functions is used to generate new seeds for downstream code on-the-fly. The main advantage of this is that PRNG-using code may be added more easily (which will be important for fixing issue #268) and it also helps to avoid using the same seed multiple times, where possible (see the message of commit 6ed82b6).
Furthermore,
seed(and.seed) arguments now have a default ofsample.int(.Machine$integer.max, 1)instead ofNULLwhich allows users to set a seed once at the beginning of their script and then use the defaultseed(and.seed) arguments.If this is merged after the upcoming CRAN release, the version number for the
NEWS.mdchange performed here needs to be adapted.