Pysyft: flatten() removes the object!

[abogaziah]

Describe the bug
when .flatten() is called on AST or a remote torch tensor it removes the original tensor from the worker
To Reproduce
A= torch.rand(4)
a=A.fix_prec().share(bob, alice, crypto_provider=james) #or A.send(bob)
b=a.flatten()
a.get()

note: this happens only with 1D tensor, 2D works fine

[abogaziah]

@Syzygianinfern0 is this related to your recent GC fixes?

[MaksymPetyak]

Hey,
I had a look at this, and I think the issue is that in torch the .flatten operation on 1d array is actually an _inplace_ operation. You can do the above example with .view(-1) and it will work fine.
I have fixed the issue locally by modifying the is_inplace_method in torch_attributes.py

        try:
            return self.inplace_methods[method_name]
        except KeyError:
            is_inplace = True if re.search(self._inplace_pattern, method_name) else False
            is_inplace = True if method_name == "flatten" else is_inplace # This would fix problem with flatten
            self.inplace_methods[method_name] = is_inplace
            return is_inplace

But note that this would only work for 1d array.
Not entirely sure what would be a good fix to this, as I don't think we use the input shape in anyway to decide if the operation is inplace.

[Syzygianinfern0]

@Syzygianinfern0 is this related to your recent GC fixes?

Seems to be the case (confirmed from a git bisect). Although don't know if I can look into it right away.

.flatten operation on 1d array is actually an _inplace_ operation

@MaksymPetyak I had tried the same thing earlier today but that didn't seem to be solving the issue. Can you confirm if that is the only change required with a recent copy of the codebase.

[Syzygianinfern0]

operation ... is actually an inplace operation

The number of such edge cases that might be hidden for pretty much every method we have scares me now
:open_mouth:

[abogaziah]

Hey @MaksymPetyak , would you make a pull request with your changes?

[Syzygianinfern0]

b=a.flatten()
a.get()

After some discussion with @MaksymPetyak it's important to be noted that if anyone is working on this, don't use a.get() to decide if the pointer exists or not. It is easily misinterpretable that you have a solution if you've managed to make this work. This is since, if made into an inplace operation, it just creates a to be a duplicate of b and hence it pretends that the tensor exists :thinking:

Use this instead

import torch as th
import syft as sy

hook = sy.TorchHook(th)
alice = sy.VirtualWorker(hook, id="alice")
bob = sy.VirtualWorker(hook, id="bob")
crypto_provider = sy.VirtualWorker(hook, id="james")

torch = th
syft = sy

# a = torch.ones(1, 5)  # <<<Toggle between this (works as expected)
a = torch.rand(4)  # <<< and this (broken)
a = a.encrypt(workers=[alice, bob], crypto_provider=crypto_provider)

print(f"Before: {len(alice._tensors)}")  # 1 (expected: 1)
b = a.flatten()
print(f"After: {len(alice._tensors)}")  # 1 (expected: 2)

[github-actions[bot]]

This issue has been marked stale because it has been open 30 days with no activity. Leave a comment or remove the stale label to unmark it. Otherwise, this will be closed in 7 days.