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

Are the procedures of weights aggregation and clients selection correct? #10

Closed
ddayzzz opened this issue May 4, 2020 · 4 comments
Closed

Comments

@ddayzzz
Copy link

ddayzzz commented May 4, 2020

Good job. I have read the paper and codes. I have some questions:

  • Weights aggregation didn't scale the sum of each weight by 1/K in codes: fedbase.py line 115
  • As you mention in paper, the target clients are chosen by their probability (p_k). But I found the relevant codes in fedbase.py line98 just uniformly chose the clients which is violated to the FedProx pseudo codes.
    来自剪切板的图片 20200504215401

Thank you.

@litian96
Copy link
Owner

litian96 commented May 4, 2020

Thanks for your questions!

There are at least two sampling schemes we can consider in this problem: (1) sampling devices with weights proportional to the number of local data points and simply averaging the updates, and (2) sampling devices uniformly and doing weighted aggregation with weights proportional to the number of local data points. In order to perform a fair comparison with FedAvg, we use the second scheme, which is slightly different from our algorithm (see Section 5.1 -- Implementation of the paper).

Interestingly, the proposed algorithm (using the first sampling scheme) performs slightly better for both FedProx and FedAvg (see Figure 12 in Appendix C.3.4).

That's a good point. Please let me know if you have other questions!

@ddayzzz
Copy link
Author

ddayzzz commented May 7, 2020

Thanks for your questions!

There are at least two sampling schemes we can consider in this problem: (1) sampling devices with weights proportional to the number of local data points and simply averaging the updates, and (2) sampling devices uniformly and doing weighted aggregation with weights proportional to the number of local data points. In order to perform a fair comparison with FedAvg, we use the second scheme, which is slightly different from our algorithm (see Section 5.1 -- Implementation of the paper).

Interestingly, the proposed algorithm (using the first sampling scheme) performs slightly better for both FedProx and FedAvg (see Figure 12 in Appendix C.3.4).

That's a good point. Please let me know if you have other questions!

Thank you for your reply. I have implemented the schemes following your instruction. I have another question: Did you consider the system heterogeneity when compare the differences between scheme 1 and 2?

@litian96
Copy link
Owner

litian96 commented May 7, 2020

For Figure 12 in Appendix C.3.4, no. But you can always do that, just that we were concerned about the statistical efficiency of different sampling schemes, so we didn't mix it with the systems heterogeneity. And Fedprox is agnostic of the sampling schemes and can yield similar improvement under either scenario.

@ddayzzz ddayzzz closed this as completed May 10, 2020
@ddayzzz
Copy link
Author

ddayzzz commented May 10, 2020

For Figure 12 in Appendix C.3.4, no. But you can always do that, just that we were concerned about the statistical efficiency of different sampling schemes, so we didn't mix it with the systems heterogeneity. And Fedprox is agnostic of the sampling schemes and can yield similar improvement under either scenario.

Thank you! I will try it.

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

No branches or pull requests

2 participants