Remove dependency of architectures #8

fuzhanrahmanian · 2021-02-21T16:36:29Z

Remove dependency of architectures to the existence both .h5 files. If files are not in the standard
download directory, download the architectures. Partially adapted alexnet code to comply with keras 2.4

to the existence both .h5 files. If files are not in the standard download directory, donwload the arichtectures

Deprecated the use of a channles_first variable in order to rely on the golbal keras backend image format varialbe.

Sparkier

Thank you for these changes. With minor adjustments, they should be mergeable.

luna/featurevis/images.py

Sparkier · 2021-02-22T08:30:51Z

luna/pretrained_models/alexnet.py

                                        ZeroPadding2D)
+from keras.utils.data_utils import get_file
+from tensorflow.keras.layers import Conv2D


Could merge with the other layers import.

Still valid.

tensorflow.keras.layers can be merged. I cannot find a merge for get_file
p.s. Alexnet does not work (properly) yet. I am still working on it

Sparkier · 2021-02-22T08:32:23Z

luna/pretrained_models/alexnet.py

@@ -141,7 +146,15 @@ def alex_net(weights_path=None, heatmap=False):

    model = Model(input=inputs, output=prediction)

-    if weights_path:
-        model.load_weights(weights_path)
+    weights_path = Path(str(Path.home()) + r"~\.keras\datasets\alexnet_weights.h5")


I don't know if we can make these assumptions. Should we rather make the weights path a mandatory variable? Then one would have to specify it, and this would not be needed.

Still valid.

I don't know if that would work on every OS etc.

luna/pretrained_models/alexnet.py

Sparkier · 2021-02-22T08:34:15Z

luna/pretrained_models/googlenet.py

@@ -430,8 +431,17 @@ def create_googlenet():
                                             loss2_classifier_act,
                                             loss3_classifier_act])

-    googlenet.load_weights(os.path.join(
-        os.path.dirname(__file__), 'googlenet_weights.h5'))
+    weights_path = Path(str(Path.home()) + r"\.keras\datasets\googlenet_weights.h5")


Same as above, thee assumptions could be problematic, so I would advocate for removing them and have the user specify weights path.

Weights path is still an optional variable but if it is not given, it will be downloaded. It is directory independent so should not give problem in different OS.

Sparkier

Thank you for the PR. Most of my comments should be easy fixes.

Sparkier · 2021-04-13T07:24:31Z

luna/featurevis/featurevis.py

-                                  padding='SAME', data_format='NHWC')
-
-
+#pylint: disable=R0914


This should be removed. You can use up to 10 arguments per function. This function only takes 9, so removing should have no effect.

Sparkier · 2021-04-13T07:26:18Z

luna/featurevis/featurevis.py

+    # Temporary method for random choice of
+    # transformation combination
+    choice_num = [0, 1, 2, 3]
+    augmentation = ['noise', 'blur', 'scale']


Don't we want this to be specified by the user? Can you give an intuition for why this is random?

Sparkier · 2021-04-13T07:28:38Z

luna/featurevis/featurevis.py

+        image_aug = {'noise': trans.add_noise(image, noise, pctg),
+                     'blur': trans.blur_image(image, blur, pctg),
+                     'scale': trans.rescale_image(image, scale)}
+        num = random.choice(choice_num)
+        if num ==1:
+            ind = random.sample(augmentation, 1)
+            print(ind)
+            image = image_aug[ind[0]]
+        if num ==2:
+            ind = random.sample(augmentation, 2)
+            print(ind)
+            image = image_aug[ind[0]]
+            image = image_aug[ind[1]]
+        else:
+            image = trans.add_noise(image, noise, pctg)
+            image = trans.blur_image(image, blur, pctg)
+            image = trans.rescale_image(image, scale)
+


This has some problems I think. First, you are ignoring the case where random choice is 0. But more importantly, it is not clear to me why this should be random. Maybe you can explain that to me?

I was thinking: if we always add some amount of (i.e.) noise, this will be done at every single iteration and (since it depends on the pctg), it will increase the noisiness at every iteration. My idea was the following: randomize the combination of different transformations in order to not over-transform any feature since the model might not learn good (correct me if my understanding is wrong please). The same goes for every other transformations that use the pctg.

So I have two ideas:

either I can find a more efficient way of writing this randomization

or if the user wants to have a particular transformation, its amount should be random at every iteration and not depended on pctg (similar to rescale image as described in the paper which I am actually using their reported values)

The transformations have a strong effect on the end result.

Sparkier · 2021-04-13T07:32:30Z

luna/featurevis/images.py

+    img = ((img - img.mean()) / img.std()) + 1e-5
+    img *= 0.15
+    img += 0.5
+    img = np.clip(img, 0, 1)

-    # Convert to RGB array
    img *= 255
-    img = img.astype("uint8")
+    img = np.clip(img, 0, 255).astype("uint8")


Can you add comments on what happens here? The one in the function doctoring seems not to cover this anymore.

Sparkier · 2021-04-13T07:37:44Z

luna/featurevis/relu_grad.py

+"""
+Copyright [2017] [https://github.com/tensorflow/lucid]
+"""


Is that really a 1:1 copy? I would assume we'd have to change some things to make it work with tf2. If that is the case, we should probably mention that we adopted but modified the code from there.

More importantly, I don't see any place where we are using these functions. If we are not using them, we should remove this file.

Sparkier · 2021-04-13T07:53:24Z

luna/featurevis/transformations.py

+def random_rotate(img, rotation):
+    """Randomly rotates the image
+
+    Args:
+        img (list): image
+        rotation (int): degree of rotation
+
+    Returns:
+        img (list): rotated image
+    """
+    return tfa.image.rotate(img, rotation)


This function is unused. Should be removed I think.

@fuzhanrahmanian this is till valid I think, no? can you comment?

Sparkier · 2021-04-13T07:53:51Z

luna/pretrained_models/alexnet.py

                                        ZeroPadding2D)
+from keras.utils.data_utils import get_file
+from tensorflow.keras.layers import Conv2D


Still valid.

Sparkier · 2021-04-13T07:55:02Z

luna/pretrained_models/alexnet.py

@@ -141,7 +146,15 @@ def alex_net(weights_path=None, heatmap=False):

    model = Model(input=inputs, output=prediction)

-    if weights_path:
-        model.load_weights(weights_path)
+    weights_path = Path(str(Path.home()) + r"~\.keras\datasets\alexnet_weights.h5")


Still valid.

Sparkier · 2021-04-13T07:55:41Z

luna/pretrained_models/alexnet.py

@@ -141,7 +146,15 @@ def alex_net(weights_path=None, heatmap=False):

    model = Model(input=inputs, output=prediction)

-    if weights_path:
-        model.load_weights(weights_path)
+    weights_path = Path(str(Path.home()) + r"~\.keras\datasets\alexnet_weights.h5")


I don't know if that would work on every OS etc.

Sparkier · 2021-04-13T07:58:27Z

luna/featurevis/featurevis.py

