parameter client library: stub and cgo part with functional test. #2172
Conversation
helinwang
requested review from
Yancey1989,
wangkuiyi,
jacquesqiao,
dzhwinter,
gongweibao and
typhoonzero
There was a problem hiding this comment.
I suggest restructure the source tree:
Following the TensorFlow's source code convention, we put all Go code in
github.com/paddlepaddle/paddle/paddle/go
and refer to Go packages in Go code as
import (
pd "github.com/paddlepaddle/paddle/paddle/go"
)Under the .../go directory, we should have the following packages:
-
pd/pserver/rpc_service.godefines the Go RPC type of pserver. -
pd/cmd/pserver/pserer.godefines the main function of pserver and builds into$GOPATH/bin/pserver. -
pd/pserver/cclient/cclient.gobuilds intocclient.h,libclient.aPlease be aware that we need to customize CMake to generate the header file and the static library in corresponding path in
/build:build/paddle/go/pserver/cclient/cclient.h build/paddle_go_pserver_cclient_libcclient.aand CMake source set the following directories as the default directory when it calls GCC:
gcc -I~/work/paddle -I~/work/paddle/build ...
so that we can write the following header inclusion in paddle_trainer.cc:
#include "paddle/go/pserver/cclient/cclient.h"
and our CMake functions should add the link flag
-lpaddle_go_pserver_cclient_cclient.a
| // servers. Other trainers will be blocked until the initialization is | ||
| // done, and they need to get the initialized parameters from | ||
| // parameter servers using GetParams. | ||
| func (c *Client) BeginInitParams(pserverConfigProto []byte) (selected bool, err error) { |
There was a problem hiding this comment.
If we naming return parameters, maybe only need return at the end of the function.
There was a problem hiding this comment.
Is there a function or function param to tell parameter server which optimization algorithm to use?
There was a problem hiding this comment.
@typhoonzero
we just set optimizer as a parameter in pserverConfig Proto, then parameterserver create optimizer with the optimizer_id
see the related API issue
#2168
There was a problem hiding this comment.
@Yancey1989 that's a good use case, but here is mainly for documentation purpose (when user see the name, he understands what the return value stands for.)
| ElementType ElementType | ||
| Content []byte | ||
| } | ||
|
|
There was a problem hiding this comment.
do we need to define the ParameterBlock type(partitioned Parameter ) and put these things in a Constant header file? These things would be a shared concept in many files.
There was a problem hiding this comment.
I don't quite follow what is "ParameterBlock type", do you mean ElementType?
| // servers. Other trainers will be blocked until the initialization is | ||
| // done, and they need to get the initialized parameters from | ||
| // parameter servers using GetParams. | ||
| func (c *Client) BeginInitParams(pserverConfigProto []byte) (selected bool, err error) { |
There was a problem hiding this comment.
Is there a function or function param to tell parameter server which optimization algorithm to use?
paddle/go/pserver/lib/client/main.go
Outdated
| typedef struct { | ||
| char* name; | ||
| paddle_element_type element_type; | ||
| char* content; |
There was a problem hiding this comment.
Should this be unsigned char * to indicate 0-255 byte stream?
There was a problem hiding this comment.
Good point! Done.
paddle/go/pserver/lib/client/main.go
Outdated
| free(param->content); | ||
| } | ||
|
|
||
| free(param); |
There was a problem hiding this comment.
Need to set the pointer to NULL after free. If this function is the finalization step, just ignore this comment.
There was a problem hiding this comment.
It's the finalization step :)
paddle/go/pserver/lib/client/main.go
Outdated
| @@ -0,0 +1,239 @@ | |||
| package main | |||
There was a problem hiding this comment.
Is this main.go program going to compile to a shared library like .so, so that trainers which are written in python can call this lib? If so, we should add a CPython wrapper to call this CGo program, and the paddle/go/pserver/lib/client/test/main.c should be a python example. Current paddle/go/pserver/lib/client/test/main.c can be moved to the python wrapper functions.
There was a problem hiding this comment.
I was thinking this need to be integrated into Paddle C++ code (a lot of pointers are used, not sure how much does CPython supports pointer), maybe I am wrong. I think we will know better when we begin to integrate.
|
@wangkuiyi Thanks! The folder structure is corrected. |
|
@typhoonzero Thanks :) cmake part is very rough, I will fix with the help from @gangliao (his work is still in PR) |
cmake is taken from /~https://github.com/gangliao/cmake_cxx_go .
The CMakeLists.txt I wrote is not perfect (some hard coded path in CMakeLists.txt, and gtest not working, perhaps because I don't know how to use cmake), but something enough to get started.
The temporary cmake solution can be fixed after #2154 being fixed.