-
Notifications
You must be signed in to change notification settings - Fork 233
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
Updates towards tf2.0 #196
base: develop_v2.0
Are you sure you want to change the base?
Conversation
…cated now) to LRPSequentialComposite*.
We can see that old_innvestigate and new_innvestigate produce different results. small_tests.py is therefore NOT sufficient for testing!!!
.gitignore
Outdated
@@ -116,8 +116,9 @@ nosetests.cfg | |||
*.jpg | |||
*.jpeg | |||
*.JPEG | |||
*.h5 | |||
#*.h5 |
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.
Debug comment?
@@ -1,23 +1,23 @@ | |||
# Get Python six functionality: |
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.
Of topic/pull request: I think we can drop python six in general as python 2 is not supported anymore.
innvestigate/analyzer/reverse_map.py
Outdated
replacement_layers.append(wrapper_class(layer, layer_next)) | ||
|
||
#connect graph structure | ||
for layer in replacement_layers: |
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.
In Keras one layer can be applied to several inputs. Does the code in this case still work? It would be great to have comment on how that case is handled.
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.
Do you mean the case for e.g. concatenate layers? The code considers this case by aggregating inputs for each layer before applying the forward pass. see try_apply function in innvestigate/analyzer/reverse_map.py:146.
innvestigate/analyzer/reverse_map.py
Outdated
@@ -0,0 +1,335 @@ | |||
|
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.
Do you have some (design) documentation on how this all works together?
I have a rough idea on what the code is doing but IMHO it would be nice to discuss/converge on the design on a high level (also to avoid conceptual problems).
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.
see #214 for a general overview
…sepcify the analysis method, "all" is performed which produces confusing heatmaps. For beginners this error might be difficult to spot.
…nnvestigate into updates_towards_tf2.0
…et; thus did not make the available in __init__ yet
…_hook, added specification option of last layer(s) (including a warning if neuron selection uses indices, so the user can make sure to specify correct neuronselection shapes)
… next commit, I will make it actively available if acknowledged
Leander, please, check the code
…tes_towards_tf2.0 # Conflicts: # innvestigate/analyzer/base.py
Leander, please, check the code
…nnvestigate into updates_towards_tf2.0
…op_mapping_at_layers was not empty
…ive testing of results is still needed.
…parameter to be a dictionary. [Untested]
…il a certain index [Untested]
Calculate only backward pass for efficiency
…tored reverse_map.py and derived classes a bit for that purpose. Gradient Based Methods are still buggy in this refactored version, so they were disabled for the time being. There may be a Memory Leak somewhere when making repeated analyses, need to investigate this further.
Base and LRP updated. Other XAI-Methods are disabled for the time being (otherwise they will conflict with updated base)