Repository: pytorch
Issue: Stop passing inplace/out arguments as (non-const) Tensor& to functions
Labels: module: internals,module: cpp,triaged,enhancement

Currently, when you define an inplace function or a function that writes to an output function, you get a non-const reference. E.g.,

```
Tensor& add_out(Tensor& result, const Tensor& self, const Tensor& other);
```

The use of a non-const reference here is highly misleading, and can lead newbies down the wrong path when writing implementations of these functions. We should instead pass these arguments as ordinary `const Tensor&` references.

**Detailed discussion.**

Consider the function above.  Assuming that `add` is implemented for tensors, is the following a valid implementation of the function?

```
Tensor& add_out(Tensor& result, const Tensor& self, const Tensor& other) {
  result = self.add(other);
  return result;
}
```

C++ will not give any error when you do this, but the function will not behave correctly when used from Python:

```
x = torch.randn(N)
y = torch.randn(N)
out = torch.zeros(N)
out2 = out # refers to the same tensor
add_out(x, y, out=out2)
print(out) # out is all zeros?!
```

The intention of the `add_out` function was to write the result of `x + y` into the preallocated memory of `zeros`. `out2` and `out` alias the same memory, so modifications to out2 should be reflected in out.

The reason for this is that when you wrote `result = self.add(other);`, you did not actually change the allocated memory of out, because your implementation was semantically equivalent to this Python code:

```
x = torch.randn(N)
y = torch.randn(N)
out = torch.zeros(N)
out2 = out
out2 = x.add(y)  # the code you wrote
print(out) # well of course this is zeros
```

Yes, you modified the C++ reference, but C++ references are meaningless in Python (there is no such thing as C++ style references in Python). You need to modify the actual *underlying memory* of the out tensor object when you implement `add_out`, and assignment over a reference doesn't do that; all it does is change the `Tensor` *pointer* that was passed in. Other aliases to the same memory will not see the difference.

**What should you do instead?**

If the final operation you do is call another function, see if you can call it's inplace/out variant instead. For example, you could implement `sub_out` as:

```
Tensor& sub_out(Tensor& result, const Tensor& self, const Tensor& other) {
  return add_out(result, self, -other);
}
```

If you are actually writing a kernel (e.g., you're writing code that writes into memory locations in a loop), simply make sure that result is the right size, and just write your results directly into it (rather than allocate a fresh buffer which you would have returned). In this case, the function is typically factored into an `_out` variant that doesn't allocate, and a regular, functional variant that simply calls the `_out` implementation after allocating the result.  Example in aten/src/ATen/native/ReduceOps.cpp:

```
Tensor _prod(const Tensor &self, int64_t dim_, bool keepdim) {
  int64_t dim = maybe_wrap_dim(dim_, self.dim());
  Tensor result = self.type().tensor();
  return at::_prod_out(result, self, dim, keepdim);
}

Tensor& prod_out(Tensor& result, const Tensor& self, int64_t dim, bool keepdim) {
  // code that writes the result into result
}
```

If you're writing some code that calls into an external library, and it insists on giving you a fresh tensor, you can always smooth over the problem by doing a copy. So, to fix our add_out example from the beginning:

```
Tensor& add_out(Tensor& result, const Tensor& self, const Tensor& other) {
  result.resize_(self.sizes());
  result.copy_(self.add(other));
}
```

Another idiom is, if an inplace version of the function is available, copy the input into the eventual result buffer, and then do the inplace operation on that buffer. Example from UnaryOps.cpp

```
Tensor& _clamp_out_cpu(Tensor& result, const Tensor& self, Scalar min, Scalar max) {
  result.resize_(self.sizes());
  result.copy_(self);
  return _th_clamp_(result, min, max);
} 
```

These strategies are less efficient, so they should be used as a last resort.

cc @yf225 @glaringlee @ezyang @bhosmer @smessmer @ljk53 @bdhirsh @ailzhang