Train Collaboratively raises error during training

[JulienVig]

From a localhost web-client, uploading titanic data and clicking on Train Collaboratively raises recurrent errors (inspect page and check the console logs)

...
> Adding contribution from node 410578ec-1b93-425c-ab2f-56e33800b657 for round (0, 2)
********************************************************************************
Buffer is full. Aggregating weights for round (0, 2)

Error: timeout
    at Timeout._onTimeout (/Users/julien/Documents/DISCO/repo/discojs/discojs-node/dist/core/client/utils.js:13:64)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)` 
...

Additionally, the training doesn't seem to ever stop.

[JulienVig]

@peacefulotter pointed at a potential cause in storeAggregationResult and Promise.race([promisedResult, client.utils.timeout()]) in the federated server implementation (server/src/router/federated/server.ts).
It seems that clients and server are both stuck waiting on each other's promise to returns, and systematically results in timeouts.

[peacefulotter]

I think a way to fix this might be to remove the await result in storeAggregationResult. The await is already taken care by the Promise.race, having 2 awaits leads the storeAggregationResult to be one step ahead: first it awaits the results the server wants to respond with, then immediately skips it and turn the result into a new promise, finally the ws receives a message, and the callback receives the new promise and not the old result (which is what it wants).

[JulienVig]

Here is the sequence of event that was expected to happen:

1. Client sends local weights to server
2. Server receives weights
3. Server adds weights to aggregation
4. Client asks for new weights
5. Server receives query and puts client on hold (via a promise on aggregated weights)
6. Once enough contributions are received, the server aggregates them
7. The promise from 5. resolves and sends the aggregated weights to the clients

However, in some cases such as when there is a single client, step 6 happens before step 4, i.e., the server aggregated the weights and moved to the next round before even receiving the client's query 4.
Therefore, when the server actually receives the query, it has already moved to the next round and is now waiting for client's contributions. As a result, the server timeouts while waiting for client's contributions, who also timeouts while waiting for the server's new weights.

There are multiple ways to solve the issue. I thought the best one was to directly return the new weights whenever a client sent their contributions, thus skipping step 4. and 5.

In practice, we first create a promise waiting on the aggregated weights before adding the local contribution to make sure that the server doesn't move to the next round before resolving the promise (which as it resolves send the aggregated weights to the client).

Here is the new sequence of events:

1. Client sends local weights to server
2. Server receives weights
3. Server puts client on hold (via a promise on aggregated weights)
4. Server adds the weights to the aggregation
5. Once enough contributions are received, the server aggregates them
6. The promise from 3. resolves and sends the aggregated weights to the clients

[peacefulotter]

So if I understand this correctly, the clients do not explicitly ask for the aggregated weights since it's assumed that they will eventually receive them after sending theirs?