@@ -135,7 +66,7 @@ def visualize_filter(image, model, layer, filter_index, iterations,
    return loss, image


-def compute_loss(input_image, model, filter_index, channels_first=False):
+def compute_loss(input_image, model, filter_index):


Should be removed from the docstring as well.

Still open.

@fuzhanrahmanian this is not fixed yet. In the function documentation, you are still referring to this variable.

Alexnet was deleted for the moment Pull request correction Adaptation for optimal Googlenet results

Sparkier

Hey, thanks for updating. I have just minor comments, mainly on the pylint stuff and some code comments. Once that is fixed (should be easy), we can merge.

Sparkier · 2021-04-23T14:21:57Z

luna/featurevis/featurevis.py

    feature_extractor = get_feature_extractor(model, layer)
+
+    # Temporary method for random choice of


What does that mean?

Sparkier · 2021-04-23T14:22:58Z

luna/featurevis/featurevis.py

    return tf.reduce_mean(filter_activation)


-@tf.function
-def gradient_ascent_step(img, model, filter_index, learning_rate, channels_first=False):
+@tf.function(experimental_relax_shapes=True)


What is that? Can you add a comment?

Not necessary anymore. I used it to reduce memory usage, memory is not a problem now.

luna/featurevis/featurevis.py

Sparkier · 2021-04-23T14:26:30Z

luna/featurevis/relu_grad.py

+
+    return tf.where(return_relu_grad, relu_grad, result_grad)
+
+#pylint: disable=R0914


Can you use the more explicit version #pylint: disable=too-many-locals

Sparkier · 2021-04-23T14:27:28Z

luna/featurevis/featurevis.py

@@ -135,7 +66,7 @@ def visualize_filter(image, model, layer, filter_index, iterations,
    return loss, image


-def compute_loss(input_image, model, filter_index, channels_first=False):
+def compute_loss(input_image, model, filter_index):


Still open.

Sparkier · 2021-04-23T14:28:26Z

luna/featurevis/featurevis.py


+#pylint: disable=too-many-locals


Are there really too many locals? If yes, can we change that?

I actually need all of them. I can't think of a way of how to get rid of them. Any idea?

Sparkier · 2021-04-23T14:31:38Z

luna/featurevis/featurevis.py


+#pylint: disable=too-many-locals
+#pylint: disable=too-many-arguments


Can we change the method to use objects for the OptimizationParameters and AugmentationParameters?
Something like this: https://stackoverflow.com/a/816517/4620643

Good idea. I did it but pylint complains that there are not enough method in the class. I have omitted this error message for PR.

clean the code for pylint

Sparkier

Lets get this in now. We can fix the minor issues later.

Remove dependency of architectures

fbb1fdf

to the existence both .h5 files. If files are not in the standard download directory, donwload the arichtectures

fuzhanrahmanian linked an issue Feb 21, 2021 that may be closed by this pull request

Referring to static weight files #6

Closed

Correction to integrate GoogleNet

8b6d735

Deprecated the use of a channles_first variable in order to rely on the golbal keras backend image format varialbe.

Sparkier reviewed Feb 22, 2021

View reviewed changes

fuzhanrahmanian added 9 commits April 13, 2021 00:37

Best outcome based on this configuration

304ed73

Delete main.py

585cfb7

Delete luna/outputs directory

747f5be

Delete feature_vis_inceptionV3_mixed5_16.npy

17debc7

Delete feature_vis_inceptionV3_mixed5_16.npy

408748c

Delete feature_vis_inceptionV3_mixed5_4.npy

0be6752

Delete feature_vis_inceptionV3_mixed5_73.npy

cc45b35

added tensorflow_addons to requirement

7326760

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

d8dde76

fuzhanrahmanian requested a review from Sparkier April 12, 2021 22:48

Sparkier requested changes Apr 13, 2021

View reviewed changes

Further correction and experimentation with transformation

5cec335

Alexnet was deleted for the moment Pull request correction Adaptation for optimal Googlenet results

fuzhanrahmanian requested a review from Sparkier April 23, 2021 07:40

Sparkier requested changes Apr 23, 2021

View reviewed changes

added generalization classes for optimization and augmentaion

9b6c9b9

clean the code for pylint

fuzhanrahmanian requested a review from Sparkier April 26, 2021 21:26

Sparkier approved these changes Apr 28, 2021

View reviewed changes

Sparkier merged commit 3d6a72d into Sparkier:master Apr 28, 2021

		feature_extractor = get_feature_extractor(model, layer)

		# Temporary method for random choice of


		return tf.where(return_relu_grad, relu_grad, result_grad)

		#pylint: disable=R0914


		#pylint: disable=too-many-locals
		#pylint: disable=too-many-arguments

Remove dependency of architectures #8

Remove dependency of architectures #8

Conversation

fuzhanrahmanian commented Feb 21, 2021

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

fuzhanrahmanian Apr 13, 2021 • edited Loading

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

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

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

Sparkier left a comment

Choose a reason for hiding this comment

fuzhanrahmanian Apr 13, 2021 •

edited

Loading