Add FedAvgM baseline #2246

gubertoli · 2023-08-23T14:36:49Z

Issue

Description

Related issues/PRs

Implements #2052

Proposal

Explanation

Checklist

Any other comments?

Correcting validation dataset references on FlowerClient class and min_eval_clients to be in accordance with the comments.

To be compatible with sanity check for the return of fit method from clients (Tuple of list,int,dict) introduced on the last version of flower.

…_metrics

jafermarq

Hi @gubertoli, just a couple of small comments.

baselines/fedavgm/README.md

baselines/fedavgm/pyproject.toml

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

…t Setup

Error from test-baseline: fedavgm/strategy.py:46: error: Incompatible default for argument "initial_parameters" (default has type "None", argument has type "Parameters") [assignment]

gubertoli · 2023-11-21T10:05:13Z

@jafermarq as a final touch, I am running the test-baseline.sh for FedAvgM and getting the following errors:

fedavgm/strategy.py:143: error: Argument 1 to "parameters_to_ndarrays" has incompatible type "Parameters | None"; expected "Parameters"  [arg-type]
fedavgm/strategy.py:172: error: Argument 1 to "parameters_to_ndarrays" has incompatible type "Parameters | None"; expected "Parameters"  [arg-type]

However, I inspected the strategy.py, and the definition for parameters_to_ndarrays in CustomFedAvgM is set to initial_parameters: Parameters (this custom strategy inherits from FedAvg, that uses parameters_to_ndarrays as Parameters | None").

Inspecting the code with VSCode shows this correct definition (non optional) - but different from mypy output:

The simulation is running, I am not sure about this mypy errors, have you ever faced similar issues?

jafermarq

Hi @gubertoli,

Find below the reason why you get that mypy error. Once fixed you'll see a few pylint errors, they are easy to fix but let me know if you need some help.

baselines/fedavgm/fedavgm/strategy.py

gubertoli · 2023-11-23T12:53:52Z

Hi @gubertoli,

Find below the reason why you get that mypy error. Once fixed you'll see a few pylint errors, they are easy to fix but let me know if you need some help.

@jafermarq as you said, there are still a few pylint errors:

- pylint: start
************* Module fedavgm.main
fedavgm/main.py:23:0: R0914: Too many local variables (23/15) (too-many-locals)
************* Module fedavgm.client
fedavgm/client.py:13:4: R0913: Too many arguments (7/5) (too-many-arguments)
************* Module fedavgm.utils
fedavgm/utils.py:14:0: R0914: Too many local variables (26/15) (too-many-locals)
************* Module fedavgm.models
fedavgm/models.py:7:0: E0401: Unable to import 'tensorflow.nn' (import-error)
************* Module fedavgm.server
fedavgm/server.py:13:22: W0613: Unused argument 'server_round' (unused-argument)
fedavgm/server.py:31:51: W0613: Unused argument 'config' (unused-argument)

------------------------------------------------------------------
Your code has been rated at 9.73/10 (previous run: 9.67/10, +0.05)

Is it expected to clear all of them? Because some would required a deeper review (too-many-locals / too-many-arguments), the import-error is not the case during tests, and for the unused-argument I think would be good for them to be present in the server.py part for future customization.

jafermarq · 2023-11-23T13:04:22Z

too-many-locals

Those are fine to be ingored. But you need to tell pylint to do so. You can use this syntax on the line (or above) the one with the error:

# pylint: disable=too-many-locals

etc for the other errors

gubertoli · 2023-11-23T16:42:59Z

too-many-locals
Those are fine to be ingored. But you need to tell pylint to do so. You can use this syntax on the line (or above) the one with the error:
# pylint: disable=too-many-locals
etc for the other errors

@jafermarq, thank you once again 👍
Now addressed the previous pending pylint findings.

jafermarq

Hey @gubertoli, FedAvgM looks ready. Thanks for taking part in the Flower Summer of Reproducibility!!

Fix the static method error --------- Co-authored-by: Daniel J. Beutel <daniel@flower.dev> Co-authored-by: Robert Steiner <robert@flower.dev> Update types-requests requirement from ==2.31.0.2 to ==2.31.0.10 (#2739) Updates the requirements on [types-requests](https://github.com/python/typeshed) to permit the latest version. - [Commits](https://github.com/python/typeshed/commits) --- updated-dependencies: - dependency-name: types-requests dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Taner Topal <taner@flower.dev> Make the optional arg "--callable" in `flower-client` a required positional arg. (#2673) Co-authored-by: Javier <jafermarq@users.noreply.github.com> Update jupyterlab requirement from ==4.0.8 to ==4.0.9 (#2740) Updates the requirements on [jupyterlab](https://github.com/jupyterlab/jupyterlab) to permit the latest version. - [Release notes](https://github.com/jupyterlab/jupyterlab/releases) - [Changelog](https://github.com/jupyterlab/jupyterlab/blob/@jupyterlab/lsp@4.0.9/CHANGELOG.md) - [Commits](https://github.com/jupyterlab/jupyterlab/compare/@jupyterlab/lsp@4.0.8...@jupyterlab/lsp@4.0.9) --- updated-dependencies: - dependency-name: jupyterlab dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Taner Topal <taner@flower.dev> HeteroFL baseline (#2439) Co-authored-by: jafermarq <javier@flower.dev> Retire `MXNet` examples (#2724) Co-authored-by: Taner Topal <taner@flower.dev> Rename draft release workflow and create release (#2658) Update Android manifest to include internet permission (#2672) Bump lewagon/wait-on-check-action from 1.3.1 to 1.3.3 (#2756) Bumps [lewagon/wait-on-check-action](https://github.com/lewagon/wait-on-check-action) from 1.3.1 to 1.3.3. - [Release notes](https://github.com/lewagon/wait-on-check-action/releases) - [Commits](lewagon/wait-on-check-action@v1.3.1...v1.3.3) --- updated-dependencies: - dependency-name: lewagon/wait-on-check-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Update ruff requirement from ==0.1.4 to ==0.1.9 (#2753) Updates the requirements on [ruff](https://github.com/astral-sh/ruff) to permit the latest version. - [Release notes](https://github.com/astral-sh/ruff/releases) - [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md) - [Commits](astral-sh/ruff@v0.1.4...v0.1.9) --- updated-dependencies: - dependency-name: ruff dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daniel J. Beutel <daniel@flower.dev> Add FedAvgM baseline (#2246) Co-authored-by: Daniel J. Beutel <daniel@adap.com> Co-authored-by: jafermarq <javier@flower.dev> Add dockerfile for flower client image (#2746) Add docker client image ci (#2747) Rename `workload_id` to `run_id` (#2769) Update README.md (#2771) Format code examples (#2767) Rename WorkloadState to RunState (#2770) Add in-place FedAvg (#2293) Update the error message when simulation crashes (#2759) Add tests Add tests Fix tests Fix tests

gubertoli and others added 30 commits March 6, 2022 00:42

Update sim.ipynb

92e598c

Correcting validation dataset references on FlowerClient class and min_eval_clients to be in accordance with the comments.

Merge branch 'main' into main

7d58b6f

Merge branch 'main' into main

2311465

Merge branch 'adap:main' into main

b559f88

Merge branch 'adap:main' into main

6372cc7

Merge branch 'adap:main' into main

64c7124

Change to comply with .fit() tuple requirements

0770547

To be compatible with sanity check for the return of fit method from clients (Tuple of list,int,dict) introduced on the last version of flower.

Merge branch 'main' into main

6a9076f

Merge branch 'adap:main' into main

99645ef

Merge branch 'adap:main' into main

6774045

custom metrics example

1eefe76

Merge branch 'adap:main' into main

3033b01

Merge branch 'main' of https://github.com/gubertoli/flower into extra…

4174ccc

…_metrics

fedavgm baseline

3d67013

initial

3ff6383

Update README.md

d4d2153

Update client.py

a0310b1

Update dataset_preparation.py

4f14b4d

Reference using Colab for Simulation

3e35e34

Delete FedAvgM_Baseline.ipynb

d336727

Experiments using Colab

143ea68

Update dataset_preparation.py

f0a31ae

Update client.py

8196db2

Update client.py

75d684b

Update main.py

0c11554

Update client.py

daaaaeb

Update main.py

0b5c374

Update client.py

f24254b

Update base.yaml

c55759d

Update main.py

5a34f77

jafermarq reviewed Oct 8, 2023

View reviewed changes

gubertoli and others added 9 commits October 8, 2023 10:08

Change point by comma for thousands

17acf20

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

Change point by comma for thousand

617613b

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

Change to description and authors

b67607d

Changes to FedAvgM with NAG description and restructure of Environmen…

cb3d226

…t Setup

Adding 1e-5 with 10000 rounds and note about lr and momentum values

fef2874

format-baseline and fixing some test-baseline raised issued

08082b7

fixing format

0308d31

Fixing initial_parameters definition

9ba6741

Error from test-baseline: fedavgm/strategy.py:46: error: Incompatible default for argument "initial_parameters" (default has type "None", argument has type "Parameters") [assignment]

Fix output filename definition

d09a014

jafermarq reviewed Nov 21, 2023

View reviewed changes

baselines/fedavgm/fedavgm/strategy.py Outdated Show resolved Hide resolved

baselines/fedavgm/fedavgm/strategy.py Show resolved Hide resolved

gubertoli added 4 commits November 23, 2023 13:16

Improved explanation for CustomFedAvgM strategy

7f9ec32

Added assert for initial_parameters not None on CustomFedAvgM

a4a2f27

Minor changes to improve test-baseline.sh metrics

a32a858

Fixing hydra yaml learning_rate after pylint changes

fed2ddf

pylint

3db5ece

gubertoli and others added 3 commits December 11, 2023 14:28

Reordering setup

8947482

Merge branch 'main' into fedavgm-baseline

5d12c7b

README tweaks

eb90138

jafermarq changed the title ~~FedAvgM Baseline~~ Add FedAvgM baseline Dec 28, 2023

jafermarq and others added 2 commits December 28, 2023 19:47

Merge branch 'main' into fedavgm-baseline

115383e

to changelog

c7ff344

jafermarq approved these changes Dec 28, 2023

View reviewed changes

jafermarq merged commit 1a04718 into adap:main Dec 28, 2023
27 checks passed

gubertoli deleted the fedavgm-baseline branch December 30, 2023 01:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FedAvgM baseline #2246

Add FedAvgM baseline #2246

gubertoli commented Aug 23, 2023 •

edited by jafermarq

jafermarq left a comment

gubertoli commented Nov 21, 2023

jafermarq left a comment

gubertoli commented Nov 23, 2023

jafermarq commented Nov 23, 2023

gubertoli commented Nov 23, 2023

jafermarq left a comment

Add FedAvgM baseline #2246

Add FedAvgM baseline #2246

Conversation

gubertoli commented Aug 23, 2023 • edited by jafermarq

Issue

Description

Related issues/PRs

Proposal

Explanation

Checklist

Any other comments?

jafermarq left a comment

Choose a reason for hiding this comment

gubertoli commented Nov 21, 2023

jafermarq left a comment

Choose a reason for hiding this comment

gubertoli commented Nov 23, 2023

jafermarq commented Nov 23, 2023

gubertoli commented Nov 23, 2023

jafermarq left a comment

Choose a reason for hiding this comment

gubertoli commented Aug 23, 2023 •

edited by jafermarq