split/rework clients

[tharvik]

there is currently a mix between a Client communicating with the server and Clients training on a given task.

• [ ] create a new dedicated ServerClient which can list tasks and upon joining one returns a specific DecentralizedClient or FederatedClient
  • allow for clearer path on where the model is created
  • allow to have a varied set of Task as this would be the only way to get theses (see #647)
  • separation of concerns, what else
• [ ] DecentralizedClient and FederatedClient would be split in multiple clients
  • [ ] {Decentralized,Federated}TrainingClient which only do the training
  • [ ] SecureAggregatorClient for secure aggregation (see #722)
  • allows for more "local" messaging and general clearer names of messages/fields
• [ ] straighten up client usage by avoiding onRound* callbacks indirections
• [ ] make it dumber
  • should not wrap an Aggregator
  • simply expose network calls, such as sendLocalWeights(weights: WeightsContainer) & receiveNetworkWeights*(): AsyncGenerator<WeightsContainer>

[JulienVig]

I think this would create a lot of Client classes with non-self-explanatory relationships. It is unclear if the word client refers to a federated user (usually called a client) or how it's currently used, which is an object only handling communication with server/peers.

Additionally, upon onboarding disco I found it very unintuitive that the Client class represented only the communication logic while I was expecting it to represent a whole user/peer partaking in distributed learning (which is the Disco object)

Some raw ideas to remove/clarify the use of "client":

ServerClient -> TaskUserBuilder

TaskUserBuilder would returns a DiscoUser (equivalent to teh current Disco or the {Decentralized,Federated}Client you are suggesting if I understood correctly).

{Decentralized,Federated}Client -> DecentralizedPeer and FederatedClient

In order to use respective naming conventions and clarify the meaning of Client. Both inherent DiscoUser.

{Decentralized,Federated}TrainingClient -> {Decentralized,Federated}Trainer

Remove the use of client. A DecentralizedPeer has a DecentralizedTrainer while a FederatedClient has a FederatedTrainer. I think these classes should handle the collaborative weight sharing schemes.

Current Client -> NetworkHandler

Simplify the current Client objects handling communication and distribution logic to only handle network communication. Can be subclassed into FederatedNetworkHandler and DecentralizedNetworkHandler. As mentioned above, the collaborative weight update logic should be handled in the {Decentralized,Federated}Trainer and not in this one (as it currently is)

Sorry I got way too involved in this, let me know what you think. It may be easier to have a call about it.