Adding new knockoff statistic stat_ranger() #10
Open
ahrensmaike wants to merge 3 commits into
Open
Conversation
…des computation skeleton for modeling and importance logic
This file contains hidden or 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
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 provides a new statistic to be used with
knockoff.statistics()based on{ranger}implementation for random forests and the importance measures supported by{ranger}.It starts with defining
stat_ranger()directly in full analogy to the existing statistics such asstat_glmnet()(bb4cf). The updated version (3672b) uses a template/blueprint functionstat_custom()that takes a function for modeling and importance computation and takes care of the input checks, swapping and post-statistic-computation sign correction to correct for the initial swapping, which makes testing and maintenance more straightforward. If you like the idea of the blueprint function, it could be used internally as well and/or be mentioned in the vignette to make writing custom statistics even easier for the package user.Either way,
stat_ranger()tests are added to the existing test suite. We introduced helpers for the test suite here as basis for discussion, happy to remove if you prefer all tests to be fully self-contained.Regarding the known issues mentioned in #4: In very rare cases, the
stat_ranger()function may return a vector ofimportance scores with
NAvalues. This is due to the underlying{ranger}implementation and is not a bug inthe new statistic. The issue has been discussed before, but got closed due to lack of reproducibility. We have recently opened
a new ranger issue with a more detailed problem description and first steps to solve the issue. We hope for it to get fixed in a future version of
{ranger}.Since it is rare to begin with, the current approach in
stat_ranger()is to rerun the computation up to two times if any variable importance could not be computed. In case the issue persists, we recommend to rerun the analysis with a different random seed.closes #4
co-authored by svoss09: sebastian.voss.ext@bayer.com