-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Description
🐛 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