IValue::deepcopy(GenericDict) returns incorrect result when dictionary values are Tensor views

[mbasmanova]

🐛 Describe the bug

I'm seeing that IValue::deepcopy returns incorrect result for GenericDict of Tensor views.

When calling deepcopy on a GenericDict with 5 entries, the result is a dictionary with 5 entries, where keys match the source, but all values are the same and match the value of the 'first' key in source dictionary.

Let's say original dictionary is

{
  k1: v1,
  k2: v2,
  k3: v3
}

, where v1, v2, v3 share storage, but have different storage_offsets. Then, deepcopy returns

{
  k1: v1,
  k2: v1,
  k3: v1
}

Notice that all values are now the same.

I believe this is because of the use of Tensor::is_alias_of, which return true if tensors share storage even if their storage offsets are not the same. At the end of deepcopy, there is this code:

  if (!isAliasOf(copy)) {
    memo[*this] = copy;
  }

isAliasOf contains:

    // Tensors should be compared based on internal storage
    if (this->isTensor()) {
      return isAliasOf(this->toTensor(), rhs.toTensor());
    }

which is

  static bool isAliasOf(const at::Tensor& a, const at::Tensor& b) {
    if (a.is_sparse()) {
      return isAliasOf(a._values(), b) || isAliasOf(a._indices(), b);
    }
    if (b.is_sparse()) {
      return isAliasOf(a, b._values()) || isAliasOf(a, b._indices());
    }
    if (a.is_sparse_csr()) {
      return isAliasOf(a.values(), b) || isAliasOf(a.crow_indices(), b) ||
          isAliasOf(a.col_indices(), b);
    }
    if (b.is_sparse_csr()) {
      return isAliasOf(a, b.values()) || isAliasOf(a, b.crow_indices()) ||
          isAliasOf(a, b.col_indices());
    }

    // Opaque tensors such as the ones constructed by the MKL-DNN backend
    // don't have storage so we just compare their TensorImpls.
    // TODO: Find way to expose alias info for opaque tensors.
    if (!a.has_storage() || !b.has_storage()) {
      return a.unsafeGetTensorImpl() == b.unsafeGetTensorImpl();
    }

    return a.is_alias_of(b);
  }

Versions

latest

cc @ezyang @gchanan @zou3519 @kadeng @msaroufim @jbschlosser

[ezyang]

NB: this is an internal request

[ezyang]

The internal users have fixed this, but this is still egregiously wrong and we should fix it.

[ezyang]

#126089 and @davidberard98 is going to finish it