Init Crypten Plan + Rebased by gmuraru · Pull Request #3254 · OpenMined/PySyft

gmuraru · 2020-03-26T00:12:36Z

Attention Those changes are not meant to go into master, I will change the PR to target a different branch.

Rebased with master Crypten using Plans (also integrated with SyftCrypTensor).

Added the initial implementation (context setup with run_multiparty_computation and test file) from Ayoub
SyftCrypTensor will be a shell for the operations that will be done on the MPCTensor -- Integrated changes from Ajay
Traced the add and get_plain_text methods

How the tested function looks:

     @run_multiworkers([alice, bob], master_addr="127.0.0.1")
     @sy.func2plan()
     def plan_func():
         alice_tensor = crypten.load("crypten_data", 1)
         bob_tensor = crypten.load("crypten_data", 2)
 
         crypt = alice_tensor + bob_tensor
         result = crypt.get_plain_text()
         return result

How the plan looks

<Plan plan_func id:96985944591 owner:me built>
def plan_func():
    <syft.execution.placeholder_id.PlaceholderId object at 0x7f555de1f750> = crypten.load(crypten_data, 1)
    <syft.execution.placeholder_id.PlaceholderId object at 0x7f555de1f890> = crypten.load(crypten_data, 2)
    <syft.execution.placeholder_id.PlaceholderId object at 0x7f555de1f9d0> = _1.add(<syft.execution.placeholder_id.PlaceholderId object at 0x7f555de1fbd0>)
    <syft.execution.placeholder_id.PlaceholderId object at 0x7f555de1fb10> = _3.get_plain_text()
    return _4

gmuraru · 2020-03-26T09:38:40Z

syft/frameworks/crypten/__init__.py

+    if src == comm.get().get_rank():
+        # Means the data is on one of our local workers
+
+        worker = syft.local_worker.get_worker_from_rank(src)


@youben11 I changed the load to look after the worker given a rank (the PR I merged into the CrypTen branch for plans did not have those changes and it fails when it looks for the tagged data on the syft.local_worker)

youben11 · 2020-03-26T10:08:09Z

pip-dep/requirements.txt

 tornado==4.5.3
 websocket_client~=0.57.0
 websockets~=8.1.0
+git+https://github.com/facebookresearch/CrypTen.git@e39a7aaf65436706321fe4e3fc055308c78b6b92#egg=crypten


We should keep with commit '68e0364c66df95ddbb98422fb641382c3f58734c' (or any other that don't break with pysyft) for the moment, as the last version introduces some changes that requires torch nightly as well as other functionalities we doesn't support yet.

Will try to keep track with the latest version of CrypTen (also removed the Python 3.6 Tests and TutorialTests from this PR) and if there are problems with the Torch, will go back to a more stable version

youben11 · 2020-03-26T10:20:13Z

syft/__init__.py

+if dependency_check.crypten_available:
+    from syft.frameworks.torch.tensors.crypten.syft_crypten import SyftCrypTensor
+
+    __all__.extend(["SyftCrypTensor"])


I'm not sure about this, but if SyftCrypTensor is a tensor type that its only purpose is for building plans with crypten functionalities, then should we make it available in the API?

gmuraru · 2020-03-26T20:07:37Z

syft/execution/plan.py

+        arg_ids = [
+            t.id for t in args if isinstance(t, FrameworkTensor) or isinstance(t, AbstractTensor)
+        ]
+        result_ids = [


This is needed because we return a SyftCrypTensor which is an AbstractTensor

Why is SyftCrypTensor not a FrameworkTensor? Is Crypten not a framework?

Interesting point. Maybe it is not because currenly SyftCrypTensor is just a helper to build Plans for Crypten, it doesn't serve other purpose than building plans. But if it proves to be more useful, than it should be definitely a FrameworkTensor

gmuraru · 2020-03-26T20:09:28Z

syft/execution/plan.py

                else:
-                    response = getattr(_self, cmd)(*args, **kwargs)
+                    try:
+                        response = getattr(_self, cmd)(*args, **kwargs)


Q: What is the better method to do it?
The problem is that when running the plan and we reach the get_plain_text instruction, _self is a PlaceHolder and .child is an MPCTensor. The placeholder does not have the get_plain_text method and in the current implementation I try to resolve it by checking the child

If Placeholder isn't properly forwarding to the child, we should probably fix that in Placeholder.

gmuraru · 2020-03-26T20:12:38Z

syft/frameworks/crypten/hook/hook.py

+        try:
+            response = func(*args, **kwargs)
+        except RuntimeError:
+            response = SyftCrypTensor(tensor=th.zeros([]))


Q: Tried with a simple th.zeros([]) (without explicitly specifying the shape). It seems that currently, the shape does not matter.

karlhigley · 2020-03-26T20:11:39Z

syft/execution/plan.py

+        arg_ids = [
+            t.id for t in args if isinstance(t, FrameworkTensor) or isinstance(t, AbstractTensor)
+        ]
+        result_ids = [


Why is SyftCrypTensor not a FrameworkTensor? Is Crypten not a framework?

karlhigley · 2020-03-26T20:12:35Z

syft/execution/plan.py

                else:
-                    response = getattr(_self, cmd)(*args, **kwargs)
+                    try:
+                        response = getattr(_self, cmd)(*args, **kwargs)


If Placeholder isn't properly forwarding to the child, we should probably fix that in Placeholder.

karlhigley · 2020-03-26T20:14:22Z

syft/execution/plan.py

 from syft_proto.execution.v1.computation_action_pb2 import ComputationAction as ComputationActionPB

+if dependency_check.crypten_available:
+    import crypten


Plans shouldn't know anything about specific frameworks, so we should avoid adding Crypten-specific code if we can help it. (@OpenMined/syft-core-team has an open issue to remove the PyTorch-specific code.)

karlhigley · 2020-03-26T20:24:30Z

syft/messaging/message.py

        )
+
+
+class CryptenInit(Message):


This seems like the kind of thing PySyft has left to be added on top by other libraries (like Grid.) I'm not opposed to having this functionality in PySyft, but the inconsistency strikes me as pretty weird and a sign that there may be a design issue here.

Sorry I didn't get the meaning of "This seems like the kind of thing PySyft has left to be added on top by other libraries"
What would you think would be the best way to proceed here?

karlhigley · 2020-03-26T20:25:52Z

syft/workers/base.py

 from syft.messaging.message import SearchMessage
 from syft.workers.abstract import AbstractWorker

+from syft.frameworks.crypten import run_party


BaseWorker intentionally doesn't know anything about frameworks, so would like to find a way to avoid letting that knowledge leak in. One possibility would be to create a CryptenWorker that overrides or adds to the message dispatch table.

Good idea :)

Alternatively, how bad would that be to put the import in the def run_crypten_party ?
I've already seen things like this but I don't know how evil it is

I think the problem is that we are exposing some methods that are not necessarily needed by the VirtualWorker (@karlhigley this is the reason?)

karlhigley · 2020-03-26T20:27:56Z

.github/workflows/tutorials.yml

      max-parallel: 4
      matrix:
-        python-version: [3.6, 3.7]
+        python-version: [3.7]


This implies we'd be removing support for Python 3.6 entirely. @iamtrask, thoughts on that?

If we want to keep 3.6 support for PySyft (but not Crypten), possible we could split the workflows into one for each Python version and exclude the Crypten tests in the 3.6 workflow.

Sorry, this should target a separate branch -- the changes are like this because the latest CrypTen master has support for python >= 3.7

I'd really ❤️ for it to target the main branch. We want Crypten support, it's just a matter of figuring out how to get it integrated without upending the rest of the apple cart. We're squeezed on one side by Tensorflow (<= 3.7) and on the other side by Crypten. We really could narrow PySyft to 3.7 support only, just want to make sure that isn't going to break something else.

Yes - I agree that we should be targeting the main branch. Having multiple separate version of PySyft is really a last resort nuclear option.

Or put another way - if we're worried about how PySyft master and Crypten changes will "fit together" - the answer is to merge small changes earlier not even bigger changes later (by having even more divergence through working with multiple branches)

We have currently two blocking issues to merge this into master, the first is this python 3.6 support, the second is that crypten is now using torch nightly, and it will actually breaks badly if ran with torch==1.4.0

What do you think should be the resolution here?
I am thinking that will be a good idea to test with an older version of Crypten (that had support for pytorch 1.4) and target the Syft master? The downside of this is that we are not up-to-date with the Crypten changes.

Changed the target to master

FWIW we've retargeted to a crypten branch after lots of discussion becuse we need torch nightly
We'll make sure this Openmined:crypten branch is updated regularly

gmuraru · 2020-03-26T20:28:39Z

syft/execution/placeholder.py

        if isinstance(tensor, PlaceHolder):
            self.child = tensor.child
-        elif tensor.is_wrapper:
+        elif hasattr(tensor, "is_wrapper") and tensor.is_wrapper:


This is needed because after we built a plan the tensor (received as a parameter) is a MPCTensor (does not have the attribute is_wrapper.
Another solution can be seen in a previous commit here (I kinda like more this approach)

Either way is probably fine. Using hasattr protects us against future tensor types that don't have the attribute though.

karlhigley · 2020-03-26T21:22:03Z

BTW, some of the changes I suggested above are probably things the @OpenMined/syft-core-team should tackle. I'm not trying to push all the PySyft refactoring onto the Crypten team, I just want to make sure we have a good foundation to build on that will support what Crypten needs. Where the foundation is lacking, I want to make sure that we're talking about the gaps and planning for how to resolve them.

Some things can probably be addressed in this PR (e.g. CryptenWorker), some things could be deferred for later (e.g. Placeholder method forwarding), and some things might be worthy of their own issues (e.g. establishing groups of communicating workers in a consistent way.) The last category especially is where I think @OpenMined/syft-core-team could help.

gmuraru · 2020-03-27T02:16:55Z

syft/execution/placeholder.py

            self.id = PlaceholderId(self.id)
        self.child = None

+    def __getattribute__(self, name):


@karlhigley moved the logic here.
We also need tome attributes from Placeholder (like the id, tags, the instantiate method, etc.).
and for the other attributes, we forward them to the child process.

Another solution for this would be to use a list of attributes that we know are called on the placeholder.
This list should be kept with the attributes like: id, tags ... (all the parameters in the constructor) + the methods we define in the Placeholder.

What would look like then:

def __getattribute__(self, name): if name in ["id", "tags", ....]: return object.__getattribute__(self, name) else: child = object.__getattribute__(self, "child") return getattr(child, name)

I think you could maybe use hasattr to check if Placeholder defines name and delegate to the child otherwise.

The suggestion above doesn't work, because hasattr calls __getattribute__. Oops!

codecov · 2020-03-27T16:53:26Z

Codecov Report

Merging #3254 into crypten will decrease coverage by 0.06%.
The diff coverage is 88.83%.

@@             Coverage Diff             @@
##           crypten    #3254      +/-   ##
===========================================
- Coverage    94.65%   94.59%   -0.05%     
===========================================
  Files          151      151              
  Lines        16240    16218      -22     
===========================================
- Hits         15370    15340      -30     
- Misses         870      878       +8

Impacted Files	Coverage Δ
syft/execution/role.py	`98.90% <ø> (+0.49%)`	⬆️
syft/execution/placeholder.py	`88.32% <57.15%> (-3.68%)`	⬇️
syft/frameworks/crypten/context.py	`81.70% <81.70%> (ø)`
syft/messaging/message.py	`90.00% <84.22%> (-0.64%)`	⬇️
syft/frameworks/crypten/hook/hook.py	`92.86% <92.86%> (ø)`
syft/workers/base.py	`91.87% <95.24%> (+0.17%)`	⬆️
syft/dependency_check.py	`88.89% <100.00%> (+1.39%)`	⬆️
syft/execution/plan.py	`93.89% <100.00%> (+3.25%)`	⬆️
syft/frameworks/torch/hook/hook.py	`94.56% <100.00%> (+0.25%)`	⬆️
...ft/frameworks/torch/tensors/interpreters/native.py	`90.74% <100.00%> (-0.21%)`	⬇️
... and 24 more

LaRiffle · 2020-03-28T11:59:32Z

syft/frameworks/torch/tensors/crypten/syft_crypten.py

+        self.tensor = new_data.child
+        return self
+
+    @tracer(method_name="add")


so do we need this one, or is the add call directly forwarded to the child?

It is forwarded to the child (in our case we do not have a child).

But, if I change in the __init__ method to have this:

self.child = tensor

Then when building the plan, the traces look like this:

-> for log in sy.hook.trace.logs: (Pdb) p sy.hook.trace.logs [(('crypten.load', None, ('crypten_data', 1), {}), SyftCrypTensor>tensor(0.)), (('crypten.load', None, ('crypten_data', 2), {}), SyftCrypTensor>tensor(0.)), (('__add__', tensor(0.), (tensor(0.),), {}), tensor(0.)), (('get_plain_text', SyftCrypTensor>tensor(0.), (), {}), SyftCrypTensor>None)] (Pdb)

And we get an error in the plan with:

ValueError: The following tensor was used but is not known in this plan: E 0.0 E Possible reasons for this can be: E - This tensor is external to the plan and should be provided using the state. See more about plan.state to fix this.``` This is because (I think) the values for the add operations are not wrapped in SyftCrypTensor. The solution (there might be another) was to trace the ```add``` operation in the ```SyftCryptensor``` class.

gmuraru · 2020-04-02T11:58:02Z

pip-dep/requirements.txt

 tornado==4.5.3
 websocket_client~=0.57.0
 websockets~=8.1.0
+git+https://github.com/facebookresearch/CrypTen.git@4ad37cb97827b3364d433251d46c75d980bc3cf6#egg=crypten


Q: Should we move this to a separate requirement file?

gmuraru · 2020-04-11T17:53:30Z

I left very few comments but I guess the main part left is that CrypTenWorker.
Regarding this, I do have a slight concern about creating workers dedicated to specific frameworks. In particular because it means you're making a choice when starting a set up on which interaction you'll allow, and the data you have and use won't be available for workers which are not from this framework.
I'd rather envision adding to workers some specialities: by default you would get a worker which is fine for pytorch but not let's say for crypten. But at any point you could say alice.add_crypten_support() or smthg like this and your worker could also support specific operations for this framework, without loosing its compatibility with pytorch workers. I don't know if this makes sense?

It makes a lot of sense @LaRiffle. Currently I want to have this PR merged and after I can add support for this. If that sounds ok? Or should I push the changes in this PR? (I am thinking that this PR becomes bigger and bigger and it might be hard to track all the changes)

Updated the PR

* Context of computation for crypten (#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

* Context of computation for crypten (OpenMined#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

* Context of computation for crypten (#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

* Context of computation for crypten (OpenMined#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

* Context of computation for crypten (#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

* Context of computation for crypten (OpenMined#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

* Context of computation for crypten (#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

… to crypten (#3411) * Init Crypten Plan + Rebased (#3254) * Context of computation for crypten (#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com> * save pytroch model to onnx and back to crypten * tests * fix test * follow master for crypten Co-authored-by: George-Cristian Muraru <murarugeorgec@gmail.com>

* Init Crypten Plan + Rebased (#3254) * Context of computation for crypten (#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com> * new message CryptenInitJail and CryptenInit -> CryptenInitPlan * choice to run jails or plans by decorating a function or a plan respectively * typos and small changes * reformatting * re-enable mpc hooking * lint * update tests * remove old comments * fix serialization CryptenInitJail and tests * lint * Update pip-dep/requirements.txt * typos Co-authored-by: George-Cristian Muraru <murarugeorgec@gmail.com>

* Context of computation for crypten (OpenMined#2963) * New message type for crypten party initialization * Handle CryptenInit message by running local party * Define a new context of computation for crypten The new context of computation can run parties that are distributed across syft workers. The communication of crypten parties remains the same, however, syft workers handle initialization and the serialization of return values. * updated docs * testing crypten context * fix: was setting the bad env variable DISTRIBUTED_BACKEND should have been set and not BACKEND * test serde of CryptenInit message * add crypten as core deps, this should change to extra * delete useless comment * fix: list CryptenInit in OBJ_SIMPLIFIER_AND_DETAILERS * fix msgpack tests * run black * don't cover empty function * Init Crypten Plan * Remove python 3.6 from github actions * Remove SyftCrypTensor as wrapper from MPCTensor * Remove SyftCrypTensor from export * Check for is_wrapper in placeholder * Add placeholder attribute * Bug Fix Message * Add serde test * Old CrypTen version * Add crypten to execution * wrap our load function around crypten.load * Update syft/frameworks/crypten/__init__.py Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Rework crypten integration for plans * linter fix * Remove duplicate test Co-Authored-By: Ayoub Benaissa <ayouben9@gmail.com> * Crypten track master * Remove SyftCryptensor as a dependency * Remove python 3.6 * Use correct commit * Add MPCTensor as FrameworkTensor and remove SyftCrypTensor * remove duplicated CrypTenInit * Review pass1 Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

gmuraru requested review from a team, LaRiffle and youben11 March 26, 2020 00:12

gmuraru commented Mar 26, 2020

View reviewed changes

gmuraru requested a review from ajnovice March 26, 2020 09:39

youben11 suggested changes Mar 26, 2020

View reviewed changes

karlhigley added the Crypto Team label Mar 26, 2020

karlhigley requested a review from a team March 26, 2020 17:51

gmuraru force-pushed the gm-new-plans branch 2 times, most recently from 33171cb to fa53d18 Compare March 26, 2020 20:04

gmuraru requested a review from youben11 March 26, 2020 20:06

gmuraru commented Mar 26, 2020

View reviewed changes

gmuraru force-pushed the gm-new-plans branch from fa53d18 to bdd78a8 Compare March 26, 2020 20:15

karlhigley previously requested changes Mar 26, 2020

View reviewed changes

gmuraru commented Mar 26, 2020

View reviewed changes

LaRiffle changed the base branch from master to crypten March 26, 2020 20:57

gmuraru mentioned this pull request Mar 27, 2020

Plans for CrypTen #3165

Closed

gmuraru commented Mar 27, 2020

View reviewed changes

gmuraru force-pushed the gm-new-plans branch from 674d0f1 to ad2c6a4 Compare March 27, 2020 16:36

LaRiffle reviewed Mar 28, 2020

View reviewed changes

gmuraru force-pushed the gm-new-plans branch 2 times, most recently from 6d62b42 to b50d282 Compare April 1, 2020 02:19

gmuraru changed the base branch from crypten to master April 2, 2020 11:56

gmuraru force-pushed the gm-new-plans branch from b50d282 to 7dda15a Compare April 2, 2020 11:57

gmuraru commented Apr 2, 2020

View reviewed changes

youben11 and others added 2 commits April 11, 2020 20:00

remove duplicated CrypTenInit

84f930c

Review pass1

0b5f5f1

gmuraru force-pushed the gm-new-plans branch from e36cf7d to 0b5f5f1 Compare April 11, 2020 17:47

gmuraru requested a review from LaRiffle April 11, 2020 17:52

gmuraru requested a review from youben11 April 11, 2020 17:53

youben11 approved these changes Apr 11, 2020

View reviewed changes

LaRiffle approved these changes Apr 13, 2020

View reviewed changes

youben11 merged commit 0bc51e9 into OpenMined:crypten Apr 13, 2020

gmuraru mentioned this pull request Apr 14, 2020

[WIP] SyftCrypTensor with Crypten Tensor (MCPTensor) functionalities #3306

Closed

gmuraru mentioned this pull request Apr 15, 2020

Add support for Crypten in worker #3363

Closed

gmuraru mentioned this pull request Apr 21, 2020

Add method to add/remove operations from base worker #3364

Merged

		)


		class CryptenInit(Message):

Uh oh!

Conversation

gmuraru commented Mar 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamtrask Mar 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karlhigley commented Mar 26, 2020

Uh oh!

gmuraru Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karlhigley Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

gmuraru commented Mar 26, 2020 •

edited

Loading

iamtrask Mar 28, 2020 •

edited

Loading

gmuraru Mar 27, 2020 •

edited

Loading

karlhigley Mar 27, 2020 •

edited

Loading

codecov bot commented Mar 27, 2020 •

edited

Loading