add rrelu kernel #573

fsx950223 · 2019-10-06T08:49:17Z

kernel implemention of rrelu. @WindQAQ

tensorflow_addons/custom_ops/activations/cc/ops/rrelu_op.cc

WindQAQ

@fsx950223 Thanks for the contribution! Generally LGTM. I know this is WIP but want to address some points first. Thanks again for bringing rrelu up!

tensorflow_addons/activations/rrelu.py

tensorflow_addons/activations/rrelu_test.py

tensorflow_addons/custom_ops/activations/cc/kernels/rrelu_op.cc

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

tensorflow_addons/custom_ops/activations/cc/kernels/rrelu_op_gpu.cu.cc

tensorflow_addons/custom_ops/activations/cc/ops/rrelu_op.cc

fsx950223 · 2019-10-08T03:24:06Z

It seems the formal(5) shows in https://arxiv.org/pdf/1505.00853.pdf is wrong.It should be x*(lower+upper)/2

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

It seems the formal(5) shows in https://arxiv.org/pdf/1505.00853.pdf is wrong.It should be x*(lower+upper)/2

I could confirm that pytorch computes like x * (lower + upper) / 2.
https://github.com/pytorch/pytorch/blob/master/aten/src/THNN/generic/RReLU.c#L66-L72

fsx950223 · 2019-10-09T03:06:13Z

tensorflow_addons/activations/rrelu_test.py

+        x = tf.constant([-2.0, -1.0, 0.0, 1.0, 2.0], dtype=dtype)
+        lower = 0.2
+        upper = 0.2
+        result, alpha = rrelu(x, lower, upper, training=True)


Do you have any idea about random test? @WindQAQ

I think we do not have to return alpha for rrelu OP. In your experiment, setting seed in TF is not enough, right?

Argument with_alpha can control the return behavior and I don't need seed

@fsx950223 I do not think it's a good practice to have an argument to return such things for public API. Is alpha deterministic while setting the same TF seed?

No, it isn't and I don't know how to let alpha deterministic.Is there a better way to set seed?

Umm, I think we could just test values with training=False. As for gradient testing, we should check both of them. How do feel about this?

seanpmorgan · 2019-10-14T23:41:01Z

Hi @fsx950223 when time allows, mind refactoring to use the custom_op_library from #581

WindQAQ

Generally LGTM. Some API designs are needed to discuss. Thanks!

WindQAQ · 2019-10-15T15:48:29Z

tensorflow_addons/activations/rrelu.py

+    if with_alpha:
+        return result, alpha
+    else:
+        return result


I have no idea if we should expose an argument to return alpha or not. My survey is that chainer does but pytorch does not. What do you think @fsx950223 ?

I return alpha just for test. It could be removed if the test case is fixed without alpha.

The problem seems has been solved by using philox random.

WindQAQ · 2019-10-15T15:48:40Z

tensorflow_addons/activations/rrelu_test.py

+                grad = t.gradient(result, x)
+                expect_grad = _ref_rrelu_grad(x, alpha, dtype)
+                self.assertAllCloseAccordingToType(
+                    grad, expect_grad, atol=1e-4)


Could we use tf.test.compute_gradient to check gradients? Just like what you do in hardtanh.

The test failed when the training argument is True and test failed when training argument is False and data is 0.

@fsx950223 Sorry for the late reply. Could you push the codes so that I could test it locally? Thanks.

@fsx950223 Sorry for the late reply. Could you push the codes so that I could test it locally? Thanks.

How do you debug the code. I can't step into source code anymore since tf2 released. @WindQAQ

Edit: I successfully re-configured the environment with ./configure.sh on the docker early today.

The problem is that I can't go to code definition.

I tried it in Vscode and Pycharm, but neither of them worked.

I believe _compute_numeric_jacobian is not suitable for this test case.Because the activcation has different gradients around 0. Please reference Relu test case

I believe _compute_numeric_jacobian is not suitable for this test case.Because the activcation has different gradients around 0. Please reference Relu test case

I think it's ok to modify the input value to avoid non-smooth part. Please see relu's gradient check.
https://github.com/tensorflow/tensorflow/blob/66ea3ed9b8cbbbf01b0eabb14e436883895e4bde/tensorflow/python/kernel_tests/relu_op_test.py#L123

The problem is that I can't go to code definition.

I tried it in Vscode and Pycharm, but neither of them worked.

Ohoh. Got it. But could the code execute successfully if ide can not find the definition of functions? Sorry that I usually use vim so that I cannot understand what you talk about for a while... Thanks!

tensorflow_addons/custom_ops/activations/BUILD

fsx950223 · 2019-10-20T14:47:05Z

Good Tzu-Wei Sung <[email protected]<mailto:[email protected]>> 于 2019年10月20日周日下午10:43写道： @WindQAQ commented on this pull request.

