Add quantized CELU operator by adding additional parameters to quantized ELU #39199

durumu · 2020-05-28T20:38:41Z

Stack from ghstack:

Add quantized CELU operator by adding additional parameters to quantized ELU #39199 Add quantized CELU operator by adding additional parameters to quantized ELU

Differential Revision: D21771202

[ghstack-poisoned]

Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

ghstack-source-id: f83f79f Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: f83f79f Pull Request resolved: #39200 Added tests ghstack-source-id: f83f79f Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: f83f79f Pull Request resolved: #39202

dr-ci · 2020-05-28T21:55:48Z

💊 CI failures summary and remediations

As of commit d48959e (more details on the Dr. CI page):

1/1 failures introduced in this PR

XLA failure

Job pytorch_xla_linux_bionic_py3_6_clang9_test is failing. Please create an issue with title prefixed by [PT_BREAK] in pytorch/xla and link to to this PR. If you have questions, please reach out to @ailzhang / @dlibenzi / @JackCaoG.

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 116 times.

vkuzo · 2020-05-29T15:48:30Z

there are some failing tests (like https://app.circleci.com/pipelines/github/pytorch/pytorch/174752/workflows/fa9a3d4b-602d-45b3-955f-8c32409c2eb5/jobs/5611015/steps), we should figure out what's going on with them before landing. The failure looks odd to me, do we have a local repro?

vkuzo · 2020-05-29T15:50:58Z

torch/nn/quantized/functional.py

        return output
    elif inplace:
-        return torch._C._nn.elu_(input, alpha)
+        return torch._C._nn.elu_(input, alpha, 1., 1.)


these have a default of 1 in native_functions.yaml, would it work to revert this and next line?

Sure, I can do that -- I didn't realize the C-style functions could have default arguments.

vkuzo · 2020-05-29T15:52:42Z

aten/src/ATen/native/quantized/cpu/qelu.cpp

 }

+Tensor& quantized_celu_(Tensor& self, Scalar alpha) {
+  double inv_alpha = 1. / alpha.to<double>();


can we verify that division by zero if alpha == 0 gives a good error message?

vkuzo

thanks for working on this! in general looks great, we should figure out why the derivative tests are failing before landing

supriyar · 2020-05-29T16:21:41Z

torch/nn/quantized/functional.py

+    # type: (Tensor, Optional[float], Optional[bool]) -> Tensor
+    r"""celu(input, inplace=False) -> Tensor
+
+    Applies the quantized continuously differentiable ELU function element-wise.


ELU -> CELU

CELU stands for continuously differentiable ELU -- maybe I should capitalize the C to make it clearer this is what I mean?

Oh right, missed that! Maybe we should say Continuously Differentiable Exponential Linear Units (CELU)

aten/src/ATen/native/native_functions.yaml

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: 37c1a45 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: 37c1a45 Pull Request resolved: #39200 Added tests ghstack-source-id: 37c1a45 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: 37c1a45 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: 389d2df Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: 389d2df Pull Request resolved: #39200 Added tests ghstack-source-id: 389d2df Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: 389d2df Pull Request resolved: #39202

supriyar · 2020-05-29T23:05:23Z

Changes look good! @vkuzo could you take a look as well?

vkuzo · 2020-05-30T00:25:13Z

looks good, can we verify that alpha == 0 is handled well with a good error message?

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: 9d1f973 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: 9d1f973 Pull Request resolved: #39200 Added tests ghstack-source-id: 9d1f973 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: 9d1f973 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: c20f914 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: c20f914 Pull Request resolved: #39200 Added tests ghstack-source-id: c20f914 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: c20f914 Pull Request resolved: #39202

durumu · 2020-06-02T23:48:51Z

@vkuzo I added a new error message that's more verbose - "ZeroDivisionError: alpha cannot be 0 in CELU".

vkuzo

looks great! thank you for working on this!

supriyar · 2020-06-23T05:03:56Z

aten/src/ATen/native/quantized/cpu/kernels/QuantizedOpKernels.cpp

+    Scalar scale,
+    Scalar input_scale,


Can you try giving default values to these arguments.

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: f2649e0 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: f2649e0 Pull Request resolved: #39200 Added tests ghstack-source-id: f2649e0 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: f2649e0 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: d75cf20 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: d75cf20 Pull Request resolved: #39200 Added tests ghstack-source-id: d75cf20 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: d75cf20 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: ca5c035 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: ca5c035 Pull Request resolved: #39200 Added tests ghstack-source-id: ca5c035 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: ca5c035 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: 447308a Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: 447308a Pull Request resolved: #39200 Added tests ghstack-source-id: 447308a Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: 447308a Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: 2b58934 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: 2b58934 Pull Request resolved: #39200 Added tests ghstack-source-id: 2b58934 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: 2b58934 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: 3a7f60d Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: 3a7f60d Pull Request resolved: #39200 Added tests ghstack-source-id: 3a7f60d Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: 3a7f60d Pull Request resolved: #39202

supriyar · 2020-06-26T21:20:05Z

test/quantization/test_quantized_op.py

+                                           dtype=torch_type)
+
+        # test regular
+        qY = torch.nn.quantized.functional.celu(qX, output_scale, output_zero_point, alpha=alpha)


I believe you can call torch.ops.quantized.celu here instead of functional

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: f70040e Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: f70040e Pull Request resolved: #39200 Added tests ghstack-source-id: f70040e Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: f70040e Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: dc6071a Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: dc6071a Pull Request resolved: #39200 Added tests ghstack-source-id: dc6071a Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: dc6071a Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: 86c2437 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: 86c2437 Pull Request resolved: #39200 Added tests ghstack-source-id: 86c2437 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: 86c2437 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: b92cff1 Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: b92cff1 Pull Request resolved: #39200 Added tests ghstack-source-id: b92cff1 Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: b92cff1 Pull Request resolved: #39202

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

…zed ELU ghstack-source-id: c5e88ba Pull Request resolved: #39199 Updated ELU to accept additional parameters ghstack-source-id: c5e88ba Pull Request resolved: #39200 Added tests ghstack-source-id: c5e88ba Pull Request resolved: #39201 Improved tests to fail when formula is wrong ghstack-source-id: c5e88ba Pull Request resolved: #39202

facebook-github-bot · 2020-07-18T02:16:32Z

@durumu merged this pull request in 445e7eb.

Dummy operator building correctly

d1996d7

[ghstack-poisoned]

durumu requested a review from apaszke as a code owner May 28, 2020 20:38

This was referenced May 28, 2020

Updated ELU to accept additional parameters #39200

Closed

Added tests #39201

Closed

Improved tests to fail when formula is wrong #39202

Closed

Update on "Dummy operator building correctly"

33a0d02

Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

durumu requested review from supriyar and vkuzo May 28, 2020 21:23

durumu changed the title ~~Dummy operator building correctly~~ Add quantized CELU operator by adding additional parameters to quantized ELU May 28, 2020

vkuzo reviewed May 29, 2020

View reviewed changes

supriyar reviewed May 29, 2020

View reviewed changes

aten/src/ATen/native/native_functions.yaml Outdated Show resolved Hide resolved

Update on "Add quantized CELU operator by adding additional parameter…

60d0bab

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

d47431a

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

2dd97a2

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

ddf7684

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

vkuzo approved these changes Jun 3, 2020

View reviewed changes

supriyar reviewed Jun 23, 2020

View reviewed changes

Update on "Add quantized CELU operator by adding additional parameter…

05b798d

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

9a8b9d0

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

910de49

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

1972ca5

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

d95738e

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

9416455

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

supriyar reviewed Jun 26, 2020

View reviewed changes

Update on "Add quantized CELU operator by adding additional parameter…

395a40d

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

0585b56

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

00e7c70

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

bad5413

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

Update on "Add quantized CELU operator by adding additional parameter…

d48959e

…s to quantized ELU" Differential Revision: [D21771202](https://our.internmc.facebook.com/intern/diff/D21771202) [ghstack-poisoned]

facebook-github-bot closed this in 445e7eb Jul 18, 2020

facebook-github-bot added the merged label Jul 18, 2020

facebook-github-bot deleted the gh/durumu/2/head branch July 21, 2020 14:17

mruberry added the Merged label Oct 28, 2020

Add quantized CELU operator by adding additional parameters to quantized ELU #39199

Add quantized CELU operator by adding additional parameters to quantized ELU #39199

Uh oh!

Conversation

durumu commented May 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented May 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

XLA failure

Uh oh!

vkuzo commented May 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vkuzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

supriyar commented May 29, 2020

Uh oh!

vkuzo commented May 30, 2020

Uh oh!

durumu commented Jun 2, 2020

Uh oh!

vkuzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jul 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

durumu commented May 28, 2020 •

edited

Loading

dr-ci bot commented May 28, 2020 •

edited

Loading