cdpierse / transformers-interpret

Model explainability that works seamlessly with 🤗 transformers. Explain your transformers model in just 2 lines of code.

Apache License 2.0

1.29k stars 97 forks source link

Feature/ner models #88

Closed pabvald closed 2 years ago

pabvald commented 2 years ago

PR Description

Support for NER models (i.e. token classification) included in the explainers/token_classification_explainer.py file, with the possibility of indicating which indexes and which labels should be ignored, as was suggested by @cdpierse. The notebook notebooks/ner_example.ipynb covers its usage in detail. The corresponding section has been added to the documentation

Motivation and Context

References issue: #86 References discussion: #83

Tests and Coverage

Tests included in test/test_token_classification.py. Coverage of 90%.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[x] Docs (Added to or improved Transformers Interpret's documentation)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Final Checklist:

[x] My code follows the code style of this project.
[x] I have updated the documentation accordingly.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

pabvald commented 2 years ago

Hi @cdpierse.

Here's my implementation of the NER explainer. I have included the possibility of specifying a list of indexes and/or labels to ignore, like you wanted.

I look forward to your review.

Best regards

cdpierse commented 2 years ago

Hi @pabvald,

Thanks so much for putting this PR together. I've wanted to add this support for quite a while so you doing the whole thing is massively appreciated. I've only had a quick glance at the PR but so far it looks great. I'll have some more specific feedback later in the week but thanks again, really looking forward to merging this in.

All the best, Charles

cdpierse commented 2 years ago

@pabvald this is looking very good so far, I'm really impressed so great job. Only a few small suggestions for this PR the main one being around keeping the result of word_attributions on par with the data displayed in visualize but it's very close to being ready to merge. Thank you so much for all of this work, you've done a huge piece of work!

pabvald commented 2 years ago

@cdpierse Thanks for review!

I have implemented the changes you have suggested. Looking forward to another review.

cdpierse commented 2 years ago

Hi @pabvald,

Haven't forgotten about this PR, hoping to get time this weekend to do a final review ☺️

cdpierse commented 2 years ago

@pabvald this is looking great, it's pretty much ready to merge, the only other change I would suggest is using an even smaller model for testing, I would go with elastic/distilbert-base-cased-finetuned-conll03-english as it's about half the size of the one you are currently using and will help prevent against flakey tests. I will merge this in and prepare a release after that 😄

pabvald commented 2 years ago

@cdpierse done! Now elastic/distilbert-base-cased-finetuned-conll03-english is used for testing. 🧪

cdpierse commented 2 years ago

Amazing work @pabvald, thanks so much for this PR, very happy to have such a great contribution. I've just merged in and will get the next release ready!