-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
BUG: Inconsistency and error in returned explainer objects with TreeExplainer #3187
Comments
Thank you for the bug report, it would be great to investigate this and get it fixed. As a first step, it would be helpful if you could update your example above to a complete minimal reproducible example, as described in this guide. |
Thanks for your answer! I added a new MRE. I hope this helps to trace down the problem. |
Haven't yet had time to look too deeply into this,
Thanks for the bug report. |
I tried to look a bit for case 6.
I tried to add the following after line 238 of _tree.py:
This case then returns Concerning case 3 and 4, I tried with an XGBoost model. Case 3 is shaped |
I tried my example again and I still get the same error as I reported above. Output of my IPython console:
conda-forge shap 0.42.1 |
Update with the current master branch (04.09.2023 16:00 CET, pip install git+https://github.com/shap/shap): Same example as posted in the first post:
No change on example 1 to 5.
This error is weird since option "tree_path_dependent" should not require any background samples because it uses the tree structure, right? conda-forge shap 0.42.1(Current master in github with pip install git+https://github.com/shap/shap) |
If I understand well the error and the paper introducing TreeSHAP, There is a need to cover all leaves to compute these values. for i in range(len(explainer_mult.model.trees)):
if np.min(explainer_mult.model.trees[i].node_sample_weight) <= 0:
print(f"tree {i}: explainer_mult.model.trees[i]") A way to avoid that is to add restrictions to your model (e.g. limit max_depth). Maybe the way nodes are weighted in the case of LGBM has to be updated ? |
Thanks @znacer for your awesome work! I also investigated the background dataset issue of task 6 a little. I set the min_child_weight parameter of the lightgbm to a higher value. This should solve the issue if @znacer your explanation is right, since preventing the weights to approach zero, right? However, for me nothing changed. So I did a dirty hack and set
Which let me pass the assert in line 297 in _tree.py:
Still, as far as I understand, this assert is a bug and the assert should not be carried out at all if no data are passed as background dataset, but only if one passes data as background dataset. Hence, the if should be different and the assert should only be carried out if background data is not None and feature_perturbation is tree_path_dependent. Passing the assert leads to the following error, which is the same as mention by @znacer:
Adding the code proposed by @znacer below line 238:
leads to the expected output of Letting only the inconsistency of task 4 open, which should be |
In my opinion there are two problems with this approach:
import shap
import xgboost
import numpy as np
X, y = shap.datasets.adult(n_points=50)
xgb = xgboost.XGBClassifier(max_depth=1).fit(X, y)
ex_xgb = shap.TreeExplainer(xgb)
e_xgb_bin = ex_xgb(X, interactions=False)
class1_pred = xgb.predict(X, output_margin=True)
# stacking outputs might be a bit tedious for the user
xgb_pred = np.vstack([-class1_pred, class1_pred]).T
assert (np.abs(e_xgb_bin.values.sum(1) + e_xgb_bin.base_values - xgb_pred).max(axis=(0, 1))
< 1e-4
)
# or just knowing that we need the last index for the class might also lead to confusion
assert (np.abs(e_xgb_bin.values.sum(1)[:, -1] + e_xgb_bin.base_values[:, -1] - class1_pred).max()
< 1e-4
) Alternatively I would propose that we align the output of shap and interaction values on the model output. The advantage is, that we have this readily available in internal attributes and that it is straight forward for the user to understand what the shap values are referring to. So that the following checks are always fulfilled: # for outputs of shape (#observations,)
pred = model.predict(X) # I know that we might need some additional parameters for the predict, but I omit this for simplicity
assert np.allclose(e_xgb_bin.values.sum(1) + e_xgb_bin.base_values, pred)
# for outputs of of shape (#observations, #num_classes_or_targets)
assert np.allclose(e_xgb_bin.values.sum(1, 2) + e_xgb_bin.base_values, pred) |
I am happy to have the SHAP output aligned with model output. My report was on LightGBM only, there I found the inconsistency between including interactions or not when using the TreeExplainer (and one case where it crashes). In my opinion case 4 should be samples x features x features x classes to be consistent with case 3, but the classes is missing and for case 6 where it is crashing it should be samples x features x features x classes as well. |
Just to make this clear, my proposal would change this to: |
Since in the binary case results of class 0 are the complement of class 1 I think it is OK. However, I would prefer to have it |
Case 6 is probably connected to #3457 |
@NegatedObjectIdentity I just tested it and case 6 should work on latest master (no idea what changed though). You can even have a look at this PR where I explicitly test case 6: https://github.com/shap/shap/pull/3459/files#diff-575fa6d845d524882a8ba5609d8a2c7f877844870c10aa551a67d8fa24461b05R1680 |
Issue Description
I use shap to explain LightGBM models for different tasks. Depending on the explaination task (classification bin/multi or regression; with or without feature interactions), the TreeExplainer class returns ShapExplaination objects in various inconsistent shapes or in the case of task multiclass+interactions it crashes.
Here my testings:
Kind of task | Explainer object shape | My assessment
Minimal Reproducible Example
Traceback
Expected Behavior
Bug report checklist
Installed Versions
conda-forge shap 0.42.1
conda-forge lightgbm 4.0.0
conda-forge python 3.11.4
Windows 11
The text was updated successfully, but these errors were encountered: