First changes to structure and AlexNet integration #3

fuzhanrahmanian · 2021-01-26T17:10:08Z

Describe your improvements
Added AlexNet model with Keras.
Modified model.py
Reshape output file structure.
Added main and argpars for ease of use
Updated .gitignore to ignore .h5 files

Modified model.py Reshape output file structure. Added main and argpars for eas of use Updated .gitignore to ignore .h5 files

Sparkier

Thank you for the PR, its a good starter and especially the amount of changes is nice as it is not too large to review :)

I added some comments which we should discuss before merging. You can obviously already start more work on another branch which can be a follow-up PR.

Additionally, there are a bunch of binary files which I think should not be committed, those are:
feature_vis_inceptionV3_mixed6_1.npy
outputs/feature_vis_inceptionV3_mixed6_1.png
pretrained_models/data/meta_clsloc.mat
test.npy
test.png

I think in general, you want to have the main.py outside of this project, and all the output as well.

Sparkier · 2021-01-27T09:02:04Z

featurevis/featurevis.py

@@ -131,7 +132,7 @@ def visualize_filter(image, model, layer, filter_index, iterations,
    print('>> 100 %')
    # Decode the resulting input image
    image = imgs.deprocess_image(image[0].numpy())
-    return loss, image
+    return loss, image, layer, filter_index


Why would we return layer and filter_index? We already provide this to the function as a parameter, so the other end should know about this, no?

You are right. I will implement the variables in the main function and use them for the file name generation there.

featurevis/image_reader.py

Sparkier · 2021-01-27T09:08:25Z

main.py

+from pretrained_models import models
+from featurevis import featurevis, images, image_reader
+import argparse
+
+parser = argparse.ArgumentParser()
+
+parser.add_argument("-a", "--architecture", type = str, default = "inceptionV3", help = "The model architecture")
+args = parser.parse_args()
+
+arch = args.architecture
+#architecture = "inceptionV3"
+
+model = models.get_model(arch)
+# model.summary()
+image = images.initialize_image(224, 224)
+loss, image, layer_name, channel_num = featurevis.visualize_filter(image, model, "mixed6", 1, 500, 2, 0, 0, 0)
+
+name = "feature_vis_{}_{}_{}".format(arch, layer_name, channel_num)
+print(loss)
+
+images.save_image(image, name= name)
+image_reader.save_npy_as_png("{}.npy".format(name))
+


I think we should not have a main.py like this. If people are supposed to use this as a library (as is probably intended), i.e., to write their own code and import this as a package (import luna), then such a file is not needed. What we could think of is two options:

Currently, we have a how-to demo in the README.md. It could stay this way and we could change that readme if we think this code is a better showcase.

We could do a luna_demo.py, which does what you have here. The downside of this is that when the project is used as a library, this does not really make sense, as there is no method to call.

These things considered, I would move this code to the README.md, if we think this is a better showcase. What do you think?

I would agree more to put it inside README.md. Maybe we can talk more about it in our meeting.

Sure, lets talk tomorrow.

pretrained_models/alexnet.py

Sparkier · 2021-01-27T09:12:38Z

pretrained_models/models.py

+    if model_name == "VGG16":
+        return keras.applications.vgg16(weights= "imagenet", include_top=False)
+    if model_name == "alexnet":
+        return alexnet.AlexNet()


This does not use a weights_path, is that intended? For other models, we load the pretrained weights by default.

I wrote the initialization of the weight_path inside the main of alexnet.py. But if we want to get rid of it, I will give the pretrained weight as default in the model.py file.

Sparkier · 2021-01-27T09:15:18Z

pretrained_models/models.py

+    else:
+        print("Model is not specified")


The problem about this is, that we do have one path now that does not return anything. I put a default return here that returns VGG16 if nothing else matches. Maybe we should go back to that and print a warning? Otherwise the code will fail if the wrong model is specified. In the long run, we might want to have individual functions for the models to get rid of this if/else, which is not very pretty.

We can talk about it. I just don't like the VGG as fallback but we can decide about this.

Sure, lets talk about this tomorrow.

Sparkier · 2021-01-27T09:17:22Z

featurevis/featurevis.py

 import tensorflow as tf
 from tensorflow import keras

-from luna.featurevis import images as imgs
+from featurevis import images as imgs


Out of curiosity, why did you change that? I think the previous version was correct, no? At least if you use this as a library.

I did this because I wrote my main.py file inside the library and this luna was redundant at that point.

In that case, we should probably change it back.

Sparkier · 2021-01-28T07:40:34Z

featurevis/featurevis.py

 import tensorflow as tf
 from tensorflow import keras

-from luna.featurevis import images as imgs
+from featurevis import images as imgs


In that case, we should probably change it back.

Sparkier · 2021-01-28T07:44:39Z

featurevis/image_reader.py

 from pathlib import Path
 import numpy as np
 from tensorflow import keras


-def save_npy_as_png(path):
+def save_npy_as_png(input_path, output_path = "lune/outputs"):


In general, I think this change is good. But I don't think we should have an outputs folder inside the project. If this is to be used as a library, this does not really make sense. So I would say this should probably not have a default value.

Sparkier · 2021-01-28T07:46:15Z

pretrained_models/models.py

+    else:
+        print("Model is not specified")


Sure, lets talk about this tomorrow.

Sparkier · 2021-02-07T15:15:17Z

featurevis/featurevis.py

@@ -132,6 +134,9 @@ def visualize_filter(image, model, layer, filter_index, iterations,
    print('>> 100 %')
    # Decode the resulting input image
    image = imgs.deprocess_image(image[0].numpy())
+    print(image.shape)
+    print(image)
+    # conver the proceed image to a valid rgb color


conver -> convert

Sparkier · 2021-02-07T15:15:51Z

featurevis/featurevis.py

+    print(image.shape)
+    print(image)


What is that for?

Sparkier · 2021-02-07T15:17:22Z

featurevis/featurevis.py

@@ -175,10 +180,11 @@ def gradient_ascent_step(img, model, filter_index, learning_rate, channels_first
    with tf.GradientTape() as tape:
        tape.watch(img)
        loss = compute_loss(img, model, filter_index, channels_first)
+        #loss_prop = tf.reduce_sum(img, axis=-1)


Uncommented lines should be removed.

Sparkier · 2021-02-07T15:17:36Z

featurevis/featurevis.py

    # Compute gradients.
    grads = tape.gradient(loss, img)
    # Normalize gradients.
-    grads = tf.math.l2_normalize(grads)
+    #grads = tf.math.l2_normalize(grads)


Uncommented lines should be removed.

Sparkier · 2021-02-08T09:10:12Z

featurevis/image_reader.py

@@ -8,7 +8,7 @@
 from tensorflow import keras


-def save_npy_as_png(input_path, output_path = "lune/outputs"):
+def save_npy_as_png(input_path, output_path = "luna/outputs"):


Still think per default, this should not be in luna.

Added AlexNet model with Keras.

d4c02c0

Modified model.py Reshape output file structure. Added main and argpars for eas of use Updated .gitignore to ignore .h5 files

Sparkier requested changes Jan 27, 2021

View reviewed changes

FuzhanRahmanian added 3 commits January 27, 2021 21:48

Structure adjusted based on pull request remarks.

4ccf5dc

updated gitignore

b7c3589

Adjusted unnecessary files

e3c49f8

Sparkier reviewed Jan 28, 2021

View reviewed changes

fuzhanrahmanian and others added 5 commits February 7, 2021 15:08

Removed PNG

7a1dcf7

changed .gitignore to ignore outputs and png

d0cca86

Fixed import file and changed main location

bffea70

Merge branch 'master' of https://github.com/fuzhanrahmanian/luna

d654834

adjusted main.py location

91bb56b

Sparkier reviewed Feb 7, 2021

View reviewed changes

the architectures are written in different functions

4ee0cbb

Sparkier reviewed Feb 8, 2021

View reviewed changes

fuzhanrahmanian added 3 commits February 14, 2021 15:30

Modified files to comply with pylint

6d23693

Modified gitignore and output folder handling

557ca26

remove uneccessary image processing

648e183

fuzhanrahmanian requested a review from Sparkier February 14, 2021 17:12

Sparkier approved these changes Feb 15, 2021

View reviewed changes

Sparkier merged commit 840064f into Sparkier:master Feb 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First changes to structure and AlexNet integration #3

First changes to structure and AlexNet integration #3

fuzhanrahmanian commented Jan 26, 2021 •

edited by Sparkier

Loading

Sparkier left a comment

Sparkier Jan 27, 2021

fuzhanrahmanian Jan 27, 2021

Sparkier Jan 27, 2021

fuzhanrahmanian Jan 27, 2021

Sparkier Jan 28, 2021

Sparkier Jan 27, 2021

fuzhanrahmanian Jan 27, 2021

Sparkier Jan 27, 2021

fuzhanrahmanian Jan 27, 2021

Sparkier Jan 28, 2021

Sparkier Jan 27, 2021

fuzhanrahmanian Jan 27, 2021

Sparkier Jan 28, 2021

Sparkier Jan 28, 2021

Sparkier Jan 28, 2021

Sparkier Jan 28, 2021

Sparkier Feb 7, 2021

Sparkier Feb 7, 2021

Sparkier Feb 7, 2021

Sparkier Feb 7, 2021

Sparkier Feb 8, 2021

First changes to structure and AlexNet integration #3

First changes to structure and AlexNet integration #3

Conversation

fuzhanrahmanian commented Jan 26, 2021 • edited by Sparkier Loading

Sparkier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fuzhanrahmanian commented Jan 26, 2021 •

edited by Sparkier

Loading