Twice differentiability of pointwise functions by t-vi · Pull Request #1531 · pytorch/pytorch

t-vi · 2017-05-10T20:17:46Z

Hi,

this attempts to convert pointwise.py to new-style functions.

I have tried to stay to close to the original and close to the argument order of torch.foo functions (in particular AddCmul and AddCdiv).
Also, I am not 100% sure about the backward in _ConstantGrad - the old implementation had quite an elaborate allocation and I just wrote grad_output.mul(cls.grad_value) without fully understanding the motivation for the old code.

I have converted a few inplace computations to not inplace to deal with gradients being variables.

It seems to pass the test/test_autograd.py and flake8.

Thank you for considering this pull request.

Thomas

t-vi · 2017-05-10T20:37:03Z

O_o I should run the full test suite.

torch/autograd/_functions/pointwise.py

+        return grad_output * i.mul(i).add(1).reciprocal()


 class Reciprocal(Function):


torch/autograd/_functions/pointwise.py

+    @staticmethod
+    def backward(ctx, grad_output):
+        i, = ctx.saved_variables
+        return grad_output * i.mul(i).add(1).reciprocal()


torch/autograd/_functions/pointwise.py

+    @staticmethod
+    def backward(ctx, grad_output):
+        i, = ctx.saved_variables
        return grad_output.mul(i.pow(-0.5)).div(2)


torch/autograd/_functions/pointwise.py

+    @staticmethod
+    def backward(ctx, grad_output):
+        a, b = ctx.saved_variables
+        mask = a.gt(b).type_as(a)


torch/autograd/_functions/pointwise.py

-            grad_output * self._min_buffer,
-            grad_output * self._min_buffer.eq(0).type_as(grad_output)
+            grad_output * mask,
+            grad_output * mask.eq(0).type_as(grad_output)


torch/autograd/_functions/pointwise.py

-        grad_input = grad_output.new(*repeat(1, grad_output.dim()))
-        grad_input = grad_input.fill_(self.grad_value).expand_as(grad_output)
-        return grad_input.mul(grad_output)
+    @classmethod


torch/autograd/_functions/pointwise.py

+    @staticmethod
+    def backward(ctx, grad_output):
+        return grad_output.mul(1 - ctx._weight), grad_output.mul(ctx._weight), None
+        # TODO: grad for weight?


torch/autograd/_functions/pointwise.py

-            self.mark_dirty(add_tensor)
-            return add_tensor.addcmul_(self.scale, mul_tensor1, mul_tensor2)
+    @staticmethod
+    def forward(ctx, add_tensor, *args, **argsd):


torch/autograd/_functions/pointwise.py

-            return add_tensor.addcmul_(self.scale, mul_tensor1, mul_tensor2)
+    @staticmethod
+    def forward(ctx, add_tensor, *args, **argsd):
+        inplace = argsd.get("inplace", False)


torch/autograd/_functions/pointwise.py

-            self.mark_dirty(add_tensor)
-            return add_tensor.addcdiv_(self.scale, div_tensor1, div_tensor2)
+    @staticmethod
+    def forward(ctx, add_tensor, *args, **argsd):


t-vi · 2017-05-10T21:39:44Z

Thank you for helping me improve my code! I hope this is better.

apaszke

Looks great! Just 3 minor comments and it's good to go!

torch/autograd/_functions/pointwise.py

-        if self.needs_input_grad[2]:
-            grad_mul2 = grad_output.mul(mul_tensor1).mul(self.scale)
+        if ctx.needs_input_grad[2]:
+            grad_mul2 = grad_output.mul(mul_tensor1).mul(ctx._scale)


torch/autograd/_functions/pointwise.py

-            grad_div2.neg_().mul_(self.scale)
-
-        return grad_add, grad_div1, grad_div2
+            grad_div2 = grad_output.mul(div_tensor1).div(div_tensor2_sq).neg().mul(ctx._scale)


torch/autograd/variable.py

-            return op()(self, *args)
+            return op.apply(self, *args)

    def addcmul(self, *args):


… of derivatives

t-vi · 2017-05-12T09:19:45Z

Trying to get the CI to try again

t-vi · 2017-05-12T09:34:44Z

So I think I have the comments covered except for the inplace test which I understand is automatic and not manual.

torch/autograd/_functions/pointwise.py

-            grad_div2.neg_().mul_(self.scale)
-
-        return grad_add, grad_div1, grad_div2
+            grad_div2 = grad_output.mul(div_tensor1).div(div_tensor2_sq).neg_().mul_(ctx._scale)


apaszke · 2017-05-14T21:03:28Z

Can you please also fix the conflicts?

soumith · 2017-05-15T17:30:02Z

I resolved conflicts and made that change.

soumith · 2017-05-15T17:32:18Z

@pytorchbot test this please

t-vi · 2017-05-15T17:44:54Z

Hi

Apologies, I've been travelling with no access to my torch computer. Thanks for fixing my patches.
Best regards

Thomas

t-vi added 2 commits May 10, 2017 22:06

Twice differentiability of pointwise functions

99e2402

python2 compatibility

c1dd758

t-vi closed this May 10, 2017

apaszke reopened this May 10, 2017

fix users of autograd

12c9007

apaszke reviewed May 10, 2017

View reviewed changes

t-vi added 2 commits May 10, 2017 23:25

review fixes

74e9ef3

style fixes for pointwise.py

71916d0

apaszke reviewed May 11, 2017

View reviewed changes

add inplace addcmul_ addcdiv_ and use more inplace in the computation…

9390682

… of derivatives

t-vi closed this May 12, 2017

t-vi reopened this May 12, 2017

apaszke approved these changes May 14, 2017

View reviewed changes

torch/autograd/_functions/pointwise.py Outdated

grad_div2.neg_().mul_(self.scale)

return grad_add, grad_div1, grad_div2

grad_div2 = grad_output.mul(div_tensor1).div(div_tensor2_sq).neg_().mul_(ctx._scale)

This comment was marked as off-topic.

Sign in to view

soumith added 2 commits May 15, 2017 11:28

Update pointwise.py

50b4c7f

Merge branch 'master' into second_derivatives

373f65e

apaszke approved these changes May 15, 2017

View reviewed changes

soumith merged commit 6107d15 into pytorch:master May 15, 2017

ezyang added the open source label Jun 24, 2019

zasdfgbnm pushed a commit to zasdfgbnm/pytorch that referenced this pull request Mar 31, 2022

Fixes pytorch#1530 (pytorch#1531)

fe82228

		return grad_output * i.mul(i).add(1).reciprocal()


		class Reciprocal(Function):

Conversation

t-vi commented May 10, 2017

Uh oh!

t-vi commented May 10, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

t-vi commented May 10, 2017

Uh oh!

apaszke left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

t-vi commented May 12, 2017

Uh oh!

t-vi commented May 12, 2017

Uh oh!

This comment was marked as off-topic.

Uh oh!

apaszke commented May 14, 2017

Uh oh!

soumith commented May 15, 2017

Uh oh!

soumith commented May 15, 2017

Uh oh!

t-vi commented May 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants