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

refactor(baselines) Upgrade FedProx Baselne to new flwr format #4937

Merged
merged 9 commits into from
Feb 25, 2025

Conversation

maddox-j
Copy link
Contributor

Issue

Description

This PR upgrades the existing FedProx baseline from the previous flwr format to the new version.
Specifically:

  1. Removed hydra library dependency
  2. Adapted existing configuration files into pyproject.toml format
  3. Use of Flower ClientApp and ServerApp
  4. Populate dataset.py file to use flwr-datasets with DistributionPartitioner instead of manual handling.
  5. Saving of results to disk by overloading the Server class definition.
  6. Update README.md with steps to run baseline simulation through flwr run .

Related issues/PRs

N/A

Proposal

Explanation

This makes the FedProx baseline consistent with the current flwr version.

Checklist

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

Any other comments?

N/A

Copy link
Contributor

@jafermarq jafermarq left a comment

Choose a reason for hiding this comment

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

Hey @maddox-j , this looks pretty good!! I added a few minor comments below.

I was able to run all the experiments running your script but then it seems the notebook ins't creating the plots. I believe some parts need (maybe just the process_data() function) adjustments since the results structure is different since we aren't using Hydra's --multirun anymore.

@maddox-j
Copy link
Contributor Author

@jafermarq Thanks for all the comments. I have resolved your comments in one commit so that we keep the history a bit neater.

With reference to the notebook: I did make these function changes initially. I added a fixed path to the results in the notebook now in case this was the error. :)

Copy link
Contributor

@jafermarq jafermarq left a comment

Choose a reason for hiding this comment

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

@maddox-j , great to see FedProx running via flwr run 🚀

@jafermarq jafermarq merged commit be663c7 into adap:main Feb 25, 2025
55 of 56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part: baselines Add or update baseline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants