-
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
aggregator rework #722
Comments
sounds good, just a quick comment that ah and the robust aggregator currently inactive, did it use median, or mean after clipping? |
yeah, we just need to be more descriptive with the names, "SecureAggregator" is not really clear, smth like
I think that it is using mean of the centered weights. (not very clear to me what a centered weights but I guess it's taken from the paper itself) |
WDYT of having a separate aggregator (or whatever may replace it) dedicated to the federated clients? Since they are always expecting a single update from the server and simply overwrite their local weights with the global ones, I don't think it makes sense to use a mean aggregator the way it currently is. |
yeah, that clearly make sense. I won't even be called "aggregator" on the federated client side, it will simply be the way we communicate weights to the server. |
we currently have two aggregators
it is quite hard to follow where and how the aggregated weights are generated.
Client
Aggregator.add
), not externallyThe text was updated successfully, but these errors were encountered: