-
Notifications
You must be signed in to change notification settings - Fork 34
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
Adding the separate and surrogate regression method classes #379
Adding the separate and surrogate regression method classes #379
Conversation
for speed-up + such that pkgdown don't need the torch stuff
… This is to reduce the .rds save files size and to remove time dependent random path names
…to creatining fig/cache in separate sub folders
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. See some comments. I will look at the vignettes (and recompile them) separately.
python/shaprpy/explain.py
Outdated
kwargs: Further arguments passed to specific approaches. See R-documentation for more information about the | ||
approach specific arguments (https://cran.r-project.org/web/packages/shapr/shapr.pdf). Note that the parameters | ||
in R are called 'approach.parameter_name', but in Python the equivalent would be 'approach_parameter_name'. | ||
# TODO: discuss with martin how to get the descriptions passed here. Because we cannot use @inheritDotParams. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First I think we should refer to the pkgdown website for documentation.
What we could do to more easily refer to the ...-parameters in python is to create a new function in R called tripledot_docs or something like that which takes ... as input and where we use @inheritDotParams explain in the documentation, then we can link directly to that function from this documentation (still describing that you need to replace dots by underscores.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this function and updated the python documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you have moved from 2 spaced indentation to 4? I am Ok with that change, but making that change here makes it hard to spot what are actual code changes and not. I suggest to revert that to ease code review. I am sure there is quick-tool for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP 8 standard. I go back to non-standard formatting. We need to determine what to do in the future in another PR.
Merge remote-tracking branch 'refs/remotes/origin/Lars/shapr_regression' into Lars/shapr_regression # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
I have made all the alterations requested in the previous review, and I have built the vignettes. |
In this pull request, we add the option to use the separate and surrogate regression method classes discussed in Olsen et al. (2023). We use the packages in the
tidymodels
framework to specify the models, tune hyperparameters, and fit them.Key points in this pull request are the following:
shapr
.shapr
.verbose = 2
.shapr
to support explaining any tidymodels fitted using the workflow procedure; see the vignette.future
as usual inshapr
. The cross-validation procedure of the surrogate regression methods can be parallelized using the same package. Thefuture
package also parallelizes the prediction step for both the separate and surrogate regression methods.We have also made several changes to the Python version of
explain()
to make the methods work in Python.regression_separate
andregression_surrogate
in Python..utils
is loaded from.**kwargs
toexplain()
so that we can actually send approach-specific arguments. These must have the formapproach_parameter_name
in Python, but we added code to translate them to the R syntaxapproach.parameter_name
.compute_vS()
so that it splits between whether we use an MC-based or regression-based method.pyr2
do not support tidymodels, we remove all objects related toregression
in theinternal
list before converting them to Python objects and returning them.