Skip to content
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

Feature: support SurvSHAP computation with {treeshap} #85

Merged
merged 37 commits into from
Oct 2, 2023

Conversation

kapsner
Copy link
Contributor

@kapsner kapsner commented Aug 31, 2023

This PR adds the feature to compute (global and local) SurvSHAP values with the {treeshap} algorithm, which drastically decreases computation time for ranger survival models as compared to using the currently implemented {kernelshap} algorithm.
Furthermore, this PR enables to compute SurvSHAP with models that were trained with data objects of type matrix.

Unit-tests have been added as well.

Addresses #75 .

The only thing I've just identified as a potential challenge is that {treeshap} is not on CRAN yet - are there plans to bring {treeshap} to CRAN in the near future?

kapsner and others added 22 commits April 4, 2023 17:26
…xactly one row

due to indexing of y_true to first element and if not using y_true the function produces strange results
including support for global survshap values and unit-tests
- removed data.table parts
- updated error handling
- added unit tests for treeshap (both, local and global shap)

addresses ModelOriented#75
…hod in surv_shap

which caused error of github action unittests when using 'exact_kernel'
@kapsner
Copy link
Contributor Author

kapsner commented Aug 31, 2023

Unit-test showed error regarding missing output_type argument when using "exact_kernel" -> this is fixed with 0e8c85c

@kapsner kapsner marked this pull request as draft August 31, 2023 10:39
@kapsner kapsner marked this pull request as ready for review August 31, 2023 12:49
@kapsner kapsner changed the title Feature: support computing SurvSHAP with {treeshap} Feature: support SurvSHAP computation with {treeshap} Aug 31, 2023
@kapsner
Copy link
Contributor Author

kapsner commented Sep 13, 2023

Hi @mikolajsp and @krzyzinskim ,
have you already had some time to have a look at this PR? Pls. let me know if you have any further questions.
Best, Lorenz

@krzyzinskim
Copy link
Collaborator

Hi @kapsner,
it looks great! Thanks so much for your contributions.

The only thing I've just identified as a potential challenge is that {treeshap} is not on CRAN yet - are there plans to bring {treeshap} to CRAN in the near future?

As you mentioned treeshap is not on CRAN, but we have already started to address this (you can look at my PR). I have slightly improved your implementation and added you also as contributor to the treeshap package. Once the stable version is ready, we will also accept this pull request too. Hopefully, it will be this week.

@kapsner
Copy link
Contributor Author

kapsner commented Sep 29, 2023

Hi @krzyzinskim ,

thanks a lot. The PR looks good and thanks for mentioning me as a contributor of the treeshap package!
Let me know if I can contribute anything else regarding the CRAN-submission of treeshap!

@krzyzinskim
Copy link
Collaborator

@kapsner, treeshap is already on CRAN, so that one issue is settled 🎉 Thanks for your willingness to help!

Unfortunately, I made one silly mistake while trying to generalize the functions for explanations also for CHFs. I've already managed to find the mistake and fixed it. However, using the version available on CRAN results in the explanations for the survival curves not being calculated properly. Sorry for that.

With CRAN's policy in mind, I don't want to do a new submission straight away. Maybe I will use this week or two to try to add support for another survival model (maybe randomForestSRC) and some other enhancements. Hope it's ok for you.

In the meantime, I'll merge this PR to a separate branch to work in parallel on the integration with survex.

@krzyzinskim krzyzinskim changed the base branch from main to treeshap October 2, 2023 11:29
@krzyzinskim krzyzinskim merged commit 0b2f4f5 into ModelOriented:treeshap Oct 2, 2023
7 checks passed
@kapsner
Copy link
Contributor Author

kapsner commented Oct 4, 2023

@krzyzinskim ,
Thank you very much for all of your efforts and congratulations that treeshap is on CRAN now 🎉

I understand the topic with the CRAN policy well - in the end, they want to avoid more than 6 or 7 CRAN submissions per year. I had that once with one of my packages, however, providing a reasonable explanation in the cran-comments allowed us to also update the package even more often (although this should be an exception).

Waiting for another survival model makes indeed sense 👍

For the treeshap package, I've also created another small PR adding my ORCID (ModelOriented/treeshap#32).

Best,
Lorenz

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