Open tharvik opened 1 month ago
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.
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.
there is currently a mix between a
Client
communicating with the server andClient
s training on a given task.ServerClient
which can list tasks and upon joining one returns a specificDecentralizedClient
orFederatedClient
Task
as this would be the only way to get theses (see #647)DecentralizedClient
andFederatedClient
would be split in multiple clients{Decentralized,Federated}TrainingClient
which only do the trainingSecureAggregatorClient
for secure aggregation (see #722)onRound*
callbacks indirectionsAggregator
sendLocalWeights(weights: WeightsContainer)
&receiveNetworkWeights*(): AsyncGenerator<WeightsContainer>