OpenMined / PySyft

Perform data science on data that remains in someone else's server
https://www.openmined.org/
Apache License 2.0
9.52k stars 1.99k forks source link

OpenMined.org Demo Broken #1905

Closed iamtrask closed 5 years ago

iamtrask commented 5 years ago

On our homepage - we link to colabs which implement a simple SocketWorker demo - as the new version of PySyft has no SocketWorker - the demo fails to work.

https://colab.research.google.com/drive/1-Jb_E_nDuBGHIJ_psI95k-ukh-P_aly-

kakirastern commented 5 years ago

Hi @iamtrask I would like to help out on this issue if no one has been assigned to it or has volunteered to take on the task yet. And, I have just checked the PySyft repo with a keyword search and it returns no results for SocketWorker.

LaRiffle commented 5 years ago

Yes any help is welcome is the issue is still there :) Check that you have the latest version of pysystf and then check out syft/workers/websocket* to find the workers.

kakirastern commented 5 years ago

Thanks for the prompt response and the tip to start! Sure, will work on it as soon as possible.

kakirastern commented 5 years ago

Working on the issue here: https://github.com/kakirastern/PySyft/tree/fix-broken-demo Will submit a PR when more certain about the edited tutorials. Currently fixed the server tutorial, but not the client one yet, which is more difficult as it turned out.

LaRiffle commented 5 years ago

Ok excellent, let us know if you're stuck somewhere :)

kakirastern commented 5 years ago

Thanks @LaRiffle... Incidentally when I was checking the code in Google Colab I found the syft-native variable id to be problematic, whereas the same issue does not crop up when I was testing locally.

The code I used prior to such an error is:

! rm -rf ./PySyft
! git clone https://github.com/OpenMined/PySyft.git
# http://pytorch.org/
from os import path
from wheel.pep425tags import get_abbr_impl, get_impl_ver, get_abi_tag
platform = '{}{}-{}'.format(get_abbr_impl(), get_impl_ver(), get_abi_tag())

accelerator = 'cu80' if path.exists('/opt/bin/nvidia-smi') else 'cpu'

!pip3 install https://download.pytorch.org/whl/cu100/torch-1.1.0-cp36-cp36m-linux_x86_64.whl
!pip3 install https://download.pytorch.org/whl/cu100/torchvision-0.3.0-cp36-cp36m-linux_x86_64.whl
import torch

!cd PySyft; pip3 install -r requirements.txt; pip3 install -r requirements_dev.txt; python3 setup.py install
import os
import sys

module_path = os.path.abspath(os.path.join('./PySyft'))
if module_path not in sys.path:
    sys.path.append(module_path)

Following this then I did:

import syft as sy
from syft.workers.websocket_server import WebsocketServerWorker

hook = sy.TorchHook(torch)

local_worker = WebsocketServerWorker(
                            host="localhost",
                            hook=hook,
                            id=0,
                            port=8182,
                            log_msgs=True,
                            verbose=True)

local_worker.start()  # Might need to Interupt with `control-C`

hook = sy.TorchHook(torch, local_worker=local_worker)

So the following is the error output:

  File "/content/PySyft/syft/frameworks/torch/tensors/interpreters/abstract.py", line 19
    id: int = None,
      ^
SyntaxError: invalid syntax

Any idea why and how to approach it? Is it just some simple syntax stuff as the error seems to suggest, or does it have to do with id() being a Python built-in function?

kakirastern commented 5 years ago

Or if Google Colab proves to be problematic... Would be also be worthwhile to explore alternatives say using Binder for an interactive Jupyter notebook on the Internet?: https://mybinder.readthedocs.io/en/latest/introduction.html

kakirastern commented 5 years ago

I think I was able to fix the error, seems like if I do sys.path.remove('/usr/local/lib/python3.6/dist-packages/syft-0.1.21a1-py3.6.egg') and also sys.path.append('./PySyft') I would then be able to patch or mask it. As for the id issue previously, that turned out to be specific to the Client notebook only, which for some reason is using Python 2.7 instead of Python 3.x... I will look more into it and make changes where appropriate for the notebooks to work on Colab.

kakirastern commented 5 years ago

What is the latest implementation of listen() (if any) for the WebsocketServerWorker?

kakirastern commented 5 years ago

I am investigating whether the following code would work well for WebsocketServerWorker:

def listen(self, backlog=5):
        self.socket.listen(backlog)
        logging.info("Listening on %s" % self.port)
        self.running = True
        self.cls = WebsocketServerWorker
        while self.running:
            rList, wList, xList = select(self.listeners, [], self.listeners, 1)
            for ready in rList:
                if ready == self.socket:
                    logging.debug("New client connection.")
                    client, address = self.socket.accept()
                    filenum = client.filenum()
                    self.listeners.append(filenum)
                    self.connections[filenum] = self.cls(client, self)
                else:
                    logging.debug("Client ready for reading %s." % ready)
                    client = self.connections[ready].client
                    data = client.recv(1024)
                    filenum = client.filenum()
                    if data:
                        self.connections[filenum].feed(data)
                    else:
                        logging.debug("Closing client %s." % ready)
                        self.connections[filenum].close()
                        del self.connections[filenum]
                        self.listeners.remove(ready)
            for failed in xList:
                if failed == self.socket:
                    logging.error("Socket is broken.")
                    for filenum, con in self.connections:
                        con.close()
                    self.running = False

where we have in __init__():

self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
self.listeners = [self.socket]
self.connections = {}

Any feedback for suggestions would be much appreciated.

kakirastern commented 5 years ago

Or should I separate out new classes WebSocket and WebSocketServer from WebSocketServerWorker? Would that approach work?