________________________________ In tensorflow_addons/activations/rrelu_test.py<#573 (comment)>:

+ ("float64", np.float64))

+ @tf.function + def test_theoretical_gradients(self, dtype): + x = tf.constant([-2.0, -1.0, 0.0, 1.0, 2.0], dtype=dtype) + lower = 0.1 + upper = 0.2 + for training in [True, False]: + with self.subTest(training=training): + with tf.GradientTape() as t: + t.watch(x) + result, alpha = rrelu( + x, lower, upper, training=training, with_alpha=True) + grad = t.gradient(result, x) + expect_grad = _ref_rrelu_grad(x, alpha, dtype) + self.assertAllCloseAccordingToType( + grad, expect_grad, atol=1e-4) I believe _compute_numeric_jacobian is not suitable for this test case.Because the activcation has different gradients around 0. Please reference Relu test case <https://github.com/tensorflow/tensorflow/blob/66ea3ed9b8cbbbf01b0eabb14e436883895e4bde/tensorflow/python/kernel_tests/relu_op_test.py#L61> I think it's ok to modify the input value to avoid non-smooth part. Please see relu's gradient check. https://github.com/tensorflow/tensorflow/blob/66ea3ed9b8cbbbf01b0eabb14e436883895e4bde/tensorflow/python/kernel_tests/relu_op_test.py#L123 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#573?email_source=notifications&email_token=AEGHB4YP7H2WHX7KATPATDLQPRU75A5CNFSM4I53DJ72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCIRKWHA#discussion_r336781718>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEGHB44BSBZ77YZDPTGRMU3QPRU75ANCNFSM4I53DJ7Q>.

fsx950223 · 2019-10-20T14:52:42Z

Amazing. There is something wrong debug by vscode.But it works well with pycharm. Tzu-Wei Sung <[email protected]<mailto:[email protected]>> 于 2019年10月20日周日下午10:47写道： @WindQAQ commented on this pull request.

________________________________ In tensorflow_addons/activations/rrelu_test.py<#573 (comment)>:

+ ("float64", np.float64))

+ @tf.function + def test_theoretical_gradients(self, dtype): + x = tf.constant([-2.0, -1.0, 0.0, 1.0, 2.0], dtype=dtype) + lower = 0.1 + upper = 0.2 + for training in [True, False]: + with self.subTest(training=training): + with tf.GradientTape() as t: + t.watch(x) + result, alpha = rrelu( + x, lower, upper, training=training, with_alpha=True) + grad = t.gradient(result, x) + expect_grad = _ref_rrelu_grad(x, alpha, dtype) + self.assertAllCloseAccordingToType( + grad, expect_grad, atol=1e-4) The problem is that I can't go to code definition. [Screenshot from 2019-10-20 17-49-01]<https://user-images.githubusercontent.com/17592563/67157689-1a97a080-f362-11e9-8cd5-4a67fcbb47a4.png> I tried it in Vscode and Pycharm, but neither of them worked. Ohoh. Got it. But could the code execute successfully if ide can not find the definition of functions? Sorry that I usually use vim so that I cannot understand what you talk about for a while... Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#573?email_source=notifications&email_token=AEGHB46G442FC6CHWUBNDP3QPRVQVA5CNFSM4I53DJ72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCIRKX6Y#discussion_r336781937>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEGHB443GY4T3AZUNYQBRV3QPRVQVANCNFSM4I53DJ7Q>.

fsx950223 · 2019-10-24T07:36:52Z

The CPU kernel and the GPU kernel have different behaviors?

tensorflow_addons/activations/rrelu_test.py

WindQAQ

Thanks for the nice and long work!

tensorflow_addons/activations/rrelu.py

WindQAQ

@fsx950223 Nice PR. Thanks for the contribution!

fsx950223 requested review from facaiy, seanpmorgan and a team as code owners October 6, 2019 08:49

googlebot added the cla: yes label Oct 6, 2019

seanpmorgan added the activations label Oct 6, 2019

WindQAQ mentioned this pull request Oct 6, 2019

Add rrelu #542

Closed

WindQAQ added the kokoro:force-run label Oct 7, 2019

kokoro-team removed the kokoro:force-run label Oct 7, 2019

fsx950223 commented Oct 7, 2019

View reviewed changes

tensorflow_addons/custom_ops/activations/cc/ops/rrelu_op.cc Show resolved Hide resolved

WindQAQ requested changes Oct 7, 2019

View reviewed changes

fsx950223 force-pushed the rrelu branch from 2459e94 to c673733 Compare October 8, 2019 12:08

fsx950223 commented Oct 9, 2019

View reviewed changes

fsx950223 force-pushed the rrelu branch 4 times, most recently from 9a8e542 to bd20659 Compare October 11, 2019 07:27

