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

[in progress] Expose API for parameter server #1039

Closed

Conversation

jacquesqiao
Copy link
Member

@jacquesqiao jacquesqiao commented Dec 29, 2016

as comment of @reyoung, split this pr into two. one for pserver and one for swig.
1, PserverUtil #1051
2, swig api. TBD

@jacquesqiao jacquesqiao changed the title Expose API for parameter server [in progress] Expose API for parameter server Dec 29, 2016
@jacquesqiao jacquesqiao self-assigned this Dec 30, 2016
Copy link
Collaborator

@reyoung reyoung left a comment

Choose a reason for hiding this comment

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

Please split this PR to 2 PRs.

First is Extract PServer Utils.

Second is Expose PServerUtils to SWIG.

void init();
void start();
void join();

Copy link
Collaborator

Choose a reason for hiding this comment

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

DISABLE_COPY(PServerUtil);


class PServerUtil {
public:
void init();
Copy link
Collaborator

Choose a reason for hiding this comment

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

init => 构造函数.

Copy link
Collaborator

Choose a reason for hiding this comment

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

这里如果方便的话,可以考虑把GFLAGS提取出来,变成函数的参数。

Copy link
Member Author

Choose a reason for hiding this comment

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

好的,这里是这么想的,变成参数

void init();
void start();
void join();

Copy link
Collaborator

Choose a reason for hiding this comment

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

析构的时候调用join.

void join();

private:
std::vector<std::shared_ptr<ParameterServer2>> pservers_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

看起来,std::vector<std::unique_ptr< ParameterServer2 >> pservers_;
就够了

for (auto& pserver : pservers) {
pserver->join();
}
PServerUtil* pserverUtil = new PServerUtil();
Copy link
Collaborator

Choose a reason for hiding this comment

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

std::unique_ptr pservers(new PServerUtil());

把init实现在构造函数里,把join放到析构函数里的。

pservers->start();

}
}
}
PServerUtil* pserverUtil = new PServerUtil();
Copy link
Collaborator

Choose a reason for hiding this comment

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

同上。这里new完了都没有delete。

如果不是必要,不要用C++的裸指针。

如果这个对象不会被多个对象使用,用unique_ptr
如果这个对象会被多个对象使用,用shared_ptr

Copy link
Member Author

Choose a reason for hiding this comment

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

好的,正在优化代码~~

@@ -0,0 +1,77 @@
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refer to source file naming convention and rename this to be pserver_util.cpp.

@@ -0,0 +1,33 @@
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refer to source file naming convention and rename this to be pserver_util.h.

Copy link
Member Author

Choose a reason for hiding this comment

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

好的,代码规范问题,会在分开的两个pr中进行,这个pr先关掉。


from py_paddle import swig_paddle as api

#import pudb;pudb.set_trace()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove the unused line other than commenting it out.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok,in progress

@@ -0,0 +1,33 @@
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refer to source file naming convention and rename this to be parameter_server.cpp.

def main():
api.initPaddle("--nics=lo0", "--port=7164", "--ports_num=1",
"--num_gradient_servers=1", "--comment=paddle_pserver")
pserver = api.ParameterServer.createParameterServer()
Copy link
Collaborator

Choose a reason for hiding this comment

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

我没有太看明白这里的API设计。我以为这里是想通过调用Python API启动一个parameter server process?

如果是,那么是不是应该把 L21到L26简化为,比如:

psid = api.pserver.start(nics="lo0", port=7164, ports_num=1, num_gradient_server=1, comment="paddle_pserver")
api.pserver.wait(psid)

Copy link
Member Author

Choose a reason for hiding this comment

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

对,这个后面正在修改,目前长这个样子是因为历史遗留问题,initPaddle实际上是去初始化各种gflags,后面的版本已经改成proto配置了,见#1051

def main():
api.initPaddle("--nics=lo0", "--port=7164", "--ports_num=1",
"--num_gradient_servers=1", "--comment=paddle_pserver")
pserver = api.ParameterServer.createParameterServer()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow Python code convention and rename module/package name ParameterServer to be pserver or parameter_server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants