Add FedNova baseline #2179
Merged
Add FedNova baseline #2179
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jafermarq
added
the
summer-of-reproducibility
|
Hi @ashdtu , how is it going? Let me know if you'd like to discuss something about your |
ashdtu
requested review from
jafermarq,
tanertopal and
danieljanes
as code owners
|
Hi @jafermarq I have a working version of the baseline now. that is producing results similar to the one in the paper. However there are certain parts that I wanted to refactor in the code that will be a work in progress in the coming week. Request you to take a look at the current state so that we can have a 1-on-1 soon. |
jafermarq
reviewed
Oct 2, 2023
There was a problem hiding this comment.
Hi @ashdtu,
I have done an initial pass. See some suggestions below. The summary of my comments are:
- (important) please rename your directory (and its subdirectory) from
fedNovatofednova. Then don't forget to reflect that in thepyproject.toml(see comment), and update all the imports so they make use offednova.<your file>. I was able to run your code after doing it by doingpython -m fednova.main. - Try to have a minimal version of the readme ready.
- If you make plots, please add them to a directory called
_staticand placed in the same directory as theREADME.md. - Does
state/init_params_torch.ptneed to be part of the repo? (it's a large file)
Please ping me when you are done making the changes. Happy to discuss them further over here or slack.
2. add logic and refactor ProxSGD and strategy.py to update state parameters in strategy instead of client optimizer 3. get rid of logic to save optimizer params on disk
2. Generate Plots in static
2. add plots in static 3. add temporary results csv
2. utils now uses hydra created directories 3. plotting support more than 2 plots at once and logic for automatic legends 4. results for momentum experiment 5. readme polished with nan update. 6. strategy.py now contains more comments and references.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FedNova
Proposal for implementation of FedNova baseline to Flower framework under Summer of Reproducibility challenge.