WindQAQ added the kokoro:force-run label Oct 11, 2019

kokoro-team removed the kokoro:force-run label Oct 11, 2019

WindQAQ reviewed Oct 15, 2019

View reviewed changes

fsx950223 force-pushed the rrelu branch from e727a6f to 9aeb856 Compare October 16, 2019 01:22

fsx950223 added 7 commits October 20, 2019 05:44

add rrelu kernel

7874b97

format code

3db6502

change test case

bb26705

simplify kernel implemention

9f2eb56

fix test case

e6e2815

refact test case

47aacae

remove forward training test

b1410c6

fsx950223 added 3 commits October 20, 2019 05:44

format code

736a3dd

modify test case

c75136b

add forget files

e708056

fsx950223 force-pushed the rrelu branch from 9aeb856 to e708056 Compare October 20, 2019 06:54

update kernel random implementation

d0c117d

WindQAQ added the kokoro:force-run label Oct 24, 2019

kokoro-team removed the kokoro:force-run label Oct 24, 2019

fix rrelu test bug

c24995c

fsx950223 commented Oct 25, 2019

View reviewed changes

tensorflow_addons/activations/rrelu_test.py Show resolved Hide resolved

add TODO

41f1633

WindQAQ reviewed Oct 25, 2019

View reviewed changes

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

tensorflow_addons/activations/rrelu.py Show resolved Hide resolved

fsx950223 force-pushed the rrelu branch from ba704c9 to 65eadc9 Compare October 25, 2019 05:32

fix comment

acf4374

fsx950223 force-pushed the rrelu branch from 65eadc9 to acf4374 Compare October 25, 2019 05:32

WindQAQ added the kokoro:force-run label Oct 25, 2019

kokoro-team removed the kokoro:force-run label Oct 25, 2019

WindQAQ added the kokoro:force-run label Oct 25, 2019

kokoro-team removed the kokoro:force-run label Oct 25, 2019

WindQAQ self-requested a review October 25, 2019 07:22

WindQAQ approved these changes Oct 25, 2019

View reviewed changes

WindQAQ merged commit 8a49f91 into tensorflow:master Oct 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add rrelu kernel #573

add rrelu kernel #573

fsx950223 commented Oct 6, 2019 •

edited

Loading

WindQAQ left a comment

fsx950223 commented Oct 8, 2019 •

edited

Loading

WindQAQ commented Oct 9, 2019

fsx950223 Oct 9, 2019

WindQAQ Oct 11, 2019

fsx950223 Oct 11, 2019

WindQAQ Oct 11, 2019

fsx950223 Oct 11, 2019 •

edited

Loading

WindQAQ Oct 11, 2019

fsx950223 Oct 11, 2019

seanpmorgan commented Oct 14, 2019

WindQAQ left a comment

WindQAQ Oct 15, 2019

fsx950223 Oct 16, 2019

fsx950223 Oct 21, 2019 •

edited

Loading

WindQAQ Oct 15, 2019

fsx950223 Oct 16, 2019 •

edited

Loading

WindQAQ Oct 19, 2019

fsx950223 Oct 20, 2019

fsx950223 Oct 20, 2019 •

edited

Loading

WindQAQ Oct 20, 2019

fsx950223 Oct 20, 2019

fsx950223 Oct 20, 2019

WindQAQ Oct 20, 2019

WindQAQ Oct 20, 2019

fsx950223 commented Oct 20, 2019 via email

fsx950223 commented Oct 20, 2019 via email

fsx950223 commented Oct 24, 2019

WindQAQ left a comment

WindQAQ left a comment

add rrelu kernel #573

add rrelu kernel #573

Conversation

fsx950223 commented Oct 6, 2019 • edited Loading

WindQAQ left a comment

Choose a reason for hiding this comment

fsx950223 commented Oct 8, 2019 • edited Loading

WindQAQ commented Oct 9, 2019

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

fsx950223 Oct 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanpmorgan commented Oct 14, 2019

WindQAQ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fsx950223 Oct 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fsx950223 Oct 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fsx950223 Oct 20, 2019 • 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

fsx950223 commented Oct 20, 2019 via email

fsx950223 commented Oct 20, 2019 via email

fsx950223 commented Oct 24, 2019

WindQAQ left a comment

Choose a reason for hiding this comment

WindQAQ left a comment

Choose a reason for hiding this comment

fsx950223 commented Oct 6, 2019 •

edited

Loading

fsx950223 commented Oct 8, 2019 •

edited

Loading

fsx950223 Oct 11, 2019 •

edited

Loading

fsx950223 Oct 21, 2019 •

edited

Loading

fsx950223 Oct 16, 2019 •

edited

Loading

fsx950223 Oct 20, 2019 •

edited

Loading