Add MOON baseline

[QinbinLi]

Issue

Description

Related issues/PRs

Proposal

Explanation

Checklist

• Implement proposed change
• Write tests
• Update documentation
• Update changelog
• Make CI checks pass
• Ping maintainers on Slack (channel #contributions)

Any other comments?

[jafermarq]

Hi @QinbinLi,

I did a first pass through your MOON implementation using Flower. I was able to run the code, so that's great! I have left a few comments here and there. Mostly small things.

More critical is the way you are loading prev_model in the client's fit() method. It's not doing what I think you want to do (i.e. use the model the client sent back to the server in the previous round). Please take a look at my suggestion below on how to fix it.

Less critical, but also important:

• Would it be possible to generate two plots showing fedprox vs moon? (as in Figures 6a,b in your paper)
• Please rename the directory where the figures are added to _static (this is helpful to make your README to appear automatically in the documentation).

[jafermarq]

I few extra comments i forgot to add above

[QinbinLi]

Hi @jafermarq ,

I have updated the code according to your comments. I have added the results of Figure 8(a) (https://arxiv.org/pdf/2103.16257.pdf) using MOON. The experiment using FedProx failed multiple times due to insufficient memory (others are using the same machine) and I launched another machine to run it. I need about 2 days to finish it.

Is it necessary to run Figure 8(b)? Figure 8(b) is more time-consuming than Figure 8(a).

[jafermarq]

Hi @QinbinLi, it's fine not including Fig 8(b). Ping me once the FedProx is added to your Fig8(a). If possible, could you make the figures to use larger font size (just so it's more readable in the README.md. Many thanks

[jafermarq]

Hi @QinbinLi ,

You'll notice i pushed some changes (I did just some formatting). We can start merging your baseline once you have added the FedProx curve to your Fig8a below in the readme.

I think it would be great if you could add some comments to your models.train_moon() function (for instance state which line(s) doe each line in Algorithm 1 in your paper). This would be ideal for others trying to use (maybe even modify!) your MOON baseline. I left a comment below.

Also, I'm thinking is better to remove the log files in _static. They simply take too much space. If you are able to host these somewhere else, you are welcome to add a link to it in the README.md if you want.

Finally, I believe the client_states directory needs to be erased before running a new experiment, right? I see you remove it once the experiment is completed. But, what if the experiment crashes for some reason. We probably want to erase it before starting the simuilation again, right?

Please ping me when you managed to add the final result and go through my comments.

[QinbinLi]

Hi @jafermarq ,

I have addressed your comments and pushed the new results. Thanks!

[jafermarq]

@QinbinLi, great! thanks for the update. This is ready to be merged. I'll do this early in the week.