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

fix aggregating sparse multiclass explanations to global in global-only streaming with batches mode #519

Merged

Conversation

imatiach-msft
Copy link
Collaborator

Fix aggregating sparse multiclass explanations to global in global-only streaming with batches mode.

In the case that the explanations are

1.) computed on sparse multiclass data, and
2.) we set parameters to only compute the global explanation from local importance values, and
3.) we are using specifically the streaming aggregation from batches mode

(notice a lot of specific cases there), the logic currently fails as it is not designed to handle a list of sparse matrices. The logic for this scenario is implemented in this PR and a test is added.

@imatiach-msft imatiach-msft force-pushed the ilmat/fix-sparse-multi branch 3 times, most recently from 882caad to fd029e5 Compare April 18, 2022 18:22
@codecov-commenter
Copy link

codecov-commenter commented Apr 18, 2022

Codecov Report

Merging #519 (b980d0b) into master (8e58580) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #519      +/-   ##
==========================================
+ Coverage   86.66%   86.70%   +0.04%     
==========================================
  Files          62       62              
  Lines        4049     4063      +14     
==========================================
+ Hits         3509     3523      +14     
  Misses        540      540              
Flag Coverage Δ
unittests 86.70% <100.00%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
...hon/interpret_community/explanation/explanation.py 85.69% <100.00%> (+0.27%) ⬆️
python/interpret_community/lime/lime_explainer.py 95.80% <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 8e58580...b980d0b. Read the comment docs.

@@ -1661,17 +1661,17 @@ def _get_local_explanation_row(explainer, evaluation_examples, i, batch_size):
return explainer.explain_local(rows)


def _aggregate_streamed_local_explanations(explainer, evaluation_examples, classification, features,
def _aggregate_streamed_local_explanations(explainer, evaluation_examples, task, features,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems a breaking change, do you mind check if any of our notebook or reference code in rai or aml does not have named parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not a breaking change, we are still passing the same parameter value, just the variable name that is passed was changed. Regardless, this method is private because it begins with an underscore anyway, so we can change it however we like.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not an optional keyword argument, right?

@imatiach-msft imatiach-msft merged commit 051f4ff into interpretml:master Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants