Repository: pytorch/pytorch
Pull request: Modify derivatives for efficiency and change `destination` to `result` for consistency
This closes https://github.com/pytorch/pytorch/issues/4402. Also, the variable names for the return values have been changed from `destination` to `result`.

Problem statement:
Some computational efficiency improvements in derivatives
I was going through the `derivatives.yaml` for the derivative of existing functions in `torch`, and feel like some of them are a bit inefficient. Some of them are listed below:

- `reciprocal` : From `grad / -(self * self)` to `- grad * result * result`
- `sqrt`: From `grad * self.pow(-0.5) / 2` to `grad * self.rsqrt() * 0.5` or `grad / (2 * result)`
- `acos`: From `grad * -((-self * self + 1).sqrt().reciprocal())` to `grad * -(self.rsqrt(self * self + 1))` and similarly for `asin`
- `div(Tensor self, Tensor other)`: Modify `other`'s derivative from `-grad * self / (other * other)` to `-grad * result / other`
- `tan`: From `grad / self.cos().pow(2)` to `grad * (1 + result.pow(2))`
- `atan`: From `grad * (self * self + 1).reciprocal()` to `grad / (self * self + 1)`
- `std`: From `grad * (2 * result).reciprocal()` to `grad / (2 * result)`
- `addcdiv`: From `-grad * value * tensor1 / (tensor2 * tensor2)` to `grad * (self - result) / tensor2`

Here I am assuming that a divide operation is costlier than a multiply operation.

Let me know your views about these ops, and I will issue a PR if required. 

- [x] `reciprocal`
- [x] `sqrt`
- [x] `acos`
- [x] `asin`
- [ ] `div`
- [x] `tan`
- [x] `atan`
- [x] `std`
- [ ] `addcdiv`

Hints from issue discussion:
* `reciprocal` looks good
* `sqrt` looks good (the version with rsqrt is better)
* `acos` + `asin` looks good
* `div` - I'm not sure about this. It would be best to use this version for the regular op, and the one we have right now for the in-place version. I don't think it's possible to do with with our current codegen, as in-place versions are automatically generated from regular definitions (or am I wrong @colesbury?)
Thanks. I will open a PR. Current improvements are in:
These changes make sense to me.

@apaszke, why would you use the current version of div for in-place?

I have a change that allows different derivatives for in-place functions, but I'd like to avoid special casing too many of the in-place derivatives.
I also have one question. Some ops have derivatives defined with `.reciprocal()` and some ops have derivatives with a direct division (`div` or `/`). Is there any difference between `.reciprocal()` and the standard `div`?
Oh my bad. The new definition would be actually much more efficient for in-place. Not sure why I thought differently. Sorry @colesbury 

@vishwakftw `x.reciprocal()` is equivalent to `1 / x`, but more efficient. On the other hand `y / x` is more efficient than `y * x.reciprocal()`.

Commit message: add changes for addcdiv also

Patch to review:
diff --git a/aten/src/ATen/Declarations.cwrap b/aten/src/ATen/Declarations.cwrap
index 4b3207d1f662..ea184350cd8f 100644
--- a/aten/src/ATen/Declarations.cwrap
+++ b/aten/src/ATen/Declarations.cwrap
@@ -1157,7 +1157,7 @@
     - method
     - function
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
 ]]
@@ -1893,7 +1893,7 @@
       return: argument 0
       scalar_check: keepdim == false && self_->dim() == 1
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: long dim
@@ -1924,7 +1924,7 @@
       return: argument 0
       scalar_check: keepdim == false && self_->dim() == 1
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: long dim
@@ -1959,7 +1959,7 @@
       return: argument 0
       scalar_check: keepdim == false && self_->dim() == 1
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: long dim
@@ -1992,7 +1992,7 @@
       return: argument 0
       scalar_check: keepdim == false && self_->dim() == 1
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: real p
@@ -2014,7 +2014,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - real p
@@ -2073,7 +2073,7 @@
     - cname: cinv
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
 ]]
@@ -2103,7 +2103,7 @@
     - cname: neg
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
 ]]
@@ -2132,7 +2132,7 @@
   cname: atan2
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - arg: THTensor* self
       broadcast: other fallback
@@ -2167,20 +2167,20 @@
   options:
     - cname: pow
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - real exponent
     - cname: cpow
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - arg: THTensor* self
           broadcast: exponent fallback
         - THTensor* exponent
     - cname: tpow
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - real base
         - THTensor* self
@@ -2220,7 +2220,7 @@
   return: argument 0
   cname: lerp
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - arg: THTensor* self
       broadcast: end fallback
@@ -2291,7 +2291,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - arg: long bins
@@ -2684,7 +2684,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - real min
@@ -2774,7 +2774,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - arg: long diagonal
@@ -2797,7 +2797,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - arg: long diagonal
@@ -2820,7 +2820,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - THTensor* other
diff --git a/tools/autograd/derivatives.yaml b/tools/autograd/derivatives.yaml
index e558feceeca6..6f3811d8e095 100644
--- a/tools/autograd/derivatives.yaml
+++ b/tools/autograd/derivatives.yaml
@@ -19,7 +19,7 @@
 #   - Any of the input arguments, tensor or non-tensor, including
 #     argument names tha only appear in Declarations.cwrap, e.g. 'output'.
 #   - 'result', representing the result of evaluating the forward
-#     expression for ATen native function decalarations. If the forward
+#     expression for ATen native function declarations. If the forward
 #     expression outputs a tuple, use 'resultX' instead to access the
 #     X-th entry
 #   - 'grad_input_mask', a std::array<bool, n> (where n is the number
@@ -56,7 +56,7 @@
   self: grad * self.sign()
 
 - name: acos(Tensor self)
-  self: grad * -((-self * self + 1).sqrt().reciprocal())
+  self: grad * -((-self * self + 1).rsqrt())
 
 - name: add(Tensor self, Scalar other, *, Scalar alpha)
   self: grad
@@ -73,7 +73,7 @@
 - name: addcdiv(Tensor self, Tensor tensor1, Tensor tensor2, *, Scalar value)
   self: grad
   tensor1: grad * value / tensor2
-  tensor2: -grad * value * tensor1 / (tensor2 * tensor2)
+  tensor2: grad * (self - result) / tensor2
 
 - name: addcmul(Tensor self, Tensor tensor1, Tensor tensor2, *, Scalar value)
   self: grad
@@ -106,10 +106,10 @@
   self: as_strided_backward(grad, TensorGeometry(self), size, stride, storage_offset)
 
 - name: asin(Tensor self)
-  self: grad * (-self * self + 1).sqrt().reciprocal()
+  self: grad * (-self * self + 1).rsqrt()
 
 - name: atan(Tensor self)
-  self: grad * (self * self + 1).reciprocal()
+  self: grad / (self * self + 1)
 
 - name: atan2(Tensor self, Tensor other)
   self, other: atan2_backward(grad, self, other, grad_input_mask)
@@ -192,7 +192,7 @@
 
 - name: div(Tensor self, Tensor other)
   self: grad / other
-  other: -grad * self / (other * other)
+  other: -grad * result / other
 
 - name: dot(Tensor self, Tensor tensor)
   self: grad * tensor
@@ -325,7 +325,7 @@
 - name: lgamma(Tensor self)
   self: not_implemented("lgamma")
 
-- name: linspace(Scalar start, Scalar end, int64_t steps)
+#linspace
 
 - name: log(Tensor self)
   self: grad.div(self)
@@ -425,7 +425,7 @@
   self: norm_backward(grad, self, p, result)
 
 - name: norm(Tensor self, Scalar p, int64_t dim, bool keepdim)
-  self: norm_backward(grad, self, p, destination, dim, keepdim)
+  self: norm_backward(grad, self, p, result, dim, keepdim)
 
 - name: normal(Tensor self, double mean, double std, Generator generator)
   self: zeros_like(grad)
@@ -504,7 +504,7 @@
 # range
 
 - name: reciprocal(Tensor self)
-  self: grad / -(self * self)
+  self: -grad * result * result
 
 - name: remainder(Tensor self, Scalar other)
   self: grad
@@ -558,7 +558,7 @@
   self: split_backward(grads, split_size, dim, self.sizes(), self.type())
 
 - name: sqrt(Tensor self)
-  self: grad * self.pow(-0.5) / 2
+  self: grad * self.rsqrt() * 0.5
 
 - name: squeeze(Tensor self)
   self: unsqueeze_to(grad, self.sizes());
@@ -567,10 +567,10 @@
   self: maybe_unsqueeze(grad, dim, self.size(dim) == 1 && self.sizes().size() != 1)
 
 - name: std(Tensor self, bool unbiased)
-  self: var_backward(grad * (result * 2).reciprocal(), self, unbiased)
+  self: var_backward(grad / (result * 2), self, unbiased)
 
 - name: std(Tensor self, int64_t dim, bool unbiased, bool keepdim)
-  self: var_backward(grad * (destination * 2).reciprocal(), self, dim, unbiased, keepdim)
+  self: var_backward(grad / (result * 2), self, dim, unbiased, keepdim)
 
 # storage_offset
 # stride
@@ -601,7 +601,7 @@
   self: zeros_like(self).put_(index, grad, true)
 
 - name: tan(Tensor self)
-  self: grad / self.cos().pow(2)
+  self: grad * (1 + result.pow(2))
 
 - name: tanh(Tensor self)
   self: _tanh_backward(grad, result)
diff --git a/torch/csrc/generic/methods/TensorMath.cwrap b/torch/csrc/generic/methods/TensorMath.cwrap
index ee08d24281b6..eeada6b7631b 100644
--- a/torch/csrc/generic/methods/TensorMath.cwrap
+++ b/torch/csrc/generic/methods/TensorMath.cwrap
@@ -13,7 +13,7 @@
     - method
     - function
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
 ]]
@@ -816,7 +816,7 @@
     - cname: mean
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: int64_t dim
@@ -826,7 +826,7 @@
       return: argument 0
       before_call: maybeThrowBackCompatKeepdimWarn("mean");
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: int64_t dim
@@ -856,7 +856,7 @@
     - cname: var
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: int64_t dim
@@ -870,7 +870,7 @@
       return: argument 0
       before_call: maybeThrowBackCompatKeepdimWarn("var");
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: int64_t dim
@@ -904,7 +904,7 @@
     - cname: std
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: int64_t dim
@@ -918,7 +918,7 @@
       return: argument 0
       before_call: maybeThrowBackCompatKeepdimWarn("std");
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - arg: int64_t dim
@@ -950,7 +950,7 @@
     - cname: norm
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - real p
@@ -961,7 +961,7 @@
       return: argument 0
       before_call: maybeThrowBackCompatKeepdimWarn("norm");
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - real p
@@ -984,7 +984,7 @@
     - cname: renorm
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - real p
@@ -1047,7 +1047,7 @@
     - cname: cinv
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
 ]]
@@ -1079,7 +1079,7 @@
     - cname: neg
       return: argument 0
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
 ]]
@@ -1110,7 +1110,7 @@
   cname: atan2
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - arg: THTensor* self
       broadcast: other fallback
@@ -1151,20 +1151,20 @@
   options:
     - cname: pow
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - THTensor* self
         - real exponent
     - cname: cpow
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - arg: THTensor* self
           broadcast: exponent fallback
         - THTensor* exponent
     - cname: tpow
       arguments:
-        - arg: THTensor* destination
+        - arg: THTensor* result
           output: True
         - real base
         - THTensor* self
@@ -1206,7 +1206,7 @@
   return: argument 0
   cname: lerp
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - arg: THTensor* self
       broadcast: end fallback
@@ -1282,28 +1282,28 @@
   return: argument 0
   options:
     - arguments:
-      - arg: THTensor* destination
+      - arg: THTensor* result
         output: True
       - THTensor* self
       - CONSTANT 100
       - CONSTANT 0
       - CONSTANT 0
     - arguments:
-      - arg: THTensor* destination
+      - arg: THTensor* result
         output: True
       - THTensor* self
       - int64_t bins
       - CONSTANT 0
       - CONSTANT 0
     - arguments:
-      - arg: THTensor* destination
+      - arg: THTensor* result
         output: True
       - THTensor* self
       - int64_t bins
       - real min
       - CONSTANT 0
     - arguments:
-      - arg: THTensor* destination
+      - arg: THTensor* result
         output: True
       - THTensor* self
       - int64_t bins
@@ -1717,7 +1717,7 @@
   options:
     - cname: clamp
       arguments:
-      - arg: THTensor* destination
+      - arg: THTensor* result
         output: True
       - THTensor* self
       - real min
@@ -1785,7 +1785,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - arg: int64_t diagonal
@@ -1810,7 +1810,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - arg: int64_t diagonal
@@ -1835,7 +1835,7 @@
     - function
   return: argument 0
   arguments:
-    - arg: THTensor* destination
+    - arg: THTensor* result
       output: True
     - THTensor* self
     - THTensor* other


Code review comments:
diff_hunk: @@ -73,7 +73,7 @@
 - name: addcdiv(Tensor self, Tensor tensor1, Tensor tensor2, *, Scalar value)
   self: grad
   tensor1: grad * value / tensor2
-  tensor2: -grad * value * tensor1 / (tensor2 * tensor2)
+  tensor2: grad * (self - result) / tensor2
path: tools/autograd/derivatives.yaml
text: @user1:
Isn't this going to end up with the same problems as `addcmul`?

@author:
I don't think so. `addcmul` is `self + value * tensor1 * tensor2`. Derivatives would be:
- `self` => `grad`
- `tensor1` => `grad * value * tensor2` (And the other way for `tensor2`). These are the best possible (I think).

@user1:
My comment was related to the gist you posted, where you demonstrated that rewriting the derivative in terms of other operations could yield weird sizes with `addcmul`. I'm wondering if the same thing can happen in here

@author:
It was with `addcdiv`. All the builds seemed to have failed. Could this be a reason? I tried inferring something from the logs, but no avail.