Release v0.6.0 #39
Release v0.6.0 #39
Conversation
keisen
added
bug
keisen
force-pushed
the
release/v0.6.0
branch
from
40f9ace
to
51fee93
Compare
keisen
force-pushed
the
release/v0.6.0
branch
from
8706610
to
0fd711c
Compare
|
As the author of #43 and #45, I was interested in testing this using |
|
Then, this test code does not run. It runs fine without import tensorflow as tf
from tf_keras_vis.activation_maximization import ActivationMaximization
policy = tf.keras.mixed_precision.experimental.Policy("mixed_float16")
tf.keras.mixed_precision.experimental.set_policy(policy)
model = tf.keras.applications.MobileNet()
ActivationMaximization(model)(lambda x: x, tf.zeros(model.input.shape[1:]))
print("Done")Output is |
|
Then, you seem to rely a lot on the compute policy being set, but that is not actually ensured. Look at this example (run twice to see the error). You might instead rely on import sys
from pathlib import Path
import tensorflow as tf
from tf_keras_vis.activation_maximization import ActivationMaximization
model_file = Path("bug.tf")
if not model_file.exists():
policy = tf.keras.mixed_precision.experimental.Policy("mixed_float16")
tf.keras.mixed_precision.experimental.set_policy(policy)
model = tf.keras.applications.MobileNet()
model.save(model_file)
sys.exit()
model = tf.keras.models.load_model(model_file)
ActivationMaximization(model)(lambda x: x, tf.zeros(model.input.shape[1:]))
print("Done")Error is |
|
@bersbersbers , Thank you for your code-snippet. Thanks! |
You're the boss, but do know that I disagree:
|
@bersbersbers , Here is a open source project, so we only have fun and contribute it as possible as we can. As I said before, unfortunately, I don't have a time enough to do all. Or
As I said begore, because here is a open source, you can open and submit a PullRequest. Either way, I want to release v0.6.0 soon. Thanks! |
Sure - I did not want to criticize anyone personally, just add my perspective on the issue.
I can easily submit #39 (comment) as a PR, if that is your intention. I have tested it locally and it solves the issue. If you are looking for something else, please let me know what you are looking for. To my first solution, you replied it wasn't "intuitive"; to my second solution, you reacted with a "confused" emoji; and that was all I got as a reply. Really, I am happy to contribute, but after declining two of my solutions you need to give me some additional criteria regarding what you think defines an acceptable solution. |
|
@keisen would you mind explaining what you think the bug in TensorFlow is? I'd like to help isolate and report it upstream, but I don't see where TensorFlow is misbehaving. Edit: as I see it, the problem is when you add
In summary, I don't see where you think the TF bug is. |
|
Here's another basic idea to fix this issue. diff --git a/tf_keras_vis/activation_maximization/__init__.py b/tf_keras_vis/activation_maximization/__init__.py
index 134c52d..d8c31c5 100644
--- a/tf_keras_vis/activation_maximization/__init__.py
+++ b/tf_keras_vis/activation_maximization/__init__.py
@@ -118,6 +118,7 @@ class ActivationMaximization(ModelVisualization):
for modifier in input_modifiers[name]:
seed_inputs[j] = modifier(seed_inputs[j])
+ regularizer_seed_inputs = seed_inputs
if mixed_precision_enabled:
seed_inputs = (tf.cast(X, dtype=lower_precision_dtype(self.model))
for X in seed_inputs)
@@ -130,7 +131,7 @@ class ActivationMaximization(ModelVisualization):
outputs = listify(outputs)
score_values = self._calculate_scores(outputs, scores)
# Calculate regularization values
- regularizations = [(regularizer.name, regularizer(seed_inputs))
+ regularizations = [(regularizer.name, regularizer(regularizer_seed_inputs))
for regularizer in regularizers]
regularized_score_values = [
(-1. * score_value) + sum([v for _, v in regularizations])Why does this work? You save the original ( If you think it's more maintainable, you can also introduce |
|
@bersbersbers I'm sorry for the late reply!
I believe that even if we fixed the error, some problems are still remain. Regularization values may be The reason of my reactions (emoji) are just because those ways can't fix the problem above. I can't decide whether to support mixed-precsion in ActivationMaximization is good or not. So I have no strong motivation to fully support Thanks your contributions! |
That is true always and everywhere.
Is that specific to
Well, for one, that is expected - these are different networks. Who would expect different networks to produce the same result? Second, are users less confused when Anyway, I will apply the changes mentioned above locally and be happy with them. Thanks and good luck! |
It looks good. Is there any impact on calculation result by the patch? |
I cannot say really: I don't have any comparison as the code without this patch does not run for my saved mixed-precision models, so this is the only result that I have. And my models take so long to train that I cannot re-train them with full precision now. I can say that the results I get are somewhat expected, but I was hoping that your testing pipeline could shed more light on the immediate comparison between full/mixed precision |
mixed-precisionof Tensorflow 2.4+, (i.e., NOT support experimental API).LosstoScoreCloses #24, #43 , #45, #47 and #51