replacing vip::vi() call with internal functionality - #31
Open
amcim wants to merge 1 commit into
Open
Conversation
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.
The
vippackage was removed from CRAN and that is problematic for Bioconductor submission. We made the decision to replace the use of the package with our own functionality.The relevant call to vip was
vip::vi()inextractTopFeats(), which returns the features a model found most important for predicting AMR phenotype. Its output is what gets written to the*_top_features.tsvfiles and plotted byplotTopFeatsVI(). As such, this was replaced with with an internal.viGlmnet().That call goes through a few functions in vip:
vi(): wrapper; computes importance, drops NAs, sorts by decreasing importancevi_model(): S3 generic that picks the extraction method for the model typevi_model.model_fit(): unwraps the parsnip fit to the underlying engine objectvi_model.glmnet(): pulls the coefficients out of the glmnet fit and returns them asVariable,Importance(absolute coefficient), andSign(POS/NEG)This implemntation takes the fitted glmnet engine, drops the intercept, and returns a tibble of
Variable,Importance(|coefficient|), andSign, sorted by decreasing importance.I also added some tests which check that
.viGlmnet()returns whatvip::vi()did(the three columns, sorted by decreasing importance, withImportanceequal to the absolute coefficient at the minimum lambda), and that multi-class fits return per-class columns instead of erroring.One thing about this approach is that this currently only works for glmnet, which is the only engine we use currently. If we want to add more models later we will need to expand this functionality.