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.

Sign up for GitHub

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

Jump to bottom

TreeExplainer extensions #4697

Merged
merged 28 commits into from
May 11, 2022
Merged

TreeExplainer extensions #4697

merged 28 commits into from
May 11, 2022

Conversation

RAMitchell
Copy link
Contributor

@RAMitchell RAMitchell commented Apr 12, 2022 •
edited
Loading

Stacked on #4671.

  • Remove extra redundant class in python layer.
  • Simplify the interface between C++ and python using variants.
  • Fix [BUG] TreeExplainer makes incorrect predictions for 64 bit data #4670 by allowing double precision data
  • Document TreeExplainer
  • Add interventional shap method
  • Add shapley interactions and taylor interactions
  • Promote from experimental
  • Support sklearn estimator types from xgb/lgbm (i.e. no need to convert to booster before using TreeExplainer)

@github-actions github-actions bot added CUDA/C++ Cython / Python Cython or Python issue labels Apr 12, 2022
@RAMitchell RAMitchell marked this pull request as ready for review April 13, 2022 11:21
@RAMitchell RAMitchell requested review from a team as code owners April 13, 2022 11:21
@RAMitchell
Copy link
Contributor Author

The segmentation fault again: https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-22-06/job/cuml/job/prb/job/cuml-gpu-test/CUDA=11.0,GPU_LABEL=driver-450,LINUX_VER=centos7,PYTHON=3.8/58/consoleText

It is only appearing on centos 7.

hcho3
hcho3 approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@hcho3 hcho3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This was referenced Apr 27, 2022
Merged
Closed
@RAMitchell
Copy link
Contributor Author

rerun tests

3 similar comments
@RAMitchell
Copy link
Contributor Author

rerun tests

@hcho3
Copy link
Contributor

hcho3 commented May 3, 2022

rerun tests

@hcho3
Copy link
Contributor

hcho3 commented May 3, 2022

rerun tests

@hcho3
Copy link
Contributor

hcho3 commented May 4, 2022

Rerun tests

@RAMitchell
Copy link
Contributor Author

The centos container is still intermittently failing. I have tried to reproduce it locally in the same container without any success.

Given that we need this PR and I don't think it is related to any changes here, we could try to detect centos and skip xgboost tests.

@cjnolet
Copy link
Member

cjnolet commented May 5, 2022

@RAMitchell not sure if related, but I'm also seeing a similar failure in the other containers on this PR: #4582

@github-actions github-actions bot added the gpuCI gpuCI issue label May 5, 2022
@RAMitchell RAMitchell force-pushed the treeshap-refactor branch from 294fbad to c47b354 Compare May 9, 2022 11:05
@RAMitchell RAMitchell requested a review from a team as a code owner May 9, 2022 11:05
@github-actions github-actions bot removed the gpuCI gpuCI issue label May 9, 2022
@ajschmidt8 ajschmidt8 removed the request for review from a team May 9, 2022 18:37
@ajschmidt8
Copy link
Member

Removing ops-codeowners from the required reviews since it doesn't seem there are any file changes that we're responsible for. Feel free to add us back if necessary.

@RAMitchell
Copy link
Contributor Author

rerun tests

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.06@6145ef3). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.06    #4697   +/-   ##
===============================================
  Coverage                ?   88.54%           
===============================================
  Files                   ?      370           
  Lines                   ?    34759           
  Branches                ?        0           
===============================================
  Hits                    ?    30777           
  Misses                  ?     3982           
  Partials                ?        0
Flag Coverage Δ
dask 34.18% <0.00%> (?)
non-dask 83.85% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6145ef3...c47b354. Read the comment docs.

@dantegd dantegd added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 11, 2022
@dantegd
Copy link
Member

dantegd commented May 11, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit bc70681 into rapidsai:branch-22.06 May 11, 2022
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
@RAMitchell
TreeExplainer extensions (rapidsai#4697)
f7ccdca 
Stacked on rapidsai#4671.

- Remove extra redundant class in python layer.
- Simplify the interface between C++ and python using variants. 
- Fix rapidsai#4670 by allowing double precision data
- Document TreeExplainer
- Add interventional shap method
- Add shapley interactions and taylor interactions
- Promote from experimental
- Support sklearn estimator types from xgb/lgbm (i.e. no need to convert to booster before using TreeExplainer)

Authors:
  - Rory Mitchell (https://github.com/RAMitchell)

Approvers:
  - Philip Hyunsu Cho (https://github.com/hcho3)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#4697
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dantegd dantegd dantegd approved these changes

@hcho3 hcho3 hcho3 approved these changes

Assignees
No one assigned
Labels
CUDA/C++ Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] TreeExplainer makes incorrect predictions for 64 bit data
6 participants
@RAMitchell @hcho3 @cjnolet @ajschmidt8 @codecov-commenter @dantegd