Add tanhshrink #493

fsx950223 · 2019-09-10T02:54:23Z

Related #437

WindQAQ · 2019-09-12T19:56:10Z

tensorflow_addons/activations/tanhshrink.py

+
+@keras_utils.register_keras_custom_object
+@tf.function
+def tanhshrink(features, name="Tanhshrink"):


Seems that I missed the name argument in my previous PRs. @facaiy @seanpmorgan Should we strictly follow this? Like activations in tf.keras.activations.*, they do not expose name argument.

I believe the name argument is important for debugging.

Sorry for the delay, I have no objection, however I prefer to keep consistency with keras module. name often conflicts with arguments backward compatibility, and it won't hurt to add it in the future. What do you think, @fsx950223 , @WindQAQ Tzu-Wei?

Sorry for the delay, I have no objection, however I prefer to keep consistency with keras module. name often conflicts with arguments backward compatibility, and it won't hurt to add it in the future. What do you think, @fsx950223 , @WindQAQ Tzu-Wei?

I opened an issue #505. We can discuss in the issue

tensorflow_addons/activations/tanhshrink.py

WindQAQ

@fsx950223 Hello, I drop some comments here. Feel free to ping me if you have further problem.

fsx950223 · 2019-09-13T04:00:14Z

@fsx950223 Hello, I drop some comments here. Feel free to ping me if you have further problem.

Keras api is based on tensorflow.nn api. Maybe we should define keras api and custom op api separately.

WindQAQ · 2019-09-13T04:05:02Z

Keras api is based on tensorflow.nn api. Maybe we should define keras api and custom op api separately.

We have discussed this previously #426. But you could still open a new issue or re-use the old one for further discussion.

fsx950223 · 2019-09-13T04:17:47Z

Keras api is based on tensorflow.nn api. Maybe we should define keras api and custom op api separately.

We have discussed this previously #426. But you could still open a new issue or re-use the old one for further discussion.

I agree with the final discussion result. But we should follow tensorflow.nn.* apis' style.
Official example just use tensorflow.nn.* for now.

WindQAQ · 2019-09-18T00:10:18Z

Hi @fsx950223, as a side note, don't forget to follow the namespace change as per #516.

fsx950223 · 2019-09-18T03:08:13Z

tensorflow.python.framework.errors_impl.InvalidArgumentError: Invalid name: Addons>Tanhshrink (Did you use CamelCase?); in OpDef: name: "Addons>Tanhshrink" input_arg { name: "features" type_attr: "T" } output_arg { name: "activations" type_attr: "T" } attr { name: "T" type: "type" allowed_values { list { type: DT_HALF type: DT_FLOAT type: DT_DOUBLE } } }

Why? Any other changes?

WindQAQ

The error you mentioned above is probably because you do not use the latest nightly tensorflow. Try to install tf-nightly-gpu-2.0-preview. Also, I drop some comments below, please check it out. Thanks for the contribution.

WindQAQ · 2019-09-18T03:20:07Z

tensorflow_addons/activations/tanhshrink.py

+
+    Args:
+        features: A `Tensor`. Must be one of the following types:
+            `float16`, `float32`, `float64`, `int32`, `int64`.


Remove int32 and int64.

WindQAQ · 2019-09-18T03:20:54Z

tensorflow_addons/custom_ops/activations/cc/kernels/tanhshrink_op.h

+
+#undef EIGEN_USE_THREADS
+
+#endif


#endif // TENSORFLOW_ADDONS_ACTIVATIONS_KERNELS_TANHSHRINK_OP_H_

WindQAQ · 2019-09-18T03:31:38Z

tensorflow_addons/custom_ops/activations/cc/kernels/tanhshrink_op.h

+  // INPUTS:
+  //   g (gradients): backpropagated gradients
+  //   a (inputs): either the inputs that were passed to TanhshrinkOp(), or its
+  //               outputs (using either one yields the same result here).


I think in your implementation a (inputs) should be the inputs that were passed to TanhshrinkOp() instead of its outputs, which will not yield the same result.

