Skip to content

Conversation

ProGamerGov
Copy link
Contributor

@ProGamerGov ProGamerGov commented Jan 11, 2021

This PR doesn't overlap with any other PRs. We don't need a RGB to BGR transform inside the model. Currently InceptionV1 is optimizing images in RGB instead of BGR, which results in really bluish visualization images. Visualization results should now match the colors found on OpenAI Microscope: https://microscope.openai.com/models/inceptionv1/mixed4c_0/?models.op.feature_vis.type=channel&models.op.technique=feature_vis

Ludwig's model had the RGB to BGR transform commented out here as well: https://github.com/ludwigschubert/captum/blob/f1fd0729dece59564a7c10b7b397617d8a09a247/captum/optim/models/inception_v1.py#L200-L209

  • I made the InceptionV1 model's internal BGR transform optional. By default it's set to False.

@ProGamerGov
Copy link
Contributor Author

ProGamerGov commented Jan 11, 2021

@NarineK The test failure appears to be a CircleCI issue and not related to any the changes in this PR (the Captum.attr module tests timed out). You can also merge this PR as soon as possible ahead of #552, #574, & #579, as it's an extremely important fix.

@NarineK
Copy link
Contributor

NarineK commented Jan 11, 2021

Thank you @ProGamerGov for finding the issue. It looks like Ludwig didn't swap the channels but changed the values? I'm not familiar with this transformation. Is that what he actually wanted to reach here (RGB to BGR)?
https://github.com/ludwigschubert/captum/blob/f1fd0729dece59564a7c10b7b397617d8a09a247/captum/optim/models/inception_v1.py#L200-L209

@NarineK NarineK self-requested a review January 11, 2021 18:04
@ProGamerGov
Copy link
Contributor Author

ProGamerGov commented Jan 11, 2021

@NarineK It looks like Ludwig copied the transform_input function from Torchvision here: https://github.com/pytorch/vision/blob/master/torchvision/models/googlenet.py#L136-L142

The InceptionV1 model does use BGR according to Lucid here: https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/caffe_models/InceptionV1.py#L34, so I'm not entirely sure where the RGB to BGR conversion occurs then since the one I added to transform_input appears to have been a duplicate transform.

@NarineK
Copy link
Contributor

NarineK commented Jan 11, 2021

@NarineK It looks like Ludwig copied the transform_input function from Torchvision here: https://github.com/pytorch/vision/blob/master/torchvision/models/googlenet.py#L136-L142

The InceptionV1 model does use BGR according to Lucid here: https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/caffe_models/InceptionV1.py#L34, so I'm not entirely sure where the RGB to BGR conversion occurs then since the one I added to transform_input appears to have been a duplicate transform.

Thank you @ProGamerGov for the prompt reply!
Is is_BGR used anywhere ? I see a definition of it.

@ProGamerGov
Copy link
Contributor Author

@NarineK For Lucid, it looks like it's used in the base class which the InceptionV1 model inherits: https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/vision_base.py#L184

@NarineK
Copy link
Contributor

NarineK commented Jan 11, 2021

@NarineK It looks like Ludwig copied the transform_input function from Torchvision here: https://github.com/pytorch/vision/blob/master/torchvision/models/googlenet.py#L136-L142
The InceptionV1 model does use BGR according to Lucid here: https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/caffe_models/InceptionV1.py#L34, so I'm not entirely sure where the RGB to BGR conversion occurs then since the one I added to transform_input appears to have been a duplicate transform.

Thank you @ProGamerGov for the prompt reply!
Is is_BGR used anywhere ? I see a definition of it.

@NarineK It looks like Ludwig copied the transform_input function from Torchvision here: https://github.com/pytorch/vision/blob/master/torchvision/models/googlenet.py#L136-L142
The InceptionV1 model does use BGR according to Lucid here: https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/caffe_models/InceptionV1.py#L34, so I'm not entirely sure where the RGB to BGR conversion occurs then since the one I added to transform_input appears to have been a duplicate transform.

Thank you @ProGamerGov for the prompt reply!
Is is_BGR used anywhere ? I see a definition of it.

@NarineK For Lucid, it looks like it's used in the base class which the InceptionV1 model inherits: https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/vision_base.py#L184

I see - Interesting if that option was used in any of their experiments. Perhaps for some of their case studies or experiments?

@ProGamerGov
Copy link
Contributor Author

ProGamerGov commented Jan 11, 2021

@NarineK Lucid seems to use it as a class variable for initializing the input preprocessing, so I think that it wasn't really an optional setting if you wanted to use the correct preprocessing.

https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/vision_base.py#L196

Github's references option shows import_graph() being used all over Lucid.

@NarineK NarineK merged commit 2a772ed into meta-pytorch:optim-wip Jan 11, 2021
@NarineK
Copy link
Contributor

NarineK commented Jan 11, 2021

@NarineK Lucid seems to use it as a class variable for initializing the input preprocessing, so I think that it wasn't really an optional setting if you wanted to use the correct preprocessing.

https://github.com/tensorflow/lucid/blob/master/lucid/modelzoo/vision_base.py#L196

Github's references option shows import_graph() being used all over Lucid.

Thank you for checking it! Merged!

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

Successfully merging this pull request may close these issues.

3 participants