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.

Sign up for GitHub

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

Jump to bottom

[Enhancement Request] Integrate Plato into Sedna as a backend for supporting federated learning - Phase one #116

Merged
merged 2 commits into from
Sep 8, 2021
Merged

[Enhancement Request] Integrate Plato into Sedna as a backend for supporting federated learning - Phase one #116

merged 2 commits into from
Sep 8, 2021

Conversation

XinYao1994
Copy link
Contributor

@XinYao1994 XinYao1994 commented Jul 7, 2021 •
edited
Loading

This is a PR for integrating Plato into Sedna to support federated learning (#50). @li-ch @baochunli @jaypume

  • We currently support Fedavg and Mistnet in Sedna via Plato.
  • We add one example for each federated learning job.

Finished

  • A tool to automatically generate Plato configuration file based on the CRD.
  • Replace the communication libs with Sedna build-in libs. S3 and asyncio are used.
  • Examples and demo presentation
    • CV: Yolo-v5 demo
  • Trainer and Estimator
  • datasource is from sedna dataset

@kubeedge-bot
Copy link
Collaborator

Welcome @XinYao1994! It looks like this is your first PR to kubeedge/sedna 🎉

@kubeedge-bot kubeedge-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 7, 2021
@kubeedge-bot kubeedge-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 7, 2021
JoeyHwong-gk
JoeyHwong-gk reviewed Jul 7, 2021
View reviewed changes
examples/federated_learning/mistnet/agg_worker.py Outdated
@@ -0,0 +1,8 @@
import sedna.service.server
Copy link
Contributor

Choose a reason for hiding this comment

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

useless import

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

JoeyHwong-gk
JoeyHwong-gk reviewed Jul 7, 2021
View reviewed changes
examples/federated_learning/mistnet/client.yml Outdated
@@ -0,0 +1,68 @@
clients:
Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to discuss whether this yaml file needs to be generated by the developer or just for plato compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a discussion with @jaypume, this Plato configuration file will be automatically generated based on the CRD.

JoeyHwong-gk
JoeyHwong-gk reviewed Jul 7, 2021
View reviewed changes
lib/requirements.txt Outdated
uvicorn~=0.14.0 # BSD
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems scary, since here add all dependences for all cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

lib/sedna/service/server/aggregation.py Outdated

from plato.servers import mistnet

class MistnetServer(mistnet.Server):
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to work directly with import plato

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we want to keep transparency when the users are using Sedna. So we package the Plato server within the Sedna lib. Sedna's communication libs (transmitter) are under development @jaypume. After that, we will replace the communication libs with Sedna build-in libs.

lib/sedna/datasources/__init__.py Outdated
@@ -130,3 +130,23 @@ def parse(self, *args, **kwargs):
return
self.x = pd.concat(x_data)
self.y = pd.concat(y_data)

# import os
# os.environ['config_file'] = '/home/work/client.yml'
Copy link
Contributor

Choose a reason for hiding this comment

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

it is recommended to clean up unwanted codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

lib/sedna/core/federated_learning/federated_learning.py Outdated
@@ -43,6 +42,7 @@ def __init__(self, estimator, aggregation="FedAvg"):
super(FederatedLearning, self).__init__(
estimator=estimator, config=config)
self.aggregation = ClassFactory.get_cls(ClassType.FL_AGG, aggregation)
self.transmitter = ClassFactory.get_cls(ClassType.TRANSMITTER, transmitter)
Copy link
Contributor

Choose a reason for hiding this comment

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

test_transmitter doesn't seem to be registered in sedna.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is under development. @jaypume
In the current stage, we enable Plato in Sedna via directly call its server and client.

lib/sedna/algorithms/transmitter/base.py Outdated

@abstractmethod
def compress(self): # 传输的内容可能有：weights，压缩后的weights， 特征向量，蒸馏后的数据
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Code comments are best described in English.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jie Pu has updated it.

examples/federated_learning/surface_defect_detection/training_worker/interface.py Outdated
@@ -21,6 +21,58 @@
os.environ['BACKEND_TYPE'] = 'KERAS'


import torch
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like the Pytorch framework used, but keras is defined in the Env variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

surface_defect_detection is using Keras, and we will perform it in a similar way.

lib/sedna/algorithms/aggregation/__init__.py Outdated
# self.fedavg_server = fedavg.Server(model=model)
pass

def aggregate0(self, weights, size=0):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is naming aggregate0 more recommended than aggregate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

lib/sedna/algorithms/aggregation/mistnet.py Outdated
from .base import BaseAggregation


class MistNet(BaseAggregation):
Copy link
Contributor

Choose a reason for hiding this comment

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

If it needs to be called by the registration factory, maybe we can add the registration wrap with ClassFactory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jie Pu has updated it.

lib/sedna/service/server/aggregation.py Outdated
@@ -241,3 +268,17 @@ async def client_info(self, request: Request):
if client_id:
return server.get_client(client_id)
return WSClientInfoList(clients=server.client_list)

import os
os.environ['config_file'] = '/home/work/server.yml'
Copy link
Contributor

Choose a reason for hiding this comment

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

The import module can be add on the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@XinYao1994 XinYao1994 force-pushed the main branch 2 times, most recently from 8fcbfee to 4639049 Compare July 8, 2021 01:36
@kubeedge-bot kubeedge-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2021
@jaypume jaypume force-pushed the main branch 2 times, most recently from bc9291c to 8aa9a9e Compare July 27, 2021 02:18
@kubeedge-bot kubeedge-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2021
@jaypume jaypume closed this Jul 27, 2021
@kubeedge-bot kubeedge-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 27, 2021
@jaypume
Copy link
Member

jaypume commented Jul 27, 2021

/reopen

@kubeedge-bot
Copy link
Collaborator

@jaypume: Failed to re-open PR: state cannot be changed. There are no new commits on the XinYao1994:main branch.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jaypume
Copy link
Member

jaypume commented Jul 27, 2021

/reopen

@kubeedge-bot kubeedge-bot reopened this Jul 27, 2021
@kubeedge-bot
Copy link
Collaborator

@jaypume: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kubeedge-bot kubeedge-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 27, 2021
JimmyYang20
JimmyYang20 reviewed Sep 8, 2021
View reviewed changes
examples/federated_learning/yolov5_coco128_mistnet/README.md Outdated
Create data interface for ```EDGE1_NODE```.

```shell
mkdir -p /data
Copy link

Choose a reason for hiding this comment

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

delete mkdir -p /data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

JimmyYang20
JimmyYang20 reviewed Sep 8, 2021
View reviewed changes
examples/federated_learning/yolov5_coco128_mistnet/README.md Outdated
Create data interface for ```EDGE2_NODE```.

```shell
mkdir -p /data
Copy link

Choose a reason for hiding this comment

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

delete mkdir -p /data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

examples/federated_learning/yolov5_coco128_mistnet/README.md Outdated
create the directory `./data/COCO` in the host of `$CLOUD_NODE` for storing test data.
```bash
mkdir ./data
mkdir ./data/COCO
Copy link

Choose a reason for hiding this comment

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

any directory ?

Suggested change
mkdir ./data/COCO
mkdir -p /data/COCO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

JimmyYang20
JimmyYang20 reviewed Sep 8, 2021
View reviewed changes
examples/federated_learning/yolov5_coco128_mistnet/README.md

```bash
# on the cloud side
mkdir -p /model
Copy link

Choose a reason for hiding this comment

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

Suggested change
mkdir -p /model
mkdir /model

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suggest sync with mkdir -p

examples/federated_learning/yolov5_coco128_mistnet/README.md
```bash
# on the cloud side
mkdir -p /model
mkdir -p /pretrained
Copy link

Choose a reason for hiding this comment

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

Suggested change
mkdir -p /pretrained
mkdir /pretrained

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

JimmyYang20
JimmyYang20 reviewed Sep 8, 2021
View reviewed changes
examples/federated_learning/yolov5_coco128_mistnet/README.md Outdated
metadata:
name: "yolo-v5-model"
spec:
url: "/model/yolov5"
Copy link

Choose a reason for hiding this comment

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

why not /model ?

Copy link
Contributor Author

@XinYao1994 XinYao1994 Sep 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

this is a file, replaced with /model/yolov5.pth

examples/federated_learning/yolov5_coco128_mistnet/README.md Outdated
metadata:
name: "yolo-v5-pretrained-model"
spec:
url: "/pretrained/yolov5"
Copy link

Choose a reason for hiding this comment

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

why not /pretrained/yolov5.pth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

jaypume and others added 2 commits September 8, 2021 21:06
@jaypume
add transmitter, client_choose, aggregation interface
d920f8e 
1. add transmitter, client_choose, aggregation interface to Lib.
2. add example of how to use new added interface.

Signed-off-by: Jie Pu <pujie2@huawei.com>
Signed-off-by: XinYao1994 <xyao@cs.hku.hk>

update
@XinYao1994 @jaypume
add federated learning inplementation by plato
2488b3f 
Signed-off-by: XinYao1994 <xyao@cs.hku.hk>
@jaypume
Copy link
Member

jaypume commented Sep 8, 2021

/lgtm

@kubeedge-bot kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2021
@llhuii
Copy link

llhuii commented Sep 8, 2021

/approve
Thank you for this great job, @XinYao1994

@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: llhuii, luosiqi, MooreZheng

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubeedge-bot kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 8, 2021
@kubeedge-bot kubeedge-bot merged commit e110ac7 into kubeedge:main Sep 8, 2021
@XinYao1994
Copy link
Contributor Author

/approve
Thank you for this great job, @XinYao1994

Thanks for the suggestions and help from all committee members.

@XinYao1994 XinYao1994 mentioned this pull request Sep 16, 2021
Merged
@MooreZheng MooreZheng mentioned this pull request May 5, 2022
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@llhuii llhuii llhuii left review comments

@jaypume jaypume jaypume left review comments

@ugvddm ugvddm ugvddm left review comments

@JimmyYang20 JimmyYang20 JimmyYang20 left review comments

@JoeyHwong-gk JoeyHwong-gk JoeyHwong-gk left review comments

@JensenJian JensenJian JensenJian left review comments

@MooreZheng MooreZheng MooreZheng approved these changes

@luosiqi luosiqi luosiqi approved these changes

@TymonXie TymonXie Awaiting requested review from TymonXie

Assignees

@jaypume jaypume

@ugvddm ugvddm

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@XinYao1994 @kubeedge-bot @jaypume @ugvddm @JoeyHwong-gk @llhuii @MooreZheng @JimmyYang20 @JensenJian @luosiqi