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
Fixes and other improvements for argument d_test
of varsel()
#341
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
submodels also needs to be evaluated on the test data (not only the predictive performance of the reference model).
even cause confusion and be used incorrectly, given the new `.get_sub_summaries()` arguments).
`test_points`, namely `NULL` (see argument `obs` of `fetch_data()`).
tedious and not necessary. So add that element internally.
use only). Also update `NEWS.md`.
`<vsel_object>$summaries` and `<vsel_object>$d_test` as in the original dataset (or test dataset, in case of a non-`NULL` argument `d_test` of `varsel()`, but that doesn't concern K-fold CV).
`<vsel_object>summaries$sub` and `proj_linpred()` as well as between `<vsel_object>summaries$ref` and `posterior_epred()` / `log_lik()` in case of a non-`NULL` object `d_test`.
fweber144
added a commit
to fweber144/projpred
that referenced
this pull request
Jul 15, 2022
The re-ordering of the `summaries` results according to the original order of observations (see PR stan-dev#341) needs special care in case of augmented-length vectors.
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.
As stated in the new
NEWS.md
entries, this PR:d_test
ofvarsel()
: Not only the predictive performance of the reference model needs to be evaluated on the test data, but also the predictive performance of the submodels.d_test
ofvarsel()
as an internal feature anymore. This was possible after fixing the bug ford_test
mentioned above.<vsel_object>$summaries
and<vsel_object>$d_test
now always corresponds to the order of the observations in the original dataset (except if<vsel_object>
was created by a call tovarsel([...], d_test = <non-NULL_d_test_object>)
, in which case the order of the observations in those subelements corresponds to the order of the observations in<non-NULL_d_test_object>
). The only case not following this rule up to now was K-fold CV.Apart from that, the existing
d_test
tests is enhanced and a new test is added which tests that whend_test
is set to actual test data, the<vsel_object>summaries$sub
results can be reproduced byproj_linpred()
and the<vsel_object>summaries$ref
results can be reproduced byposterior_epred()
andlog_lik()
.Some refactoring of
d_test
-related code is also performed, to enhance readability and consistency.