Skip to content
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

re-organize aggregators #299

Merged
merged 1 commit into from
Aug 8, 2022
Merged

re-organize aggregators #299

merged 1 commit into from
Aug 8, 2022

Conversation

xieyxclack
Copy link
Collaborator

As the title says.

Copy link
Collaborator

@DavdGao DavdGao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@@ -10,7 +10,7 @@ def get_aggregator(method, model=None, device=None, online=False, config=None):
from federatedscope.cross_backends import FedAvgAggregator
return FedAvgAggregator(model=model, device=device)
else:
from federatedscope.core.aggregator import ClientsAvgAggregator, \
from federatedscope.core.aggregators import ClientsAvgAggregator, \
Copy link
Collaborator

@DavdGao DavdGao Aug 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xieyxclack It seems like more than one argument will influence the type of aggregator, maybe we should determine the final type of aggregator and resolve the potential conflicts before Fedrunner.run(), so that only one argument aggregator.type is enough.

@DavdGao DavdGao merged commit ec1979c into alibaba:master Aug 8, 2022
@xieyxclack xieyxclack deleted the agg_file branch August 9, 2022 02:59
Schichael pushed a commit to Schichael/FederatedScope_thesis that referenced this pull request Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants