ndraws and nclusters revisited
#291
Merged
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
clustering to be used, so it should be less confusing now).
the section for the most recent version. This should avoid a lot of confusion and make the `NEWS` file easier to read when it is published on CRAN.
…usters`, `ndraws_pred`, and `nclusters_pred`.
the section for the most recent version. This should avoid a lot of confusion and make the `NEWS` file easier to read when it is published on CRAN." Add another important commit which used to be included in v2.0.4.
…f `ndraws <= 20`. Thereby, most of the related code is moved to `.get_refdist()` which is also simplified at multiple places.
throw a warning if `ndraws <= 20`. Thereby, most of the related code is moved to `.get_refdist()` and a corresponding change is also necessary for `summary()` output.
using it to suppress warnings in the tests.
…ed` variants) in summary output.
…ws` and turn the warning into a message.
avoid an inconsistency for `datafit`s, for example (but generally for all situations where `nclusters == S == 1`). This also allows to remove the special `S == 1` case in the `nclusters == 1` case.
unit tests with `run_more = TRUE`).
init_refmodel()) works for rstanarm reference models" runs through.
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.
In a personal communication some weeks ago (@avehtari, @paul-buerkner, @yannmclatchie), we talked about how arguments
ndrawsandnclusters(and their*_predvariants) should be changed before the next CRAN release. This PR implements the discussed changes, in particular:ndrawsdefault tonclusters.ndrawstonclusters(and neither for their*_predvariants).ndraws <= 20(orndraws_pred <= 20).I moved most of the input-checking code for
ndrawstonclusters(and their*_predvariants) to.get_refdist()because this allows for a lot of simplifications (thereby avoiding redundancies). It also seems like the most natural place. The updated.get_refdist()is also simplified at multiple places. Updating.get_refdist()required a few changes with regard to how post-processing functions likesummary.vsel()can get information about the number of projected draws and whether clustering was used. However, this turned out to be quite straightforward and even safer (by using the output of.get_refdist()). So the changes with respect to the post-processing functions again seem like the most natural approach to me. The informational message mentioned above is now directly implemented in the updated.get_refdist()(this again shows how it can avoid redundant code).So with this PR, we should be ready for the new CRAN release. Most of my statements from this PR comment apply again, slightly modified:
Note that automated checking via Travis or GitHub Actions still seems to be deactivated (see issue #236). I have run
R CMD checklocally on my available machines (Ubuntu 20.04.3 LTS and Windows 10) and it succeeded. I also checked withdevtools::check_win_devel(),devtools::check_win_oldrelease(), anddevtools::check_win_release():gh-pageshas been updated, see below).Given these check results, I think it would be good if someone could double-check if everything is ready for a CRAN release, especially with regard to the test failure on R devel (4.2.0 alpha).
If possible, it would be great if the recommendations from this "R Packages" book section (in particular, the creation of a version tag) could be followed after the CRAN release. Furthermore, after the CRAN release, I can create a PR for updating branch
gh-pages(I have already prepared that locally), so the pkgdown site is in sync with the CRAN version of the package.