Closed AlexKubiesa closed 3 years ago
Thanks for the PR @AlexKubiesa! I think the way you've done it is nice, I just put some minor comments on the PR. Thanks again!
@AlexKubiesa could you rebase your PR on master? I've fixed the failing tests, and migrated from Travis to Github Actions. Thanks!
Thanks for having a look @RaphaelMeudec, I'll push again once I've addressed all the comments.
By the way, it looks like the tox.ini file is gone from master, which means tox doesn't run for me locally after I rebased. Is that intended?
I've rebased now and I think I've addressed all the comments.
All good! Thanks for the PR @AlexKubiesa!
Addresses #159
I've modified VanillaGradients.explain so instead of using the full model, it first creates a "score model" - the same as the original model, except without the final activation layer - and uses that instead.
If callers supply a model where the last layer isn't an activation layer, a warning is generated, and we use the original model as the score model. This tends to produce uninteresting output, so it should be pretty clear to users that something has gone wrong in this case.
Calling VanillaGradients.explain directly from VanillaGradientsCallback caused tf.function rerouting because the score model was being regenerated on every epoch. This made Tensorflow generate lots of warning messages and presumably reduces performance (I think I did see a performance hit when I tried it locally). So, instead, I split VanillaGradientsCallback.explain into "get_score_model" and "explain_score_model". The idea is that get_score_model gets called only once by the callback, and explain_score_model gets called every epoch. This is possible because the base Callback class has a set_model method which gets called by the framework, and I added an override for it.