-
Notifications
You must be signed in to change notification settings - Fork 188
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
Add support for white-box explainers to alibi-explain runtime #1279
Add support for white-box explainers to alibi-explain runtime #1279
Conversation
runtimes/alibi-explain/mlserver_alibi_explain/explainers/sklearn_api_runtime.py
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/explainers/white_box_runtime.py
Outdated
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/explainers/white_box_runtime.py
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/alibi_dependency_reference.py
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/explainers/black_box_runtime.py
Show resolved
Hide resolved
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.
This is a great PR @ascillitoe !!
I love the test thoroughness - the more tests the better IMO (as long as they are not massively slow!). Added a few comments here-and-there, but they are mainly questions and suggestions - nothing biggie.
runtimes/alibi-explain/mlserver_alibi_explain/alibi_dependency_reference.py
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/explainers/black_box_runtime.py
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/explainers/sklearn_api_runtime.py
Outdated
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/explainers/sklearn_api_runtime.py
Show resolved
Hide resolved
runtimes/alibi-explain/mlserver_alibi_explain/explainers/sklearn_api_runtime.py
Outdated
Show resolved
Hide resolved
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.
Nice one @ascillitoe! Thanks for making those changes.
Once the linter passes, this looks good to go from my side! Conscious it's still marked as a draft though, so happy to re-review once you think it's ready.
Thanks @adriangonz, I'll make the linter changes now and turn off draft once done! |
Summary
This PR adds support for the sklearn API white-box explainers (
TreeShap
,TreePartialDependence
,PartialDependenceVariance
) to theAlibiExplainRuntime
.Implementation details
New runtime: Support is implemented via the new
AlibiExplainSKLearnAPIRuntime
subclass. For the remaining white-box explainers two further subclassed runtimes will be required (one being theIntegratedGradientsWrapper
). See the notion doc for more details.Tests: Some explainer's have various "nitty-gritty" details wrt to serving, for example the
target
kwarg forIntegratedGradients
, callingfit
with no data forTreeShap
, and black-box/white-box mode ofPartialDependenceVariance
being determined bytype(predictor)
. Therefore, the PR is proposing to perform an E2E test (test_end_2_end
) on every explainer (with relevant parametrisation such as testing uri vs init_param's based loading). This would provide more confidence that all explainers can actually be served. However, it will increase the number of tests being run somewhat, so needs discussion...TODO
Docs (leave until all white box explainers supported...)