Pysyft: plan.send() implementation is not intuitive

[mari-linhares]

Currently, the send operation for plans is lazy, which in practice means that the plan is not actually sent when you call send.

This behavior is confusing and different from the expected behavior when sending tensors.

Here is an example of a unintended effect that can be very confusing :confused: : https://github.com/OpenMined/PySyft/blob/dev/test/workers/test_plan.py#L137

[LaRiffle]

You're right this is a real problem. The lazy behaviour is because we need to build the plan prior to sending it, so we wait until it is executed on some data to sent it, otherwise we should run it on mock data which would very un practical.

One thing easy would be to change the name .send() to something else so that there would be no confusion, like .delegate()
Another possibility would be that A send a plan empty to B and B fills it when the sender A executes it with some pointers to B, with some mechanism to create. If C request plan execution to B before it is built it would raise some error.

[jvmncs]

Just to chime in an opinion without much context -- the ability to take code that's executed dynamically (i.e. without a plan) and turn it into code that's compiled statically (i.e. with a plan) should require as little modification as possible, since I imagine the intended workflow would be to develop code dynamically (for intuition) in preparation to deploy it statically (for speed). There are past issues where we explored various APIs for this, although they may have never left the Grid project.

[jvmncs]

One of those APIs was a context manager that compiles code that's sent and executes when you leave it -- could be an interesting option to investigate

[mari-linhares]

I feel that we're trying to hard to make the tensor interface work for the plan interface (pointers, search, send, ...) when in practice it doesn't bring that many benefits as we expect.

Having said that I think the first option (send -> delegate (or something else)) could be a cleaner and easier to follow interface than actually trying to explain how send works for plans.

[jvmncs]

I think what I meant is that it would be nice if there was minimal thought for a user when interacting with this abstraction. With a context manager, one could take code that runs in the normal way (i.e. interactively sending tensors/RPCs), wrap it with a single line, and all of a sudden have it execute as a plan. So a user can focus on the logic of their code without worrying about batching things up, and then the jump from that to something more optimized is a one-step thought.

What might make a context manager useful at addressing the original comment is that lazy methods can be handled in a way that seems non-lazy. I'm a bit far removed from the codebase so I'm not sure what this means in practice, but I hope this makes sense conceptually.

[iamtrask]

Context manager seems to have some critical UX limitations here. We need the ability to explicitly send plans, serialize plans, store plans, and search for plans. I don't see how a context manager would enable this. A context manager could be swapped out for the decorator we're currently using - but this seems tangential to the plan architecture decisions we're talking about.

[iamtrask]

Note - I think it's quite likely that the Async / Futures framework will be able to do everything Plans intend to do - although the code required for it is considerably more complex (as is the UX). Plans offer a simpler, more intuitive interface:

1) take some code
2) wrap it in a function (With a @plan decorator)
3) send that code wherever you want
4) execute it whenever you want using a simple function.

[iamtrask]

@mari-linhares - it seems like we should either ditch .send() altogether OR make .send() not lazy (and as a result have to run mock data through it - which despite typing concerns is actually default behavior in both Tensorflow Eager and PyTorch's tracing mechanisms)