facaiy · 2019-09-18T03:50:35Z

Try to install tf-nightly-gpu-2.0-preview.

+1, please try to upgrade tf

fsx950223 · 2019-09-19T00:18:54Z

Fixed. @WindQAQ

WindQAQ

Want to check the last thing :-) Thanks.

tensorflow_addons/activations/tanhshrink.py

fsx950223 · 2019-09-19T06:06:26Z

Want to check the last thing :-) Thanks.

Please review again.

WindQAQ

Thanks!

fsx950223 requested review from facaiy, seanpmorgan and a team as code owners September 10, 2019 02:54

googlebot added the cla: yes label Sep 10, 2019

seanpmorgan added the kokoro:force-run label Sep 10, 2019

kokoro-team removed the kokoro:force-run label Sep 10, 2019

seanpmorgan added the activations label Sep 10, 2019

seanpmorgan mentioned this pull request Sep 12, 2019

Add GIOU loss #477

Merged

WindQAQ reviewed Sep 12, 2019

View reviewed changes

tensorflow_addons/activations/tanhshrink.py Outdated Show resolved Hide resolved

WindQAQ reviewed Sep 12, 2019

View reviewed changes

fsx950223 mentioned this pull request Sep 13, 2019

Activation api style #505

Closed

fsx950223 force-pushed the activation branch from a724813 to df686c3 Compare September 18, 2019 02:55

fsx950223 requested a review from Squadrick as a code owner September 18, 2019 02:55

fsx950223 force-pushed the activation branch 2 times, most recently from 0a50383 to 9e09680 Compare September 18, 2019 03:02

fsx950223 added 6 commits September 18, 2019 11:03

Add tanhshrink

0ad0dac

Update readme

df3672a

Add name

924d4ff

Add gradient

f9641a3

Fix build bug

4ebd26f

Migrate api style

18f1bb8

fsx950223 force-pushed the activation branch from 9e09680 to 18f1bb8 Compare September 18, 2019 03:05

WindQAQ added the kokoro:force-run label Sep 18, 2019

kokoro-team removed the kokoro:force-run label Sep 18, 2019

WindQAQ requested changes Sep 18, 2019

View reviewed changes

Fix docs

969dd10

fsx950223 force-pushed the activation branch from add33d1 to 969dd10 Compare September 18, 2019 05:59

Remove name argument

fc0e4c4

WindQAQ reviewed Sep 19, 2019

View reviewed changes

tensorflow_addons/activations/tanhshrink.py Outdated Show resolved Hide resolved

Remove name scope

b44b88b

WindQAQ added the kokoro:force-run label Sep 19, 2019

WindQAQ self-requested a review September 19, 2019 15:09

kokoro-team removed the kokoro:force-run label Sep 19, 2019

WindQAQ approved these changes Sep 19, 2019

View reviewed changes

WindQAQ merged commit 33baec0 into tensorflow:master Sep 19, 2019

WindQAQ mentioned this pull request Sep 19, 2019

fix permission #528

Merged


		#undef EIGEN_USE_THREADS

		#endif

Add tanhshrink #493

Add tanhshrink #493

Uh oh!

Conversation

fsx950223 commented Sep 10, 2019

Uh oh!

WindQAQ Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

fsx950223 Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

facaiy Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

fsx950223 Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WindQAQ left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fsx950223 commented Sep 13, 2019

Uh oh!

WindQAQ commented Sep 13, 2019

Uh oh!

fsx950223 commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WindQAQ commented Sep 18, 2019

Uh oh!

fsx950223 commented Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WindQAQ left a comment

Choose a reason for hiding this comment

Uh oh!

WindQAQ Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

WindQAQ Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

WindQAQ Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

facaiy commented Sep 18, 2019

Uh oh!

fsx950223 commented Sep 19, 2019

Uh oh!

WindQAQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fsx950223 commented Sep 19, 2019

Uh oh!

WindQAQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WindQAQ left a comment •

edited

Loading

fsx950223 commented Sep 13, 2019 •

edited

Loading

fsx950223 commented Sep 18, 2019 •

edited

Loading