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

added Grad-CAM implementation #3

Merged
merged 6 commits into from Jul 14, 2017
Merged

Conversation

ruthcfong
Copy link
Contributor

Here's an implementation of Grad-CAM that extends SaliencyMask class.

I've included a few optional arguments in the GetMask function (i.e., should_resize, three_dims) to allow the option to return the original, low-res 2D Grad-CAM heatmap, which I need.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@ruthcfong
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

Copy link
Collaborator

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Thanks. This is a great contribution! One comment regarding dependency on skimage.

grad_cam.py Outdated
import tensorflow as tf

from saliency import SaliencyMask
from skimage.transform import resize
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the core library is trying to avoid dependencies, how about using tf.image.resize_bilinear(...) instead of skimage

Copy link
Collaborator

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

One more comment: Update README.md bullet list by adding Grad-CAM to the list

@ruthcfong
Copy link
Contributor Author

I've replaced skimage with tf's resize function as suggested and updated the README file.

Copy link
Collaborator

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks for this great contribution!

@dsmilkov dsmilkov merged commit 4a6a8e6 into PAIR-code:master Jul 14, 2017
@nsthorat
Copy link
Collaborator

+1 thanks for the contribution! :)

@nalinimsingh
Copy link
Contributor

nalinimsingh commented Aug 16, 2018

@ruthcfong -- I'm curious about this line in the GradCAM implementation here.

Why is this matrix initialized with ones -- from my read of the paper, it seems it should be filled with zeroes?

[This doesn't affect the qualitative trend you see across the image in the GradCAM masks I've generated, since this is a constant shift at each pixel, but it does slightly perturb some of the numeric values.]

@ruthcfong
Copy link
Contributor Author

ruthcfong commented Aug 18, 2018

@nalinimsingh unfortunately, I don't remember what my reasoning was when I added the GradCAM implementation. I think you're right though that the proper way to do it is to initialize with zeros.

My hypothesis that I probably was looking at this keras implementation by jacobgil (committed March 2017; my commit was July 2017), which does the same thing.

I created a new pull request with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants