-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Let users join federated training mid session #741
Conversation
BTW for delayed updates during training, the easiest way (and algorithmically valid) is to define a maximum threshold delay, so a time delta (in number of steps/rounds): if an update arrives which was computed based on a model older than this delta, the update is dropped. otherwise it's included. the aggregation is simply triggered once the number of present valid update candidates (buffer size) is sufficient (this part is already implemented). that would work well both in decentralized and federated |
Yes! This should already be implemented, although current defaults always set the time delta to 0. We can probably increase the cutoff to 1 or 2. |
dce0d20
to
5a9ce14
Compare
Sorry for the ugly commit history I had to debug a test case that failed only in the github actions but passed locally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouhou, great work, that's a very neat fix for an important feature. thanks for tests also, very useful to avoid breaking subtle status changes 🎉
a few beautifulization comments, nothing really important
impressive work, big thanks! |
Fixes the federated case of #718
Refactoring:
server/router
intoroutes
andcontrollers
as commonly done in Express.routes
setup the router API andcontrollers
handle the actual federated and decentralized logic (viainitTask
andhandle
)client/client.ts
,client/federated_client.ts
,client/decentralized_client.ts
)