disco
disco copied to clipboard
Signaling server mixes clients and models across tasks in DeAI
Describe the bug The decentralized router has a signaling server that is used for managing all communication. The same instance of signalingServer is used for all decentralized tasks that are running at any given time, but this is not accounted for in our implementation. For instance, nothing is stopping the signalingServer from telling clients to aggregate with each other, even if the clients have no task in common.
To Reproduce We noticed this in the code, not by using DeAI. Maybe you could show this by creating multiple decentralized learning tasks, creating different clients for them, and observing who aggregates with who...
Expected behavior The signalingServer should keep a separate readyClientsBuffer for each task, and the task ID should be included in the relevant messages between clients and server.
Solution
Use readyClientsBuffers : Map<taskID,Set<PeerID>>
instead of readyClientsBuffer : PeerID[]
and adapt the messages where relevant.
Now implementing readyClientsBuffers : Map<taskID,Set<PeerID>>
as readyClientsBuffers : Map<taskID,PeerID[]>
because in line 34 of discojs/src/client/decentralized/clearText
and 58 of discojs/src/client/decentralized/secAgg
, we are unable to call the different methods of the immutable library Set() or List() objects
I remember this problem. If we don't manage to use Set, then we must prevent the PeerID[]
from containing duplicate values!
this is probably already done now with the network refactoring? @morganridel @tharvik @s314cy will know...
@tharvik confirmed his implementation passively fixed the issue :)