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

Misc edits to explain_prediction documentation #992

Merged
merged 3 commits into from Jul 29, 2020

Conversation

freddyaboulton
Copy link
Contributor

@freddyaboulton freddyaboulton commented Jul 28, 2020

Pull Request Description

  1. Adds a sentence to the api reference/User Guide about how XGBoost and CatBoost are not fully supported.
  2. Moves a paragraph of the explaining predictions section of the user guide below the code block.

Change to API Reference

image

Change to Model Understanding Tutorial

image


After creating the pull request: in order to pass the release_notes_updated check you will need to update the "Future Release" section of docs/source/release_notes.rst to include this pull request by adding :pr:123.

@codecov
Copy link

codecov bot commented Jul 28, 2020

Codecov Report

Merging #992 into main will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #992   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files         179      179           
  Lines        9283     9283           
=======================================
  Hits         9270     9270           
  Misses         13       13           
Impacted Files Coverage Δ
...alml/pipelines/prediction_explanations/__init__.py 100.00% <ø> (ø)
...ml/pipelines/prediction_explanations/explainers.py 100.00% <100.00%> (ø)
...peline_tests/explanations_tests/test_explainers.py 100.00% <100.00%> (ø)

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 61ef918...8fa0e4c. Read the comment docs.

@freddyaboulton freddyaboulton marked this pull request as ready for review Jul 29, 2020
@freddyaboulton freddyaboulton requested review from angela97lin and dsherry and removed request for angela97lin Jul 29, 2020
Copy link
Contributor

@angela97lin angela97lin left a comment

LGTM! 👍 Made two non-blocking suggestions :D

@@ -231,6 +231,8 @@ Regressors are components that output a predicted target value.
Prediction Explanations
========================

XGBoost models and CatBoost multiclass classifiers are not currently supported.
Copy link
Contributor

@angela97lin angela97lin Jul 29, 2020

Choose a reason for hiding this comment

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

Nice! I wonder if it'd be useful (or maybe just redundant haha) to include this on the explain_prediction page too? 🤷

"source": [
"The interpretation of the table is the same for regression problems - but the SHAP value now corresponds to the change in the estimated value of the dependent variable rather than a change in probability. For multiclass classification problems, a table will be output for each possible class.\n",
"\n",
"This functionality is currently not **supported** for **XGBoost** models or **CatBoost multiclass** classifiers."
Copy link
Contributor

@angela97lin angela97lin Jul 29, 2020

Choose a reason for hiding this comment

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

Maybe it'd make sense to bold not?

Copy link
Contributor Author

@freddyaboulton freddyaboulton Jul 29, 2020

Choose a reason for hiding this comment

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

Great point!

@@ -231,6 +231,8 @@ Regressors are components that output a predicted target value.
Prediction Explanations
========================

XGBoost models and CatBoost multiclass classifiers are not currently supported.
Copy link
Collaborator

@dsherry dsherry Jul 29, 2020

Choose a reason for hiding this comment

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

@freddyaboulton should this go in the code instead?

Copy link
Collaborator

@dsherry dsherry Jul 29, 2020

Choose a reason for hiding this comment

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

Yeah, I suggest you move this into the docstring in evalml/pipelines/prediction_explanations/explainers.py::explain_prediction

Copy link
Contributor Author

@freddyaboulton freddyaboulton Jul 29, 2020

Choose a reason for hiding this comment

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

Will do!

"source": [
"The interpretation of the table is the same for regression problems - but the SHAP value now corresponds to the change in the estimated value of the dependent variable rather than a change in probability. For multiclass classification problems, a table will be output for each possible class.\n",
"\n",
"This functionality is currently not **supported** for **XGBoost** models or **CatBoost multiclass** classifiers."
Copy link
Collaborator

@dsherry dsherry Jul 29, 2020

Choose a reason for hiding this comment

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

Thanks!

Copy link
Collaborator

@dsherry dsherry left a comment

LGTM! thanks for updating the naming of explain_predictions and the file

@@ -9,9 +9,11 @@
)


def _explain_prediction(pipeline, input_features, top_k=3, training_data=None, include_shap_values=False):
def explain_prediction(pipeline, input_features, top_k=3, training_data=None, include_shap_values=False):
Copy link
Collaborator

@dsherry dsherry Jul 29, 2020

Choose a reason for hiding this comment

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

Thank you!

"""Creates table summarizing the top_k positive and top_k negative contributing features to the prediction of a single datapoint.

XGBoost models and CatBoost multiclass classifiers are not currently supported.
Copy link
Collaborator

@dsherry dsherry Jul 29, 2020

Choose a reason for hiding this comment

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

👍

from evalml.pipelines.prediction_explanations._explainers import (
_explain_prediction
from evalml.pipelines.prediction_explanations.explainers import (
explain_prediction
Copy link
Collaborator

@dsherry dsherry Jul 29, 2020

Choose a reason for hiding this comment

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

👍

@freddyaboulton freddyaboulton merged commit ab1e295 into main Jul 29, 2020
2 checks passed
@freddyaboulton freddyaboulton deleted the misc-edits-to-explain_prediction-doc branch Jul 29, 2020
@angela97lin angela97lin mentioned this pull request Jul 31, 2020
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.

None yet

3 participants