albermax / innvestigate

A toolbox to iNNvestigate neural networks' predictions!
Other
1.25k stars 233 forks source link

mnist_perturbation notebook won't run #184

Open tmerrittsmith opened 4 years ago

tmerrittsmith commented 4 years ago

A few issues i've found with running this:

  1. cell 5, methods[0] is called "input", but the outputs of the uploaded notebook show that it should be called 'random'. Is this correct?

  2. cell 9 returns an error on line 37: project() got an unexpected keyword argument 'input_is_postive_only' - this appears to be the same as https://github.com/albermax/innvestigate/issues/176

Happy to submit a pull request to fix these if that's helpful

enryH commented 4 years ago

If you have done it already, just make an PR:)

tmerrittsmith commented 4 years ago

ok will do - have run in to a few more issues so going to get to the bottom of those first

tmerrittsmith commented 4 years ago

Hey @enryH, I don't think a PR is the right way to resolve this.

My original question was whether the 'input' perturbation method is the same as 'random'. I'm pretty sure it's not. In the git log for this notebook, @pseegerer added the 'random' method, but this was then removed in following commits. (added in https://github.com/albermax/innvestigate/commit/e46f268a4ffc113025c635ffe36a9df4991f29ac and removed by @albermax on https://github.com/albermax/innvestigate/commit/a084b17bda698cdd21b4cbc0df88d52b48db5558

Can you take a look to confirm which commit is correct?

Reverting this line seems to get the notebook to run without problem now. However, the actual images of the perturbations don't look like they're working properly - my understanding is that this method should additively mask relevant regions of the image, but in my version of the notebook this masking seems to be happening at random (see attached snip) - not sure if this is related to the above overwrites or not?

Capture )

tmerrittsmith commented 4 years ago

I've run the notebook reverted to that previous commit (https://github.com/albermax/innvestigate/commit/e46f268a4ffc113025c635ffe36a9df4991f29ac) and the perturbation looks right, so it must have been introduced since then.