Skip to content

Adding new knockoff statistic stat_ranger() #10

Open
ahrensmaike wants to merge 3 commits into
devfrom
feat-stat-ranger
Open

Adding new knockoff statistic stat_ranger() #10
ahrensmaike wants to merge 3 commits into
devfrom
feat-stat-ranger

Conversation

@ahrensmaike

Copy link
Copy Markdown
Collaborator

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 as stat_glmnet() (bb4cf). The updated version (3672b) uses a template/blueprint function stat_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 of
importance scores with NA values. This is due to the underlying {ranger} implementation and is not a bug in
the 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants