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

Add FedPFT baseline #3268

Merged
merged 31 commits into from
Apr 25, 2024
Merged

Add FedPFT baseline #3268

merged 31 commits into from
Apr 25, 2024

Conversation

mahdibeit
Copy link
Contributor

@mahdibeit mahdibeit commented Apr 15, 2024

Issue

Description

This PR adds FedPFT baseline as introduced in the "Parametric Feature Transfer: One-shot Federated Learning with Foundation Models" paper.

Related issues/PRs

Implements #3232

Proposal

Explanation

Implements FedPFT as a baseline.

Checklist

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

Changelog entry

Any other comments?

@jafermarq jafermarq self-assigned this Apr 16, 2024
@mahdibeit mahdibeit mentioned this pull request Apr 21, 2024
14 tasks
@jafermarq
Copy link
Contributor

Hi @mahdibeit , i did a couple of small fixes. now the test pipeline starts but you'll notice that mypy shows some errors. Let me know when you have some time to take a look.

@mahdibeit
Copy link
Contributor Author

Hi @jafermarq, thanks for taking the time to fix the issues. I noticed that the test-baseline.sh script that I ran did not do all the tests (like mypy or lint). I fixed all the issues this time and ran it successfully without errors. Let me know if it requires any further changes.

@jafermarq
Copy link
Contributor

@mahdibeit , cool. The tests pass. I'll take a closer look into the code today/tomorrow and get back to you if i see something that we could improve. In most baselines, contributors decided to add their contact details (e.g. email address) in the contributors point under the About this baseline section. Feel free to do that if you wish.

@mahdibeit
Copy link
Contributor Author

Awesome. Thank you so much @jafermarq. I look forward to your review. Thanks for your suggestion. I added my email to the README file.

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.

Hi @mahdibeit , I was able to reproduce the results you show in the plot at the end of your README.md (but needed to make the small changes i suggest in my review below). Let me know what you think.

The PR is in a pretty good state so once those small issues are resolved it can be merged 🙌

baselines/fedpft/README.md Outdated Show resolved Hide resolved
baselines/fedpft/fedpft/conf/strategy/fedavg.yaml Outdated Show resolved Hide resolved
mahdibeit and others added 2 commits April 25, 2024 13:05
update readme

Co-authored-by: Javier <jafermarq@users.noreply.github.com>
fix config arg

Co-authored-by: Javier <jafermarq@users.noreply.github.com>
@mahdibeit
Copy link
Contributor Author

Hi @jafermarq, that is great news. I merged the suggested changes. Everything looks great to me.

@jafermarq jafermarq enabled auto-merge (squash) April 25, 2024 20:37
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.

Great! FedPFT is great and really fast! 🚀

@jafermarq jafermarq disabled auto-merge April 25, 2024 20:39
@jafermarq jafermarq enabled auto-merge (squash) April 25, 2024 20:43
@jafermarq jafermarq merged commit bb434b0 into adap:main Apr 25, 2024
27 checks passed
mohammadnaseri pushed a commit that referenced this pull request May 2, 2024
Co-authored-by: jafermarq <javier@flower.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants