-
Notifications
You must be signed in to change notification settings - Fork 167
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
[Enhancement Request] Integrate Plato into Sedna as a backend for supporting federated learning - Phase one #116
Conversation
Welcome @XinYao1994! It looks like this is your first PR to kubeedge/sedna 🎉 |
@@ -0,0 +1,8 @@ | |||
import sedna.service.server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
@@ -0,0 +1,68 @@ | |||
clients: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
lib/requirements.txt
Outdated
uvicorn~=0.14.0 # BSD |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
|
||
from plato.servers import mistnet | ||
|
||
class MistnetServer(mistnet.Server): |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
@@ -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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
||
@abstractmethod | ||
def compress(self): # 传输的内容可能有:weights,压缩后的weights, 特征向量,蒸馏后的数据 | ||
pass |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -21,6 +21,58 @@ | |||
os.environ['BACKEND_TYPE'] = 'KERAS' | |||
|
|||
|
|||
import torch |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
# self.fedavg_server = fedavg.Server(model=model) | ||
pass | ||
|
||
def aggregate0(self, weights, size=0): |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
from .base import BaseAggregation | ||
|
||
|
||
class MistNet(BaseAggregation): |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
@@ -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' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
8fcbfee
to
4639049
Compare
bc9291c
to
8aa9a9e
Compare
/reopen |
@jaypume: Failed to re-open PR: state cannot be changed. There are no new commits on the XinYao1994:main branch. In response to this:
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. |
/reopen |
@jaypume: Reopened this PR. In response to this:
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. |
32769a7
to
f4fc8c4
Compare
Create data interface for ```EDGE1_NODE```. | ||
|
||
```shell | ||
mkdir -p /data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete mkdir -p /data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
a7195a9
to
12ff782
Compare
Create data interface for ```EDGE2_NODE```. | ||
|
||
```shell | ||
mkdir -p /data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete mkdir -p /data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
create the directory `./data/COCO` in the host of `$CLOUD_NODE` for storing test data. | ||
```bash | ||
mkdir ./data | ||
mkdir ./data/COCO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any directory ?
mkdir ./data/COCO | |
mkdir -p /data/COCO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
e7373b4
to
972af2a
Compare
|
||
```bash | ||
# on the cloud side | ||
mkdir -p /model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mkdir -p /model | |
mkdir /model |
There was a problem hiding this comment.
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
```bash | ||
# on the cloud side | ||
mkdir -p /model | ||
mkdir -p /pretrained |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mkdir -p /pretrained | |
mkdir /pretrained |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
03223dc
to
959a8c7
Compare
metadata: | ||
name: "yolo-v5-model" | ||
spec: | ||
url: "/model/yolov5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not /model
?
There was a problem hiding this comment.
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
metadata: | ||
name: "yolo-v5-pretrained-model" | ||
spec: | ||
url: "/pretrained/yolov5" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
ec17133
to
e135a4d
Compare
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
Signed-off-by: XinYao1994 <xyao@cs.hku.hk>
/lgtm |
/approve |
[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 |
Thanks for the suggestions and help from all committee members. |
This is a PR for integrating Plato into Sedna to support federated learning (#50). @li-ch @baochunli @jaypume
Fedavg
andMistnet
in Sedna via Plato.Finished
Replace the communication libs with Sedna build-in libs.S3
andasyncio
are used.Trainer
andEstimator
datasource
is from sedna dataset