search

gnahtb / RealFL

Implement basic steps in a model-centric federated learning model
The Unlicense
0 stars 2 forks source link

FedAvg weighted average implementation #19

Closed gnahtb closed 1 year ago

gnahtb commented 1 year ago

The current FedAvg implementation doesn't support weighted average aggregation. We need to support this. This issue possibly is dependent on #18.

https://github.com/bnnthang/RealFL/blob/c3510b7dfc32b54e74eda4207fc335fc3c82d837/Server/src/main/java/com/bnnthang/fltestbed/Server/AggregationStrategies/FedAvg.java#L12-L24

gnahtb commented 1 year ago

@tem556 Regarding this issue, I think the easiest way is to add a property in the FedAvg class to save the weight list. Then in the aggregate method, we can check the length of the weight list and the update list and calculate the weighted average. What do you think?

tem556 commented 1 year ago

Isn't that a big against single responsiblity principle or am I misunderstanding. Like the FedAvg class does the average. A new FedWeighted class would do weights. What do you think?

tem556 commented 1 year ago

But yes, I think your way would be much easier actually

gnahtb commented 1 year ago

Isn't that a big against single responsiblity principle or am I misunderstanding. Like the FedAvg class does the average. A new FedWeighted class would do weights. What do you think?

Well, I would not say so because apparently when people talk about FedAvg, they expect it to be weighted. I appreciate the thoughts though.