Repository: OpenMined/PySyft
Pull request: Fix the placeholder serialization.
Fixes #3214. 
The problem was that when reconstructing the network placeholders, the is_wrapper parameter from tensor was ignored.
The apparent issue with parameter serialization was due to the fact that when fetching a plan the wrapper parameter was incorrectly reconstructed as a plain parameter. This is actually done inside the placeholder deserialization function, where the "is_wrapper" field is stripped from the parameter tensors inside the instantiate method, hence the missing "Wrapper" when printing the deserialized network's parameters.

Problem statement:
Fix parameter serialization
In some situations, parameters are not serialized properly. I suspect this is due to our implementation of parameter.data

Here is one example:
```python
class Net(sy.Plan):
    def __init__(self):
        super(Net, self).__init__()
        self.fc1 = nn.Linear(1, 1)

    def forward(self, x):
        return self.fc1(x)

plan = Net()
plan.build(th.tensor([1.2]))

x = th.tensor([-1.0])
expected = plan(x)

plan.fix_precision().share(alice, bob, crypto_provider=charlie)
print(plan.state.tensors())
ptr_plan = plan.send(james)

# Fetch plan
fetched_plan = plan.owner.fetch_plan(ptr_plan.id_at_location, james)
print('***')
print(fetched_plan.state.tensors())
```
Output
```
[Parameter containing:
(Wrapper)>FixedPrecisionTensor>[AdditiveSharingTensor]
	-> [PointerTensor | me:94226517866 -> alice:74685210613]
	-> [PointerTensor | me:30028513485 -> bob:91228892047]
	*crypto provider: charlie*, Parameter containing:
(Wrapper)>FixedPrecisionTensor>[AdditiveSharingTensor]
	-> [PointerTensor | me:16955185561 -> alice:5015164314]
	-> [PointerTensor | me:77573712688 -> bob:21883177159]
	*crypto provider: charlie*]
***
[FixedPrecisionTensor>[AdditiveSharingTensor]
	-> [PointerTensor | me:94226517866 -> alice:74685210613]
	-> [PointerTensor | me:30028513485 -> bob:91228892047]
	*crypto provider: charlie*, FixedPrecisionTensor>[AdditiveSharingTensor]
	-> [PointerTensor | me:16955185561 -> alice:5015164314]
	-> [PointerTensor | me:77573712688 -> bob:21883177159]
	*crypto provider: charlie*]
```

Hints from issue discussion:
I can take a look

Commit message: Update comment to be accurate

Patch to review:
diff --git a/syft/execution/placeholder.py b/syft/execution/placeholder.py
index 7986dc508fe..964b6e4600f 100644
--- a/syft/execution/placeholder.py
+++ b/syft/execution/placeholder.py
@@ -30,12 +30,10 @@ def instantiate(self, tensor):
         Add a tensor as a child attribute. All operations on the placeholder will be also
         executed on this child tensor.
 
-        We remove wrappers or Placeholders if is there are any.
+        We remove Placeholders if is there are any.
         """
         if isinstance(tensor, PlaceHolder):
             self.child = tensor.child
-        elif tensor.is_wrapper:
-            self.instantiate(tensor.child)
         else:
             self.child = tensor
         return self
diff --git a/test/execution/test_plan.py b/test/execution/test_plan.py
index cfa82320b04..705cfee3db5 100644
--- a/test/execution/test_plan.py
+++ b/test/execution/test_plan.py
@@ -640,6 +640,7 @@ def forward(self, x):
     # Compare with local plan
     assert th.all(decrypted - expected.detach() < 1e-2)
     # assert fetched_plan.state.state_placeholders != plan.state.state_placeholders #TODO
+    assert str(fetched_plan.state.tensors()) == str(plan.state.tensors())
 
     # Make sure fetched_plan is using the readable_plan
     assert fetched_plan.forward is None


Code review comments:
diff_hunk: @@ -640,6 +640,7 @@ def forward(self, x):
     # Compare with local plan
     assert th.all(decrypted - expected.detach() < 1e-2)
     # assert fetched_plan.state.state_placeholders != plan.state.state_placeholders #TODO
+    assert str(fetched_plan.state.tensors()) == str(plan.state.tensors())
path: test/execution/test_plan.py
text: @user1:
Do we have to compare string representations here? Could we compare the state tensors directly?