-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
Improve Pareto k-value warnings, use loo::sis()
for weighted draws
#438
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
needing `warn = 1` anymore.
… nonconstant weights.
…(they are `Inf` in that case), throw the PSIS warnings conditionally on global option `projpred.warn_psis`, and add a `TODO`.
…` (see the newly added inline comments for details).
weights (constant weights are necessary because nonconstant weights would have to be incorporated into the Pareto smoothing).
`capture.output()` to filter out some of them.
…pred.warn_subsampled_loo`.
…ditionally on global option `projpred.warn_kfold_refits`.
…bal option `projpred.warn_instable_projections`.
`validate_search = FALSE` submodel performance evaluation to refer to the original-draws (formerly: "complete-draws") Pareto k-values: Only checking for no original-draws Pareto k-values > 0.7 is not enough. When done precisely, the Pareto k-values themselves would need to be compared.
`testthat::test_that()` seems to redirect warning messages (probably so that it can throw them itself) so they are not captured correctly by `capture.output()`.
…arsel()` section.
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 improves the Pareto k-value warnings by suppressing the original warnings from the loo package and throwing customized warnings instead (thank you, @n-kall, for the suggestion).
While implementing this, I realized that if the projected draws used for the performance evaluation in the
validate_search = FALSE
case have different weights, thenloo::sis()
needs to be used instead ofloo::psis()
(because the weights would need to be taken into account when performing the Pareto smoothing). Hence, this fallback toloo::sis()
is implemented here as well (and apart from that, we now useloo::sis()
explicitly ifloo::psis()
would not perform the Pareto smoothing due to a small number of draws, but this is merely a safety measure, see the inline comments added in the code).In order to differentiate between different warnings in the unit tests, other warnings are now thrown conditionally on their own "hidden" global options.
Some enhancements for
capture.output()
usage infit_cumul()
andfit_cumul_mlvl()
are included as well (found when starting to usecapture.output()
inloo_varsel()
).Also added is a unit test that the warning for full Gaussian multilevel models added in #426 is thrown correctly (this test required the differentiation between different warnings, so it is included here).