From 3919b75884749684e0bd8b502e426fa4949f2c1f Mon Sep 17 00:00:00 2001 From: gongweibao Date: Wed, 28 Jun 2017 12:01:32 +0000 Subject: [PATCH 01/48] modify cmake --- go/master/c/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/master/c/CMakeLists.txt b/go/master/c/CMakeLists.txt index acce698051..3eb598a877 100644 --- a/go/master/c/CMakeLists.txt +++ b/go/master/c/CMakeLists.txt @@ -6,7 +6,7 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${PARENT_DIR}/cmake") project(cxx_go C Go) -include(golang) +#include(golang) include(flags) set(MASTER_LIB_NAME "paddle_master") From fc3d03142582dcd673cc97fb3b0239bac59815f4 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Thu, 29 Jun 2017 09:38:25 +0800 Subject: [PATCH 02/48] first add --- go/master/c/client.go | 5 ++ go/master/client.go | 3 +- python/paddle/v2/master/client.py | 3 ++ python/paddle/v2/reader/creator.py | 49 ++++++++++++++----- python/paddle/v2/reader/tests/creator_test.py | 2 +- 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/go/master/c/client.go b/go/master/c/client.go index b186474dc3..b88911b858 100644 --- a/go/master/c/client.go +++ b/go/master/c/client.go @@ -88,7 +88,12 @@ func paddle_set_dataset(client C.paddle_master_client, path **C.char, size C.int func paddle_next_record(client C.paddle_master_client, record **C.uchar) C.int { c := get(client) r := c.NextRecord() + if r == nil { + // EOF + return -1 + } if len(r) == 0 { + // Empty record *record = (*C.uchar)(nullPtr) return 0 } diff --git a/go/master/client.go b/go/master/client.go index 8451820c19..4f8df5ba66 100644 --- a/go/master/client.go +++ b/go/master/client.go @@ -60,6 +60,7 @@ func (c *Client) getRecords() { } err = f.Close() + c.ch <- nil if err != nil { log.Errorln(err) } @@ -112,7 +113,7 @@ func (c *Client) monitorMaster(addr Addresser) { // // SetDataset can be call multiple times from different nodes. But // only the first call will be honored. -func (c *Client) SetDataset(globPaths []string) error { +func (c *Client) SetDataset(globPaths ...string) error { return c.conn.Call("Service.SetDataset", globPaths, nil) } diff --git a/python/paddle/v2/master/client.py b/python/paddle/v2/master/client.py index de8e9bb88e..9fd3ef0860 100644 --- a/python/paddle/v2/master/client.py +++ b/python/paddle/v2/master/client.py @@ -30,6 +30,9 @@ class client(object): p = ctypes.c_char_p() ret = ctypes.pointer(p) size = lib.paddle_next_record(self.c, ret) + if size < 0: + # EOF + return None if size == 0: # Empty record return "" diff --git a/python/paddle/v2/reader/creator.py b/python/paddle/v2/reader/creator.py index 9f888b16d6..669867fd10 100644 --- a/python/paddle/v2/reader/creator.py +++ b/python/paddle/v2/reader/creator.py @@ -57,22 +57,49 @@ def text_file(path): return reader -def recordio(path): +def recordio_local(paths): """ - Creates a data reader that outputs record one one by one from given recordio file - :path: path of recordio file - :returns: data reader of recordio file + Creates a data reader that outputs record one one by one + from given local recordio fils path. + :path: path of recordio files. + :returns: data reader of recordio files. """ import recordio as rec def reader(): - f = rec.reader(path) - while True: - r = f.read() - if r is None: - break - yield r - f.close() + for i, path in enumerate(paths): + f = rec.reader(path) + while True: + r = f.read() + if r is None: + break + yield r + f.close() return reader + + +def recordio(paths, addr="", buf_size=100): + """ + Creates a data reader that outputs record one one by one + from given local or cloud recordio path. + :path: path of recordio files. + :returns: data reader of recordio files. + """ + import os + import paddle.v2.master.client as cloud + + if len(os.environ["KUBERNETES_SERVICE_HOST"]) == 0: + return recordio_local(path) + + c = cloud(addr, buf_size) + c.set_dataset(paths) + + while True: + r = client.next_record() + if r is None: + break + yield r + + c.close() diff --git a/python/paddle/v2/reader/tests/creator_test.py b/python/paddle/v2/reader/tests/creator_test.py index ba4f558874..b42d273ecf 100644 --- a/python/paddle/v2/reader/tests/creator_test.py +++ b/python/paddle/v2/reader/tests/creator_test.py @@ -38,7 +38,7 @@ class TestRecordIO(unittest.TestCase): def test_recordio(self): path = os.path.join( os.path.dirname(__file__), "test_recordio_creator.dat") - reader = paddle.v2.reader.creator.recordio(path) + reader = paddle.v2.reader.creator.recordio([path]) for idx, r in enumerate(reader()): self.assertSequenceEqual(r, str(idx)) From 4874810ba5a1e6f8f6b4a9530e6854f65077a59e Mon Sep 17 00:00:00 2001 From: gongweibao Date: Thu, 29 Jun 2017 04:28:44 +0000 Subject: [PATCH 03/48] fix bugs --- go/master/client.go | 2 +- python/paddle/v2/reader/creator.py | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/go/master/client.go b/go/master/client.go index 4f8df5ba66..fa479338c5 100644 --- a/go/master/client.go +++ b/go/master/client.go @@ -113,7 +113,7 @@ func (c *Client) monitorMaster(addr Addresser) { // // SetDataset can be call multiple times from different nodes. But // only the first call will be honored. -func (c *Client) SetDataset(globPaths ...string) error { +func (c *Client) SetDataset(globPaths []string) error { return c.conn.Call("Service.SetDataset", globPaths, nil) } diff --git a/python/paddle/v2/reader/creator.py b/python/paddle/v2/reader/creator.py index 669867fd10..3376d7accb 100644 --- a/python/paddle/v2/reader/creator.py +++ b/python/paddle/v2/reader/creator.py @@ -93,13 +93,17 @@ def recordio(paths, addr="", buf_size=100): if len(os.environ["KUBERNETES_SERVICE_HOST"]) == 0: return recordio_local(path) - c = cloud(addr, buf_size) - c.set_dataset(paths) + def reader(): + c = cloud(addr, buf_size) + c.set_dataset(paths) + + while True: + r = client.next_record() + if r is None: + break + yield r - while True: - r = client.next_record() - if r is None: - break - yield r + c.close() + + return reader - c.close() From 0fa409246b98c636d4dd32553782ca962f70a6f7 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Thu, 29 Jun 2017 09:43:00 +0000 Subject: [PATCH 04/48] fix bugs --- go/master/c/client.go | 18 ++++++++++++++++-- go/master/client.go | 21 +++++++++++++++------ go/master/client_test.go | 18 ++++++++++++++---- python/paddle/v2/reader/creator.py | 6 ++---- 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/go/master/c/client.go b/go/master/c/client.go index b88911b858..79e13e4b63 100644 --- a/go/master/c/client.go +++ b/go/master/c/client.go @@ -13,6 +13,7 @@ typedef int paddle_master_client; import "C" import ( + "io" "sync" "unsafe" @@ -84,14 +85,27 @@ func paddle_set_dataset(client C.paddle_master_client, path **C.char, size C.int return C.PADDLE_MASTER_OK } +// return value: +// 0:ok +// -1:EOF +// -2:error //export paddle_next_record func paddle_next_record(client C.paddle_master_client, record **C.uchar) C.int { c := get(client) - r := c.NextRecord() - if r == nil { + r, err := c.NextRecord() + if err == io.EOF { // EOF + *record = (*C.uchar)(nullPtr) return -1 } + + if err != nil { + // Error + // TODO: return the type of error? + *record = (*C.uchar)(nullPtr) + return -2 + } + if len(r) == 0 { // Empty record *record = (*C.uchar)(nullPtr) diff --git a/go/master/client.go b/go/master/client.go index fa479338c5..c122d17c8f 100644 --- a/go/master/client.go +++ b/go/master/client.go @@ -1,6 +1,7 @@ package master import ( + "io" "os" "time" @@ -17,7 +18,12 @@ type Addresser interface { // Client is the client of the master server. type Client struct { conn *connection.Conn - ch chan []byte + ch chan record +} + +type record struct { + r []byte + err error } // NewClient creates a new Client. @@ -27,7 +33,7 @@ type Client struct { func NewClient(addr Addresser, bufSize int) *Client { c := &Client{} c.conn = connection.New() - c.ch = make(chan []byte, bufSize) + c.ch = make(chan record, bufSize) go c.monitorMaster(addr) go c.getRecords() return c @@ -52,18 +58,20 @@ func (c *Client) getRecords() { s := recordio.NewRangeScanner(f, &chunk.Index, -1, -1) for s.Scan() { - c.ch <- s.Record() + c.ch <- record{s.Record(), nil} } if s.Err() != nil { + c.ch <- record{nil, s.Err()} log.Errorln(err, chunk.Path) } err = f.Close() - c.ch <- nil if err != nil { log.Errorln(err) } + + c.ch <- record{nil, io.EOF} } // We treat a task as finished whenever the last data @@ -133,6 +141,7 @@ func (c *Client) taskFinished(taskID int) error { // // NextRecord will block until the next record is available. It is // thread-safe. -func (c *Client) NextRecord() []byte { - return <-c.ch +func (c *Client) NextRecord() ([]byte, error) { + r := <-c.ch + return r.r, r.err } diff --git a/go/master/client_test.go b/go/master/client_test.go index 85a86761c2..05201941e3 100644 --- a/go/master/client_test.go +++ b/go/master/client_test.go @@ -2,6 +2,7 @@ package master_test import ( "fmt" + "io" "net" "net/http" "net/rpc" @@ -69,13 +70,22 @@ func TestNextRecord(t *testing.T) { for pass := 0; pass < 50; pass++ { received := make(map[byte]bool) - for i := 0; i < total; i++ { - r := c.NextRecord() + for i := 0; i <= total; i++ { + r, err := c.NextRecord() + if err == io.EOF { + break + } + + if err != nil { + t.Fatal(pass, i, "Read error:", err) + } + if len(r) != 1 { - t.Fatal("Length should be 1.", r) + t.Fatal(pass, i, "Length should be 1.", r) } + if received[r[0]] { - t.Fatal("Received duplicate.", received, r) + t.Fatal(pass, i, "Received duplicate.", received, r) } received[r[0]] = true } diff --git a/python/paddle/v2/reader/creator.py b/python/paddle/v2/reader/creator.py index 3376d7accb..b575f57dc6 100644 --- a/python/paddle/v2/reader/creator.py +++ b/python/paddle/v2/reader/creator.py @@ -79,7 +79,6 @@ def recordio_local(paths): return reader - def recordio(paths, addr="", buf_size=100): """ Creates a data reader that outputs record one one by one @@ -90,8 +89,8 @@ def recordio(paths, addr="", buf_size=100): import os import paddle.v2.master.client as cloud - if len(os.environ["KUBERNETES_SERVICE_HOST"]) == 0: - return recordio_local(path) + if "KUBERNETES_SERVICE_HOST" not in os.environ.keys(): + return recordio_local(paths) def reader(): c = cloud(addr, buf_size) @@ -106,4 +105,3 @@ def recordio(paths, addr="", buf_size=100): c.close() return reader - From b79784ee9e0fd67933d4793e8ab4564f7a30c780 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Thu, 29 Jun 2017 09:52:21 +0000 Subject: [PATCH 05/48] fix bugs --- python/paddle/v2/master/client.py | 18 ++++++++++++++---- python/paddle/v2/reader/creator.py | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/python/paddle/v2/master/client.py b/python/paddle/v2/master/client.py index 9fd3ef0860..0cc01b7310 100644 --- a/python/paddle/v2/master/client.py +++ b/python/paddle/v2/master/client.py @@ -26,17 +26,27 @@ class client(object): holder[idx] = c_ptr lib.paddle_set_dataset(self.c, holder, len(paths)) + # return format: (record, errno) + # errno = 0: ok + # = -1: EOF + # < -1: error def next_record(self): p = ctypes.c_char_p() ret = ctypes.pointer(p) size = lib.paddle_next_record(self.c, ret) - if size < 0: + if size == -1: # EOF - return None + return None, -1 + + if size < -1: + # Error + return None, size + if size == 0: # Empty record - return "" + return "", 0 + record = ret.contents.value[:size] # Memory created from C should be freed. lib.mem_free(ret.contents) - return record + return record, 0 diff --git a/python/paddle/v2/reader/creator.py b/python/paddle/v2/reader/creator.py index b575f57dc6..2e8626e565 100644 --- a/python/paddle/v2/reader/creator.py +++ b/python/paddle/v2/reader/creator.py @@ -97,7 +97,7 @@ def recordio(paths, addr="", buf_size=100): c.set_dataset(paths) while True: - r = client.next_record() + r, err = client.next_record() if r is None: break yield r From b3c5808e13bc94fbc933c803c59fed979a11f515 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Fri, 30 Jun 2017 03:11:57 +0000 Subject: [PATCH 06/48] rm cloud EOF --- go/master/c/client.go | 7 ------- go/master/client.go | 3 --- go/master/client_test.go | 7 +------ python/paddle/v2/master/client.py | 5 ----- 4 files changed, 1 insertion(+), 21 deletions(-) diff --git a/go/master/c/client.go b/go/master/c/client.go index 79e13e4b63..a37894fefe 100644 --- a/go/master/c/client.go +++ b/go/master/c/client.go @@ -13,7 +13,6 @@ typedef int paddle_master_client; import "C" import ( - "io" "sync" "unsafe" @@ -93,12 +92,6 @@ func paddle_set_dataset(client C.paddle_master_client, path **C.char, size C.int func paddle_next_record(client C.paddle_master_client, record **C.uchar) C.int { c := get(client) r, err := c.NextRecord() - if err == io.EOF { - // EOF - *record = (*C.uchar)(nullPtr) - return -1 - } - if err != nil { // Error // TODO: return the type of error? diff --git a/go/master/client.go b/go/master/client.go index c122d17c8f..985b96b0af 100644 --- a/go/master/client.go +++ b/go/master/client.go @@ -1,7 +1,6 @@ package master import ( - "io" "os" "time" @@ -70,8 +69,6 @@ func (c *Client) getRecords() { if err != nil { log.Errorln(err) } - - c.ch <- record{nil, io.EOF} } // We treat a task as finished whenever the last data diff --git a/go/master/client_test.go b/go/master/client_test.go index 05201941e3..0a401d8a43 100644 --- a/go/master/client_test.go +++ b/go/master/client_test.go @@ -2,7 +2,6 @@ package master_test import ( "fmt" - "io" "net" "net/http" "net/rpc" @@ -70,12 +69,8 @@ func TestNextRecord(t *testing.T) { for pass := 0; pass < 50; pass++ { received := make(map[byte]bool) - for i := 0; i <= total; i++ { + for i := 0; i < total; i++ { r, err := c.NextRecord() - if err == io.EOF { - break - } - if err != nil { t.Fatal(pass, i, "Read error:", err) } diff --git a/python/paddle/v2/master/client.py b/python/paddle/v2/master/client.py index 0cc01b7310..6ddb09e4e8 100644 --- a/python/paddle/v2/master/client.py +++ b/python/paddle/v2/master/client.py @@ -28,16 +28,11 @@ class client(object): # return format: (record, errno) # errno = 0: ok - # = -1: EOF # < -1: error def next_record(self): p = ctypes.c_char_p() ret = ctypes.pointer(p) size = lib.paddle_next_record(self.c, ret) - if size == -1: - # EOF - return None, -1 - if size < -1: # Error return None, size From 97bbd179569f48bfcf1a3ff3225c331ad8e3fbf4 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Fri, 30 Jun 2017 03:14:29 +0000 Subject: [PATCH 07/48] rm cloud EOF --- go/master/c/client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/master/c/client.go b/go/master/c/client.go index a37894fefe..13ed3b7680 100644 --- a/go/master/c/client.go +++ b/go/master/c/client.go @@ -86,7 +86,6 @@ func paddle_set_dataset(client C.paddle_master_client, path **C.char, size C.int // return value: // 0:ok -// -1:EOF // -2:error //export paddle_next_record func paddle_next_record(client C.paddle_master_client, record **C.uchar) C.int { From 26e661bc51e2fac36c3692d748b7db8a950cb370 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Mon, 3 Jul 2017 03:05:36 +0000 Subject: [PATCH 08/48] fix by helin's comments --- go/master/c/client.go | 4 ++-- python/paddle/v2/master/client.py | 4 ++-- python/paddle/v2/reader/creator.py | 34 ++++++++++++++++++------------ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/go/master/c/client.go b/go/master/c/client.go index 635688f196..31f4311974 100644 --- a/go/master/c/client.go +++ b/go/master/c/client.go @@ -106,7 +106,7 @@ func paddle_set_dataset(client C.paddle_master_client, path **C.char, size C.int // return value: // 0:ok -// -2:error +// -1:error //export paddle_next_record func paddle_next_record(client C.paddle_master_client, record **C.uchar) C.int { c := get(client) @@ -115,7 +115,7 @@ func paddle_next_record(client C.paddle_master_client, record **C.uchar) C.int { // Error // TODO: return the type of error? *record = (*C.uchar)(nullPtr) - return -2 + return -1 } if len(r) == 0 { diff --git a/python/paddle/v2/master/client.py b/python/paddle/v2/master/client.py index 6ddb09e4e8..70f9e43c96 100644 --- a/python/paddle/v2/master/client.py +++ b/python/paddle/v2/master/client.py @@ -28,12 +28,12 @@ class client(object): # return format: (record, errno) # errno = 0: ok - # < -1: error + # < 0: error def next_record(self): p = ctypes.c_char_p() ret = ctypes.pointer(p) size = lib.paddle_next_record(self.c, ret) - if size < -1: + if size < 0: # Error return None, size diff --git a/python/paddle/v2/reader/creator.py b/python/paddle/v2/reader/creator.py index 2e8626e565..20624d5286 100644 --- a/python/paddle/v2/reader/creator.py +++ b/python/paddle/v2/reader/creator.py @@ -57,29 +57,31 @@ def text_file(path): return reader -def recordio_local(paths): +def recordio_local(paths, buf_size=100): """ - Creates a data reader that outputs record one one by one - from given local recordio fils path. + Creates a data reader from given RecordIO file paths separated by ",", + glob pattern is supported. :path: path of recordio files. :returns: data reader of recordio files. """ import recordio as rec + import paddle.v2.reader.decorator as dec def reader(): - for i, path in enumerate(paths): - f = rec.reader(path) - while True: - r = f.read() - if r is None: - break - yield r - f.close() + a = ','.join(paths) + f = rec.reader(a) + while True: + r = f.read() + if r is None: + break + yield r + f.close() + + return dec.buffered(reader, buf_size) - return reader -def recordio(paths, addr="", buf_size=100): +def recordio(paths, buf_size=100): """ Creates a data reader that outputs record one one by one from given local or cloud recordio path. @@ -92,6 +94,12 @@ def recordio(paths, addr="", buf_size=100): if "KUBERNETES_SERVICE_HOST" not in os.environ.keys(): return recordio_local(paths) + host_name = "MASTER_SERVICE_HOST" + if host_name not in os.environ.keys(): + raise Exception('not find ' + host_name + ' in environ.') + + addr = os.environ(host) + def reader(): c = cloud(addr, buf_size) c.set_dataset(paths) From 5ef1425adb75eb1b0212518e0f12fefd8d9a8970 Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Mon, 3 Jul 2017 21:13:20 +0800 Subject: [PATCH 09/48] "init saving model" --- go/pserver/optimizer.go | 16 ++++++++++++++-- go/pserver/service.go | 12 +++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/go/pserver/optimizer.go b/go/pserver/optimizer.go index b4a040f46b..427251f900 100644 --- a/go/pserver/optimizer.go +++ b/go/pserver/optimizer.go @@ -40,17 +40,23 @@ func newOptimizer(paramWithConfigs ParameterWithConfig) *optimizer { o.elementType = paramWithConfigs.Param.ElementType p := paramWithConfigs.Param c := paramWithConfigs.Config + s := paramWithConfigs.State log.WithFields(log.Fields{ "ElementType": p.ElementType, "ParamSize": len(p.Content), "ConfigSize": len(c), + "StateSize": len(s), }).Info("New Optimizer Created with config:") var cbuffer unsafe.Pointer cbuffer = C.malloc(C.size_t(len(p.Content))) C.memcpy(cbuffer, unsafe.Pointer(&p.Content[0]), C.size_t(len(p.Content))) + var cstate unsafe.Pointer + if len(s) != 0 { + cstate = unsafe.Pointer(&s[0]) + } + o.opt = C.paddle_create_optimizer((*C.uchar)(&c[0]), C.int(len(c)), - C.paddle_element_type(p.ElementType), cbuffer, C.int(len(p.Content)/C.sizeof_float), - (*C.char)(nullPtr), 0) + C.paddle_element_type(p.ElementType), cbuffer, C.int(len(p.Content)/C.sizeof_float), (*C.char)(cstate), C.int(len(s))) return o } @@ -60,6 +66,12 @@ func (o *optimizer) GetWeights() []byte { return cArrayToSlice(buffer, int(buffer_len)*C.sizeof_float) } +func (o *optimizer) GetStates() []byte { + var cbuffer *C.char + cbuffer_len := C.paddle_optimizer_get_state(o.opt, &cbuffer) + return cArrayToSlice(unsafe.Pointer(cbuffer), int(cbuffer_len)) +} + func (o *optimizer) UpdateParameter(g Gradient) error { if o.elementType != g.ElementType { return fmt.Errorf("Name: %s, parameter and gradient element type not match, parameter: %v, gradient: %v", g.Name, o.elementType, g.ElementType) diff --git a/go/pserver/service.go b/go/pserver/service.go index e15a4e5a58..a5ff862903 100644 --- a/go/pserver/service.go +++ b/go/pserver/service.go @@ -38,6 +38,7 @@ type Parameter struct { type ParameterWithConfig struct { Param Parameter Config []byte // parameter configuration in Proto Buffer format + State []byte // parameter training state } // Gradient is the gradient of the parameter. @@ -58,7 +59,7 @@ func NewService(idx int) (*Service, error) { s := &Service{ idx: idx, } - s.optMap = make(map[string]*optimizer) + s.optMap = make(map[string]*optimizer) s.initialized = make(chan struct{}) return s, nil } @@ -143,7 +144,12 @@ func (s *Service) GetParam(name string, parameter *Parameter) error { // Save tells the parameter server to save parameters. func (s *Service) Save(path string, dummy *int) error { <-s.initialized - - // TODO + for opt, ok := range s.optMap { + if ok != nil { + return fmt.Errorf("parameter optimizerMap error: ", ok) + } + state := opt.GetStates() + weights := opt.GetWeights() + } return nil } From f1330e216a1b8130bb578b69ff2d6a67357cdd1b Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Mon, 3 Jul 2017 23:20:39 +0800 Subject: [PATCH 10/48] "saving checkpoint" --- go/pserver/service.go | 79 +++++++++++++++++++++++++++++++++++--- go/pserver/service_test.go | 6 +++ 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/go/pserver/service.go b/go/pserver/service.go index a5ff862903..a4cf3e4750 100644 --- a/go/pserver/service.go +++ b/go/pserver/service.go @@ -1,9 +1,19 @@ package pserver import ( + "bufio" + "bytes" + "crypto/md5" + "encoding/gob" + "encoding/hex" "errors" "fmt" + "os" + "strconv" "sync" + "time" + + log "github.com/sirupsen/logrus" ) // ElementType is the type of elements of a Parameter. @@ -14,6 +24,10 @@ const ( Uninitialized = "pserver not fully initialized" ) +const ( + checkpoint_path = "/checkpoints/" +) + // Supported element types const ( Int32 ElementType = iota @@ -53,6 +67,24 @@ type Service struct { optMap map[string]*optimizer } +type Checkpoint struct { + uuid string + md5sum string + timestamp string +} + +//serialize ParameterWithConfig to byte stream +func GetBytes(content ...interface{}) ([]byte, error) { + + var buf bytes.Buffer + encoder := gob.NewEncoder(&buf) + err := encoder.Encode(content) + if err != nil { + return nil, err + } + return buf.Bytes(), nil +} + // NewService creates a new service, will bypass etcd registration if no // endpoints specified. func NewService(idx int) (*Service, error) { @@ -143,13 +175,50 @@ func (s *Service) GetParam(name string, parameter *Parameter) error { // Save tells the parameter server to save parameters. func (s *Service) Save(path string, dummy *int) error { + //FIXME: checkpoint is only used by pserver + // and has a constant path of */checkpoints/{pserver_idx}* <-s.initialized - for opt, ok := range s.optMap { - if ok != nil { - return fmt.Errorf("parameter optimizerMap error: ", ok) + s.mu.Lock() + defer s.mu.Unlock() + var paramWithConfig ParameterWithConfig + for name, opt := range s.optMap { + paramWithConfig.Param.Name = name + paramWithConfig.Param.ElementType = opt.elementType + paramWithConfig.Param.Content = opt.GetWeights() + paramWithConfig.State = opt.GetStates() + content, err := GetBytes(paramWithConfig) + if err != nil { + log.Errorln(err) + } + ck := Checkpoint{} + h := md5.New() + ck.md5sum = hex.EncodeToString(h.Sum(content)) + ck.timestamp = time.Now().String() + ck.uuid = checkpoint_path + strconv.Itoa(s.idx) + ckbytes, err := GetBytes(ck) + if err != nil { + log.Errorln(err) + } + // TODO: according design doc, need to save uuid to etcd in json format + // {\"uuid\": [UUID], \"md5\", \"MD5 sum\", \"timestamp\": xxxx} + log.Infof("parameter checkpoint %s", ckbytes) + + if _, err = os.Stat(ck.uuid); os.IsNotExist(err) { + log.Info("checkpoint not exists.") + } else { + err = os.Remove(ck.uuid) + log.Infof("remove %s", ck.uuid) + } + f, err := os.Create(ck.uuid) + defer f.Close() + if err != nil { + log.Errorln(err) + } + writer := bufio.NewWriter(f) + _, err = writer.Write(content) + if err != nil { + log.Errorln(err) } - state := opt.GetStates() - weights := opt.GetWeights() } return nil } diff --git a/go/pserver/service_test.go b/go/pserver/service_test.go index f86619447c..28956e4d85 100644 --- a/go/pserver/service_test.go +++ b/go/pserver/service_test.go @@ -79,6 +79,8 @@ func TestServiceFull(t *testing.T) { if !reflect.DeepEqual(param1, p) { t.FailNow() } + var dummy int + s.Save("", &dummy) } func TestMultipleInit(t *testing.T) { @@ -166,3 +168,7 @@ func TestBlockUntilInitialized(t *testing.T) { wg.Wait() } + +func TestCheckpointSpeed(t *testing.T) { + //TODO: test speed +} From 65afbe11853c2e32ca4196965e309e33ab843fd1 Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Mon, 3 Jul 2017 23:38:21 +0800 Subject: [PATCH 11/48] "fix gob register error" --- go/pserver/service.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/go/pserver/service.go b/go/pserver/service.go index a4cf3e4750..decd3682ae 100644 --- a/go/pserver/service.go +++ b/go/pserver/service.go @@ -25,7 +25,7 @@ const ( ) const ( - checkpoint_path = "/checkpoints/" + checkpoint_path = "./checkpoints/" ) // Supported element types @@ -67,10 +67,10 @@ type Service struct { optMap map[string]*optimizer } -type Checkpoint struct { - uuid string - md5sum string - timestamp string +type checkpoint struct { + Uuid string + Md5sum string + Timestamp string } //serialize ParameterWithConfig to byte stream @@ -93,6 +93,8 @@ func NewService(idx int) (*Service, error) { } s.optMap = make(map[string]*optimizer) s.initialized = make(chan struct{}) + gob.Register(ParameterWithConfig{}) + gob.Register(checkpoint{}) return s, nil } @@ -190,32 +192,33 @@ func (s *Service) Save(path string, dummy *int) error { if err != nil { log.Errorln(err) } - ck := Checkpoint{} + ck := checkpoint{} h := md5.New() - ck.md5sum = hex.EncodeToString(h.Sum(content)) - ck.timestamp = time.Now().String() - ck.uuid = checkpoint_path + strconv.Itoa(s.idx) + ck.Md5sum = hex.EncodeToString(h.Sum(content)) + ck.Timestamp = time.Now().String() + ck.Uuid = checkpoint_path + strconv.Itoa(s.idx) ckbytes, err := GetBytes(ck) if err != nil { log.Errorln(err) } - // TODO: according design doc, need to save uuid to etcd in json format - // {\"uuid\": [UUID], \"md5\", \"MD5 sum\", \"timestamp\": xxxx} + // TODO: according design doc, need to save Uuid to etcd in json format + // {\"Uuid\": [UUID], \"md5\", \"MD5 sum\", \"Timestamp\": xxxx} log.Infof("parameter checkpoint %s", ckbytes) - if _, err = os.Stat(ck.uuid); os.IsNotExist(err) { + if _, err = os.Stat(ck.Uuid); os.IsNotExist(err) { log.Info("checkpoint not exists.") } else { - err = os.Remove(ck.uuid) - log.Infof("remove %s", ck.uuid) + err = os.Remove(ck.Uuid) + log.Infof("remove %s", ck.Uuid) } - f, err := os.Create(ck.uuid) + f, err := os.Create(ck.Uuid) defer f.Close() if err != nil { log.Errorln(err) } writer := bufio.NewWriter(f) _, err = writer.Write(content) + writer.Flush() if err != nil { log.Errorln(err) } From 6935dd7bc96e101ec65de39be1d2d8f4f79f1af3 Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Tue, 4 Jul 2017 00:16:45 +0800 Subject: [PATCH 12/48] "lr state serialization" --- paddle/optimizer/lr_policy.h | 46 ++++++++++++++++++++++--------- paddle/optimizer/sgd_optimizer.cc | 4 +-- proto/OptimizerConfig.proto | 27 ++++++++---------- 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/paddle/optimizer/lr_policy.h b/paddle/optimizer/lr_policy.h index d8e33ad37a..ab5101e2e8 100644 --- a/paddle/optimizer/lr_policy.h +++ b/paddle/optimizer/lr_policy.h @@ -19,34 +19,54 @@ class ConstLr final : public LrPolicy { public: ConstLr(double lr) : learning_rate(lr){}; double LearningRate(const uint64_t num_sample_passed) { - return learning_rate; + return learning_rate_; + } + const char *SerializeState(int *state_len) { + LrPolicyState state; + state.set_learning_rate(learning_rate_); + auto str = state.SerializeAsString(); + *state_len = str.size(); + return str.c_str(); + } + void DeserializeState(const std::string &state) { + LrPolicyState state; + state.ParseFromString(str); + learning_rate_ = state.learning_rate(); } - const char *SerializeState(int *state_len) { return nullptr; } - void DeserializeState(const std::string &state) {} private: - double learning_rate; + double learning_rate_; }; class LinearLr final : public LrPolicy { public: LinearLr(double lr, double lr_decay_a, double lr_decay_b) - : learning_rate(lr), lr_decay_a(lr_decay_a), lr_decay_b(lr_decay_b) {} + : learning_rate_(lr), lr_decay_a_(lr_decay_a), lr_decay_b_(lr_decay_b) {} double LearningRate(const uint64_t num_sample_passed) { - return std::max(learning_rate - lr_decay_a * num_sample_passed, lr_decay_b); + return std::max(learning_rate_ - lr_decay_a_ * num_sample_passed, + lr_decay_b_); } const char *SerializeState(int *state_len) { - // TODO(zhihong) : add lr_policy serialization - return nullptr; + LrPolicyState state; + state.set_learning_rate(learning_rate_); + state.set_lr_decay_a(lr_decay_a_); + state.set_lr_decay_b(lr_decay_b_); + auto str = state.SerializeAsString(); + *state_len = str.size(); + return str.c_str(); } - void DeserializeState(const std::string &state) { - // TODO(zhihong) : add lr_policy serialization + void DeserializeState(const std::string &str) { + LrPolicyState state; + state.ParseFromString(str); + learning_rate_ = state.learning_rate(); + lr_decay_a_ = state.lr_decay_a(); + lr_decay_b_ = state.lr_decay_b(); } private: - double learning_rate; - double lr_decay_a; - double lr_decay_b; + double learning_rate_; + double lr_decay_a_; + double lr_decay_b_; }; } // namespace optimizer diff --git a/paddle/optimizer/sgd_optimizer.cc b/paddle/optimizer/sgd_optimizer.cc index 34e051003f..9e5477b2ff 100644 --- a/paddle/optimizer/sgd_optimizer.cc +++ b/paddle/optimizer/sgd_optimizer.cc @@ -30,10 +30,10 @@ void SGDOptimizer::Update(const Tensor *gradient) { const char *SGDOptimizer::SerializeState(int *state_len) { SGDOptimizerState state; state.set_num_sample_passed(num_sample_passed_); - TensorToProto(*parameter_, state.mutable_parameter()); + state.set_lr_ TensorToProto(*parameter_, state.mutable_parameter()); if (momentum_ != 0.0) TensorToProto(*momentums_, state.mutable_momentums()); auto str = state.SerializeAsString(); - *state_len = str.size(); + *state_len += str.size(); return str.c_str(); } diff --git a/proto/OptimizerConfig.proto b/proto/OptimizerConfig.proto index c698d3c2dd..19ce289ea3 100644 --- a/proto/OptimizerConfig.proto +++ b/proto/OptimizerConfig.proto @@ -78,11 +78,15 @@ enum DataType { repeated bytes content = 2; } +message LrPolicyState { + // learninRate Policy + optional double learning_rate = 1 [default = 1.0]; + optional double lr_decay_a = 2; + optional double lr_decay_b = 3; +} + message SGDOptimizerState { - // learning rate policy - optional double learning_rate = 101; - optional double lr_decay_a = 102; - optional double lr_decay_b = 103; + optional LrPolicyState lrstate = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; @@ -91,9 +95,7 @@ message SGDOptimizerState { message AdadeltaOptimizerState { // learning rate policy - optional double learning_rate = 101; - optional double lr_decay_a = 102; - optional double lr_decay_b = 103; + optional LrPolicyState lrstate = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; @@ -102,11 +104,9 @@ message AdadeltaOptimizerState { optional TensorProto update_delta = 4; } + message AdagradOptimizerState { - // learning rate policy - optional double learning_rate = 101; - optional double lr_decay_a = 102; - optional double lr_decay_b = 103; + optional LrPolicyState lrstate = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; @@ -114,10 +114,7 @@ message AdagradOptimizerState { } message AdamOptimizerState { - // learning rate policy - optional double learning_rate = 101; - optional double lr_decay_a = 102; - optional double lr_decay_b = 103; + optional LrPolicyState lrstate = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; From e1acd73fab4e17db5700feba09339f09d7152406 Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Tue, 4 Jul 2017 01:13:30 +0800 Subject: [PATCH 13/48] "fix typo deleted part" --- paddle/optimizer/lr_policy.h | 4 ++-- paddle/optimizer/sgd_optimizer.cc | 6 +++++- proto/OptimizerConfig.proto | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/paddle/optimizer/lr_policy.h b/paddle/optimizer/lr_policy.h index ab5101e2e8..036c376e10 100644 --- a/paddle/optimizer/lr_policy.h +++ b/paddle/optimizer/lr_policy.h @@ -17,7 +17,7 @@ public: // constant learning rate policy class ConstLr final : public LrPolicy { public: - ConstLr(double lr) : learning_rate(lr){}; + ConstLr(double lr) : learning_rate_(lr){}; double LearningRate(const uint64_t num_sample_passed) { return learning_rate_; } @@ -28,7 +28,7 @@ public: *state_len = str.size(); return str.c_str(); } - void DeserializeState(const std::string &state) { + void DeserializeState(const std::string &str) { LrPolicyState state; state.ParseFromString(str); learning_rate_ = state.learning_rate(); diff --git a/paddle/optimizer/sgd_optimizer.cc b/paddle/optimizer/sgd_optimizer.cc index 9e5477b2ff..527e65144d 100644 --- a/paddle/optimizer/sgd_optimizer.cc +++ b/paddle/optimizer/sgd_optimizer.cc @@ -30,7 +30,11 @@ void SGDOptimizer::Update(const Tensor *gradient) { const char *SGDOptimizer::SerializeState(int *state_len) { SGDOptimizerState state; state.set_num_sample_passed(num_sample_passed_); - state.set_lr_ TensorToProto(*parameter_, state.mutable_parameter()); + std::string lr_str = this->lr_policy_->SerializeState(state_len); + LrPolicyState lr_state; + lr_state.ParseFromString(lr_str); + state.mutable_lr_state() = lr_state; + TensorToProto(*parameter_, state.mutable_parameter()); if (momentum_ != 0.0) TensorToProto(*momentums_, state.mutable_momentums()); auto str = state.SerializeAsString(); *state_len += str.size(); diff --git a/proto/OptimizerConfig.proto b/proto/OptimizerConfig.proto index 19ce289ea3..290932898e 100644 --- a/proto/OptimizerConfig.proto +++ b/proto/OptimizerConfig.proto @@ -95,7 +95,7 @@ message SGDOptimizerState { message AdadeltaOptimizerState { // learning rate policy - optional LrPolicyState lrstate = 101; + optional LrPolicyState lr_state = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; From 7edabe74d45b9dd35603ac786e6a36e201bb1177 Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Tue, 4 Jul 2017 01:21:13 +0800 Subject: [PATCH 14/48] "polish name convention" --- paddle/optimizer/sgd_optimizer.cc | 4 +++- proto/OptimizerConfig.proto | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/paddle/optimizer/sgd_optimizer.cc b/paddle/optimizer/sgd_optimizer.cc index 527e65144d..96570eab26 100644 --- a/paddle/optimizer/sgd_optimizer.cc +++ b/paddle/optimizer/sgd_optimizer.cc @@ -33,7 +33,7 @@ const char *SGDOptimizer::SerializeState(int *state_len) { std::string lr_str = this->lr_policy_->SerializeState(state_len); LrPolicyState lr_state; lr_state.ParseFromString(lr_str); - state.mutable_lr_state() = lr_state; + state.mutable_lr_state()->ParseFromString(lr_str); TensorToProto(*parameter_, state.mutable_parameter()); if (momentum_ != 0.0) TensorToProto(*momentums_, state.mutable_momentums()); auto str = state.SerializeAsString(); @@ -44,6 +44,8 @@ const char *SGDOptimizer::SerializeState(int *state_len) { void SGDOptimizer::DeserializeState(const std::string &str) { SGDOptimizerState state; state.ParseFromString(str); + auto lr_state = state.lr_state(); + this->lr_policy_->DeserializeState(lr_state.SerializeAsString()); num_sample_passed_ = state.num_sample_passed(); ProtoToTensor(state.parameter(), parameter_); if (momentum_ != 0.0) ProtoToTensor(state.parameter(), momentums_); diff --git a/proto/OptimizerConfig.proto b/proto/OptimizerConfig.proto index 290932898e..2a87e293f6 100644 --- a/proto/OptimizerConfig.proto +++ b/proto/OptimizerConfig.proto @@ -86,7 +86,7 @@ message LrPolicyState { } message SGDOptimizerState { - optional LrPolicyState lrstate = 101; + optional LrPolicyState lr_state = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; @@ -106,7 +106,7 @@ message AdadeltaOptimizerState { message AdagradOptimizerState { - optional LrPolicyState lrstate = 101; + optional LrPolicyState lr_state = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; @@ -114,7 +114,7 @@ message AdagradOptimizerState { } message AdamOptimizerState { - optional LrPolicyState lrstate = 101; + optional LrPolicyState lr_state = 101; optional double num_sample_passed = 104; // state optional TensorProto parameter = 1; From dec65aca7ddcbeac1ba54608bc487dc93d2d28f3 Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Tue, 4 Jul 2017 01:24:27 +0800 Subject: [PATCH 15/48] "fix parameter accumulate size" --- paddle/optimizer/adadelta_optimizer.cc | 8 +++++--- paddle/optimizer/adagrad_optimizer.cc | 9 ++++++--- paddle/optimizer/adam_optimizer.cc | 9 ++++++--- paddle/optimizer/sgd_optimizer.cc | 2 -- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/paddle/optimizer/adadelta_optimizer.cc b/paddle/optimizer/adadelta_optimizer.cc index 465ad5e0d2..6eec5d846f 100644 --- a/paddle/optimizer/adadelta_optimizer.cc +++ b/paddle/optimizer/adadelta_optimizer.cc @@ -27,22 +27,24 @@ void AdadeltaOptimizer::Update(const Tensor* gradient) { const char* AdadeltaOptimizer::SerializeState(int* state_len) { AdadeltaOptimizerState state; - // TODO(zhihong) : add lr_policy serialization state.set_num_sample_passed(num_sample_passed_); + std::string lr_str = this->lr_policy_->SerializeState(state_len); + state.mutable_lr_state()->ParseFromString(lr_str); TensorToProto(*parameter_, state.mutable_parameter()); TensorToProto(*accum_gradient_, state.mutable_accum_gradient()); TensorToProto(*accum_delta_, state.mutable_accum_delta()); TensorToProto(*update_delta_, state.mutable_update_delta()); auto str = state.SerializeAsString(); - *state_len = str.size(); + *state_len += str.size(); return str.c_str(); } void AdadeltaOptimizer::DeserializeState(const std::string& str) { AdadeltaOptimizerState state; state.ParseFromString(str); - // TODO(zhihong) : add lr_policy DeserializeState + auto lr_state = state.lr_state(); + this->lr_policy_->DeserializeState(lr_state.SerializeAsString()); num_sample_passed_ = state.num_sample_passed(); ProtoToTensor(state.parameter(), parameter_); diff --git a/paddle/optimizer/adagrad_optimizer.cc b/paddle/optimizer/adagrad_optimizer.cc index bdaa7877d2..5b92610ac5 100644 --- a/paddle/optimizer/adagrad_optimizer.cc +++ b/paddle/optimizer/adagrad_optimizer.cc @@ -19,20 +19,23 @@ void AdagradOptimizer::Update(const Tensor* gradient) { } const char* AdagradOptimizer::SerializeState(int* state_len) { AdagradOptimizerState state; - // TODO(zhihong) : add lr_policy serialization state.set_num_sample_passed(num_sample_passed_); + std::string lr_str = this->lr_policy_->SerializeState(state_len); + state.mutable_lr_state()->ParseFromString(lr_str); TensorToProto(*parameter_, state.mutable_parameter()); TensorToProto(*accum_gradient_, state.mutable_accum_gradient()); auto str = state.SerializeAsString(); - *state_len = str.size(); + *state_len += str.size(); return str.c_str(); } void AdagradOptimizer::DeserializeState(const std::string& str) { AdagradOptimizerState state; state.ParseFromString(str); - // TODO(zhihong) : add lr_policy DeserializeState + auto lr_state = state.lr_state(); + this->lr_policy_->DeserializeState(lr_state.SerializeAsString()); + num_sample_passed_ = state.num_sample_passed(); ProtoToTensor(state.parameter(), parameter_); ProtoToTensor(state.accum_gradient(), accum_gradient_); diff --git a/paddle/optimizer/adam_optimizer.cc b/paddle/optimizer/adam_optimizer.cc index ceab7397d8..1ebb6b1e0f 100644 --- a/paddle/optimizer/adam_optimizer.cc +++ b/paddle/optimizer/adam_optimizer.cc @@ -24,20 +24,23 @@ void AdamOptimizer::Update(const Tensor *gradient) { const char *AdamOptimizer::SerializeState(int *state_len) { AdamOptimizerState state; - // TODO(zhihong) : add lr_policy serialization + std::string lr_str = this->lr_policy_->SerializeState(state_len); + state.mutable_lr_state()->ParseFromString(lr_str); state.set_num_sample_passed(num_sample_passed_); + TensorToProto(*parameter_, state.mutable_parameter()); TensorToProto(*momentums_, state.mutable_momentums()); TensorToProto(*velocitys_, state.mutable_velocitys()); auto str = state.SerializeAsString(); - *state_len = str.size(); + *state_len += str.size(); return str.c_str(); } void AdamOptimizer::DeserializeState(const std::string &str) { AdamOptimizerState state; state.ParseFromString(str); - // TODO(zhihong) : add lr_policy DeserializeState + auto lr_state = state.lr_state(); + this->lr_policy_->DeserializeState(lr_state.SerializeAsString()); num_sample_passed_ = state.num_sample_passed(); ProtoToTensor(state.parameter(), parameter_); diff --git a/paddle/optimizer/sgd_optimizer.cc b/paddle/optimizer/sgd_optimizer.cc index 96570eab26..15418faa84 100644 --- a/paddle/optimizer/sgd_optimizer.cc +++ b/paddle/optimizer/sgd_optimizer.cc @@ -31,8 +31,6 @@ const char *SGDOptimizer::SerializeState(int *state_len) { SGDOptimizerState state; state.set_num_sample_passed(num_sample_passed_); std::string lr_str = this->lr_policy_->SerializeState(state_len); - LrPolicyState lr_state; - lr_state.ParseFromString(lr_str); state.mutable_lr_state()->ParseFromString(lr_str); TensorToProto(*parameter_, state.mutable_parameter()); if (momentum_ != 0.0) TensorToProto(*momentums_, state.mutable_momentums()); From e12d7269ff473db5cc87de1344630eb348017a4a Mon Sep 17 00:00:00 2001 From: gongweibao Date: Tue, 4 Jul 2017 01:22:01 +0000 Subject: [PATCH 16/48] fix by helin's comments --- python/paddle/v2/reader/creator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/paddle/v2/reader/creator.py b/python/paddle/v2/reader/creator.py index 20624d5286..61b5cc134f 100644 --- a/python/paddle/v2/reader/creator.py +++ b/python/paddle/v2/reader/creator.py @@ -106,7 +106,7 @@ def recordio(paths, buf_size=100): while True: r, err = client.next_record() - if r is None: + if err < 0: break yield r From 7364fcd4c3c6b08b569ed2bb809bed9904b55030 Mon Sep 17 00:00:00 2001 From: wuyi05 Date: Wed, 5 Jul 2017 15:42:17 +0800 Subject: [PATCH 17/48] add golang precommit --- .pre-commit-config.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4cd8eb12f6..a7c450176d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,3 +21,10 @@ sha: 28c0ea8a67a3e2dbbf4822ef44e85b63a0080a29 hooks: - id: clang-formater +- repo: https://github.com/dnephin/pre-commit-golang + sha: e4693a4c282b4fc878eda172a929f7a6508e7d16 + hooks: + - id: go-fmt + - id: go-vet + - id: go-lint + - id: gometalinter From 7c6aa04f6185e92082b9a742d5c746b335406711 Mon Sep 17 00:00:00 2001 From: wuyi05 Date: Wed, 5 Jul 2017 16:24:53 +0800 Subject: [PATCH 18/48] add go pre-commit and travis build --- .pre-commit-config.yaml | 4 ++-- .travis.yml | 4 ++-- go/pserver/service.go | 6 ++++-- paddle/scripts/travis/build_doc.sh | 11 ++++++----- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a7c450176d..61b989dc69 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,6 +25,6 @@ sha: e4693a4c282b4fc878eda172a929f7a6508e7d16 hooks: - id: go-fmt - - id: go-vet + files: (.*\.go) - id: go-lint - - id: gometalinter + files: (.*\.go) diff --git a/.travis.yml b/.travis.yml index 16432dac0c..aafeeba027 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,7 +33,7 @@ addons: - ccache before_install: - if [[ "$JOB" == "check_style" ]]; then sudo ln -s /usr/bin/clang-format-3.8 /usr/bin/clang-format; fi - # Paddle is using protobuf 3.1 currently. Protobuf 3.2 breaks the compatibility. So we specify the python + # Paddle is using protobuf 3.1 currently. Protobuf 3.2 breaks the compatibility. So we specify the python # protobuf version. - pip install numpy wheel 'protobuf==3.1' sphinx==1.5.6 recommonmark sphinx-rtd-theme==0.1.9 virtualenv pre-commit requests==2.9.2 LinkChecker - pip install rarfile @@ -42,7 +42,7 @@ before_install: function timeout() { perl -e 'alarm shift; exec @ARGV' "$@"; } script: - | - export WITH_GOLANG=ON && timeout 2580 paddle/scripts/travis/${JOB}.sh # 43min timeout + timeout 2580 paddle/scripts/travis/${JOB}.sh # 43min timeout RESULT=$?; if [ $RESULT -eq 0 ] || [ $RESULT -eq 142 ]; then true; else false; fi; notifications: email: diff --git a/go/pserver/service.go b/go/pserver/service.go index 7711dc027e..ad16a5708d 100644 --- a/go/pserver/service.go +++ b/go/pserver/service.go @@ -10,8 +10,10 @@ import ( type ElementType int const ( + // AlreadyInitialized is true if pserver is initialized AlreadyInitialized = "pserver already initialized" - Uninitialized = "pserver not fully initialized" + // Uninitialized is true if pserver not fully initialized + Uninitialized = "pserver not fully initialized" ) // Supported element types @@ -55,7 +57,7 @@ func NewService(idx int) (*Service, error) { s := &Service{ idx: idx, } - s.optMap = make(map[string]*optimizer) + s.optMap = make(map[string]*optimizer) s.initialized = make(chan struct{}) return s, nil } diff --git a/paddle/scripts/travis/build_doc.sh b/paddle/scripts/travis/build_doc.sh index a44bd35357..a443851580 100755 --- a/paddle/scripts/travis/build_doc.sh +++ b/paddle/scripts/travis/build_doc.sh @@ -5,13 +5,14 @@ set -e mkdir -p $TRAVIS_BUILD_DIR/build cd $TRAVIS_BUILD_DIR/build -# Compile Documentation only. -cmake .. -DCMAKE_BUILD_TYPE=Debug -DWITH_GPU=OFF -DWITH_DOC=OFF -DWITH_STYLE_CHECK=OFF +# Compile paddle binaries first +cmake .. -DCMAKE_BUILD_TYPE=Debug -DWITH_GPU=OFF -DWITH_DOC=OFF -DWITH_GOLANG=ON -DWITH_STYLE_CHECK=OFF mkdir output make -j `nproc` find .. -name '*whl' | xargs pip install # install all wheels. rm -rf * +# Compile Documentation only. cmake .. -DCMAKE_BUILD_TYPE=Debug -DWITH_GPU=OFF -DWITH_DOC=ON make -j `nproc` paddle_docs paddle_docs_cn @@ -25,7 +26,7 @@ SSH_REPO=${REPO/https:\/\/github.com\//git@github.com:} SHA=`git rev-parse --verify HEAD` # Documentation branch name -# gh-pages branch is used for PaddlePaddle.org. The English version of +# gh-pages branch is used for PaddlePaddle.org. The English version of # documentation in `doc` directory, and the chinese version in `doc_cn` # directory. TARGET_BRANCH="gh-pages" @@ -51,7 +52,7 @@ function deploy_docs() { # checkout github page branch git checkout $TARGET_BRANCH || git checkout --orphan $TARGET_BRANCH - + mkdir -p ${DIR} # remove old docs. mv new docs. set +e @@ -62,7 +63,7 @@ function deploy_docs() { git add . } -deploy_docs "master" "." +deploy_docs "master" "." deploy_docs "develop" "./develop/" # Check is there anything changed. From 81bfd47eb3fdbf7a0c398f6ad7e62f1d6e7350c1 Mon Sep 17 00:00:00 2001 From: wuyi05 Date: Wed, 5 Jul 2017 16:32:14 +0800 Subject: [PATCH 19/48] add glide in travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index aafeeba027..498674469b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,6 +37,7 @@ before_install: # protobuf version. - pip install numpy wheel 'protobuf==3.1' sphinx==1.5.6 recommonmark sphinx-rtd-theme==0.1.9 virtualenv pre-commit requests==2.9.2 LinkChecker - pip install rarfile + - curl https://glide.sh/get | bash - eval "$(GIMME_GO_VERSION=1.8.3 gimme)" - | function timeout() { perl -e 'alarm shift; exec @ARGV' "$@"; } From 2f085a7bcf11f5501bded27862988022e32299a0 Mon Sep 17 00:00:00 2001 From: wuyi05 Date: Wed, 5 Jul 2017 17:08:19 +0800 Subject: [PATCH 20/48] add go pserver deps --- go/cmd/pserver/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/pserver/CMakeLists.txt b/go/cmd/pserver/CMakeLists.txt index bc1da3348c..51db6dff04 100644 --- a/go/cmd/pserver/CMakeLists.txt +++ b/go/cmd/pserver/CMakeLists.txt @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -go_binary(pserver SRCS pserver.go) +go_binary(pserver SRCS pserver.go DEPS paddle_go_optimizer) From b220c4757e79ec42be7e1180e3e74cf05f403495 Mon Sep 17 00:00:00 2001 From: wuyi05 Date: Wed, 5 Jul 2017 18:18:32 +0800 Subject: [PATCH 21/48] fix auto cgo LDFLAGS --- go/pserver/optimizer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/pserver/optimizer.go b/go/pserver/optimizer.go index d84f55b987..54d1082094 100644 --- a/go/pserver/optimizer.go +++ b/go/pserver/optimizer.go @@ -2,7 +2,7 @@ package pserver // #cgo CFLAGS: -I ../../ // //FIXME: ldflags contain "build" path -// #cgo LDFLAGS: ../../build/go/pserver/client/c/libpaddle_go_optimizer.a -lstdc++ -lm +// #cgo LDFLAGS: ${SRCDIR}/../../build/go/pserver/client/c/libpaddle_go_optimizer.a -lstdc++ -lm // #include "paddle/optimizer/optimizer.h" // #include // #include @@ -56,8 +56,8 @@ func newOptimizer(paramWithConfigs ParameterWithConfig) *optimizer { func (o *optimizer) GetWeights() []byte { var buffer unsafe.Pointer - buffer_len := C.paddle_optimizer_get_weights(o.opt, &buffer) - return cArrayToSlice(buffer, int(buffer_len)*C.sizeof_float) + bufferLen := C.paddle_optimizer_get_weights(o.opt, &buffer) + return cArrayToSlice(buffer, int(bufferLen)*C.sizeof_float) } func (o *optimizer) UpdateParameter(g Gradient) error { From b68e90be820f7a925e114f76f27156e728fc9e79 Mon Sep 17 00:00:00 2001 From: "yi.wu" Date: Wed, 5 Jul 2017 21:30:28 +0800 Subject: [PATCH 22/48] fix go test building --- go/pserver/client/c/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/go/pserver/client/c/CMakeLists.txt b/go/pserver/client/c/CMakeLists.txt index a3fcaeef19..34aa7ca5ff 100644 --- a/go/pserver/client/c/CMakeLists.txt +++ b/go/pserver/client/c/CMakeLists.txt @@ -1,4 +1,5 @@ cc_library(paddle_go_optimizer DEPS paddle_optimizer paddle_proto glog gflags protobuf) +target_link_libraries(paddle_go_optimizer stdc++ m) go_library(paddle_pserver_cclient STATIC DEPS paddle_go_optimizer) if(WITH_TESTING) add_subdirectory(test) From 78f1274d6e2c75d0036ae2a7da6cbccfc844b8f0 Mon Sep 17 00:00:00 2001 From: "yi.wu" Date: Wed, 5 Jul 2017 21:40:12 +0800 Subject: [PATCH 23/48] remove unnessesary cc_test link --- cmake/generic.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/generic.cmake b/cmake/generic.cmake index d51b95a5d7..c2962e35ef 100644 --- a/cmake/generic.cmake +++ b/cmake/generic.cmake @@ -192,7 +192,7 @@ function(cc_test TARGET_NAME) set(multiValueArgs SRCS DEPS) cmake_parse_arguments(cc_test "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) add_executable(${TARGET_NAME} ${cc_test_SRCS}) - target_link_libraries(${TARGET_NAME} ${cc_test_DEPS} gtest gtest_main -lstdc++ -lm) + target_link_libraries(${TARGET_NAME} ${cc_test_DEPS} gtest gtest_main) add_dependencies(${TARGET_NAME} ${cc_test_DEPS} gtest gtest_main) add_test(NAME ${TARGET_NAME} COMMAND ${TARGET_NAME} WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}) endif() @@ -285,7 +285,7 @@ function(go_library TARGET_NAME) add_custom_command(TARGET ${TARGET_NAME} POST_BUILD COMMAND rm "${${TARGET_NAME}_LIB_PATH}" # Golang build source code - COMMAND env LIBRARY_PATH=${CMAKE_BINARY_DIR}/go/pserver/client/c/:$ENV{LIBRARY_PATH} GOPATH=${GOPATH} ${CMAKE_Go_COMPILER} build ${BUILD_MODE} + COMMAND GOPATH=${GOPATH} ${CMAKE_Go_COMPILER} build ${BUILD_MODE} -o "${${TARGET_NAME}_LIB_PATH}" "./${CMAKE_CURRENT_SOURCE_REL_DIR}/${GO_SOURCE}" # must run under GOPATH From 4d2a83c750c6168d16a4ee302b0c69e553bd0b34 Mon Sep 17 00:00:00 2001 From: "yi.wu" Date: Wed, 5 Jul 2017 21:58:46 +0800 Subject: [PATCH 24/48] update again --- go/pserver/client/c/test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/pserver/client/c/test/CMakeLists.txt b/go/pserver/client/c/test/CMakeLists.txt index f287f85071..dce8645ce7 100644 --- a/go/pserver/client/c/test/CMakeLists.txt +++ b/go/pserver/client/c/test/CMakeLists.txt @@ -1,2 +1,2 @@ -cc_test(test_cclient SRCS test_cclient.c DEPS paddle_pserver_cclient) +cc_test(test_cclient SRCS test_cclient.c DEPS paddle_pserver_cclient paddle_go_optimizer) add_style_check_target(test_cclient test_cclient.c) From 7848a3fb5c6de5c21a6c1c34a9d12e8e866c760c Mon Sep 17 00:00:00 2001 From: wuyi05 Date: Thu, 6 Jul 2017 09:45:01 +0800 Subject: [PATCH 25/48] remove cclient test --- go/pserver/client/c/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/pserver/client/c/CMakeLists.txt b/go/pserver/client/c/CMakeLists.txt index a3fcaeef19..d5c1ed38e5 100644 --- a/go/pserver/client/c/CMakeLists.txt +++ b/go/pserver/client/c/CMakeLists.txt @@ -1,5 +1,7 @@ cc_library(paddle_go_optimizer DEPS paddle_optimizer paddle_proto glog gflags protobuf) go_library(paddle_pserver_cclient STATIC DEPS paddle_go_optimizer) if(WITH_TESTING) - add_subdirectory(test) + # FIXME: this test requires pserver which is not managed by the test + # we need some kind of e2e testing machanism. + # add_subdirectory(test) endif() From d6ecae779a28d51e669a4c029d00ec57a98f2bc8 Mon Sep 17 00:00:00 2001 From: liaogang Date: Thu, 6 Jul 2017 11:25:28 +0800 Subject: [PATCH 26/48] FIX: propagation dependencies and out of date rebuild --- cmake/generic.cmake | 51 ++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/cmake/generic.cmake b/cmake/generic.cmake index cae9524b2f..87d8caaec4 100644 --- a/cmake/generic.cmake +++ b/cmake/generic.cmake @@ -99,15 +99,37 @@ function(merge_static_libs TARGET_NAME) set(libs ${ARGN}) list(REMOVE_DUPLICATES libs) - # First get the file names of the libraries to be merged + # Get all propagation dependencies from the merged libraries foreach(lib ${libs}) + list(APPEND libs_deps ${${lib}_LIB_DEPENDS}) + endforeach() + + # To produce a library we need at least one source file. + # It is created by add_custom_command below and will helps + # also help to track dependencies. + set(dummyfile ${CMAKE_CURRENT_BINARY_DIR}/${TARGET_NAME}_dummy.c) + + # Make the generated dummy source file depended on all static input + # libs. If input lib changes,the source file is touched + # which causes the desired effect (relink). + add_custom_command(OUTPUT ${dummyfile} + COMMAND ${CMAKE_COMMAND} -E touch ${dummyfile} + DEPENDS ${libs}) + + # Generate dummy staic lib + file(WRITE ${dummyfile} "const char * dummy = \"${dummyfile}\";") + add_library(${TARGET_NAME} STATIC ${dummyfile}) + target_link_libraries(${TARGET_NAME} ${libs_deps}) + + foreach(lib ${libs}) + # Get the file names of the libraries to be merged set(libfiles ${libfiles} $) endforeach() + # Get the file name of the generated library + set(outlibfile "$") + if(APPLE) # Use OSX's libtool to merge archives - set(dummyfile ${CMAKE_CURRENT_BINARY_DIR}/${TARGET_NAME}_dummy.c) - file(WRITE ${dummyfile} "const char * dummy = \"${dummyfile}\";") - add_library(${TARGET_NAME} STATIC ${dummyfile}) add_custom_command(TARGET ${TARGET_NAME} POST_BUILD COMMAND rm "${CMAKE_CURRENT_BINARY_DIR}/lib${TARGET_NAME}.a" COMMAND /usr/bin/libtool -static -o "${CMAKE_CURRENT_BINARY_DIR}/lib${TARGET_NAME}.a" ${libfiles}) @@ -117,7 +139,8 @@ function(merge_static_libs TARGET_NAME) set(objdir ${lib}.objdir) add_custom_command(OUTPUT ${objdir} - COMMAND ${CMAKE_COMMAND} -E make_directory ${objdir}) + COMMAND ${CMAKE_COMMAND} -E make_directory ${objdir} + DEPENDS ${lib}) add_custom_command(OUTPUT ${objlistfile} COMMAND ${CMAKE_AR} -x "$" @@ -125,23 +148,9 @@ function(merge_static_libs TARGET_NAME) DEPENDS ${lib} ${objdir} WORKING_DIRECTORY ${objdir}) - # Empty dummy source file that goes into merged library - set(mergebase ${lib}.mergebase.c) - add_custom_command(OUTPUT ${mergebase} - COMMAND ${CMAKE_COMMAND} -E touch ${mergebase} - DEPENDS ${objlistfile}) - - list(APPEND mergebases "${mergebase}") - endforeach() - - # We need a target for the output merged library - add_library(${TARGET_NAME} STATIC ${mergebases}) - set(outlibfile "$") - - foreach(lib ${libs}) add_custom_command(TARGET ${TARGET_NAME} POST_BUILD - COMMAND ${CMAKE_AR} ru ${outlibfile} @"../${lib}.objlist" - WORKING_DIRECTORY ${lib}.objdir) + COMMAND ${CMAKE_AR} ru ${outlibfile} *.o + WORKING_DIRECTORY ${objdir}) endforeach() add_custom_command(TARGET ${TARGET_NAME} POST_BUILD From 3e4ba647eec7bc16511e1146d5a696cd124c6a27 Mon Sep 17 00:00:00 2001 From: liaogang Date: Thu, 6 Jul 2017 11:28:52 +0800 Subject: [PATCH 27/48] FIX: remove duplicate --- cmake/generic.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/cmake/generic.cmake b/cmake/generic.cmake index 87d8caaec4..1a4600ef4b 100644 --- a/cmake/generic.cmake +++ b/cmake/generic.cmake @@ -103,6 +103,7 @@ function(merge_static_libs TARGET_NAME) foreach(lib ${libs}) list(APPEND libs_deps ${${lib}_LIB_DEPENDS}) endforeach() + list(REMOVE_DUPLICATES libs_deps) # To produce a library we need at least one source file. # It is created by add_custom_command below and will helps From e2ea1f42e9202e5591e2de1ce5f96c573dcc6484 Mon Sep 17 00:00:00 2001 From: Yu Yang Date: Thu, 6 Jul 2017 14:12:45 +0800 Subject: [PATCH 28/48] Generate python protobufs for paddle.v2.framework Python should be able to manipulate Protobuf message because: 1. Python's `create_op_creation_methods` take the `OpProto` array to generate all `op_creation_methods` in RunTime. 2. All `op_creation_methods` will create an `OpDesc` and pass it to Paddle C++ method `CreateOp` and return the Op handle. Here is the list of what is added in this commit: * Add `protobuf_generate_python` if it is not defined. * Before cmake 3.4, `protobuf_generate_python` is not defined. Just copy the implementation of that function in `protobuf.cmake` * Add `py_proto_compile` function in `cmake/generic.cmake`. * It follows bazel's API interface. * https://github.com/pubref/rules_protobuf#rules * Add an empty package named `paddle.v2.framework`, all python code of `paddle::framework` will be in that package. * Generate protobuf's python module `__init__.py` by `touch` while compiling. * Change setup.py.in, make `paddle.v2.framework.proto` uses the generated protobuf pythons. --- cmake/external/protobuf.cmake | 59 +++++++++++++++++++ cmake/generic.cmake | 9 +++ paddle/framework/CMakeLists.txt | 5 +- python/CMakeLists.txt | 3 +- python/paddle/v2/framework/__init__.py | 1 + .../paddle/v2/framework/tests/CMakeLists.txt | 1 + .../v2/framework/tests/test_protobuf.py | 26 ++++++++ python/setup.py.in | 9 ++- 8 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 python/paddle/v2/framework/__init__.py create mode 100644 python/paddle/v2/framework/tests/CMakeLists.txt create mode 100644 python/paddle/v2/framework/tests/test_protobuf.py diff --git a/cmake/external/protobuf.cmake b/cmake/external/protobuf.cmake index 3c74944bc2..e629d61585 100644 --- a/cmake/external/protobuf.cmake +++ b/cmake/external/protobuf.cmake @@ -17,6 +17,65 @@ INCLUDE(ExternalProject) FIND_PACKAGE(Protobuf QUIET) SET(PROTOBUF_FOUND "OFF") +if(NOT COMMAND protobuf_generate_python) # before cmake 3.4, protobuf_genrerate_python is not defined. + function(protobuf_generate_python SRCS) + # shameless copy from https://github.com/Kitware/CMake/blob/master/Modules/FindProtobuf.cmake + if(NOT ARGN) + message(SEND_ERROR "Error: PROTOBUF_GENERATE_PYTHON() called without any proto files") + return() + endif() + + if(PROTOBUF_GENERATE_CPP_APPEND_PATH) + # Create an include path for each file specified + foreach(FIL ${ARGN}) + get_filename_component(ABS_FIL ${FIL} ABSOLUTE) + get_filename_component(ABS_PATH ${ABS_FIL} PATH) + list(FIND _protobuf_include_path ${ABS_PATH} _contains_already) + if(${_contains_already} EQUAL -1) + list(APPEND _protobuf_include_path -I ${ABS_PATH}) + endif() + endforeach() + else() + set(_protobuf_include_path -I ${CMAKE_CURRENT_SOURCE_DIR}) + endif() + + if(DEFINED PROTOBUF_IMPORT_DIRS AND NOT DEFINED Protobuf_IMPORT_DIRS) + set(Protobuf_IMPORT_DIRS "${PROTOBUF_IMPORT_DIRS}") + endif() + + if(DEFINED Protobuf_IMPORT_DIRS) + foreach(DIR ${Protobuf_IMPORT_DIRS}) + get_filename_component(ABS_PATH ${DIR} ABSOLUTE) + list(FIND _protobuf_include_path ${ABS_PATH} _contains_already) + if(${_contains_already} EQUAL -1) + list(APPEND _protobuf_include_path -I ${ABS_PATH}) + endif() + endforeach() + endif() + + set(${SRCS}) + foreach(FIL ${ARGN}) + get_filename_component(ABS_FIL ${FIL} ABSOLUTE) + get_filename_component(FIL_WE ${FIL} NAME_WE) + if(NOT PROTOBUF_GENERATE_CPP_APPEND_PATH) + get_filename_component(FIL_DIR ${FIL} DIRECTORY) + if(FIL_DIR) + set(FIL_WE "${FIL_DIR}/${FIL_WE}") + endif() + endif() + + list(APPEND ${SRCS} "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}_pb2.py") + add_custom_command( + OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${FIL_WE}_pb2.py" + COMMAND ${Protobuf_PROTOC_EXECUTABLE} --python_out ${CMAKE_CURRENT_BINARY_DIR} ${_protobuf_include_path} ${ABS_FIL} + DEPENDS ${ABS_FIL} ${Protobuf_PROTOC_EXECUTABLE} + COMMENT "Running Python protocol buffer compiler on ${FIL}" + VERBATIM ) + endforeach() + + set(${SRCS} ${${SRCS}} PARENT_SCOPE) + endfunction() +endif() # Print and set the protobuf library information, # finish this cmake process and exit from this file. diff --git a/cmake/generic.cmake b/cmake/generic.cmake index d51b95a5d7..a92671ae62 100644 --- a/cmake/generic.cmake +++ b/cmake/generic.cmake @@ -335,3 +335,12 @@ function(proto_library TARGET_NAME) protobuf_generate_cpp(proto_srcs proto_hdrs ${proto_library_SRCS}) cc_library(${TARGET_NAME} SRCS ${proto_srcs} DEPS ${proto_library_DEPS} protobuf) endfunction() + +function(py_proto_compile TARGET_NAME) + set(oneValueArgs "") + set(multiValueArgs SRCS) + cmake_parse_arguments(py_proto_compile "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) + set(py_srcs) + protobuf_generate_python(py_srcs ${py_proto_compile_SRCS}) + add_custom_target(${TARGET_NAME} ALL DEPENDS ${py_srcs}) +endfunction() \ No newline at end of file diff --git a/paddle/framework/CMakeLists.txt b/paddle/framework/CMakeLists.txt index dcd70d2851..970b2b9abd 100644 --- a/paddle/framework/CMakeLists.txt +++ b/paddle/framework/CMakeLists.txt @@ -9,6 +9,9 @@ cc_test(enforce_test SRCS enforce_test.cc) proto_library(attr_type SRCS attr_type.proto) proto_library(op_proto SRCS op_proto.proto DEPS attr_type) cc_test(op_proto_test SRCS op_proto_test.cc DEPS op_proto protobuf) - proto_library(op_desc SRCS op_desc.proto DEPS attr_type) cc_test(op_desc_test SRCS op_desc_test.cc DEPS op_desc protobuf) +py_proto_compile(framework_py_proto SRCS attr_type.proto op_proto.proto op_desc.proto) +# Generate an empty __init__.py to make framework_py_proto as a valid python module. +add_custom_target(framework_py_proto_init ALL COMMAND ${CMAKE_COMMAND} -E touch __init__.py) +add_dependencies(framework_py_proto framework_py_proto_init) diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 361e764e25..13a1802ee3 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -29,7 +29,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/setup.py.in add_custom_command(OUTPUT ${OUTPUT_DIR}/.timestamp COMMAND env ${py_env} ${PYTHON_EXECUTABLE} setup.py bdist_wheel COMMAND ${CMAKE_COMMAND} -E touch ${OUTPUT_DIR}/.timestamp - DEPENDS gen_proto_py ${PY_FILES} ${external_project_dependencies} ${COPY_PADDLE_MASTER}) + DEPENDS gen_proto_py framework_py_proto ${PY_FILES} ${external_project_dependencies} ${COPY_PADDLE_MASTER}) add_custom_target(paddle_python ALL DEPENDS ${OUTPUT_DIR}/.timestamp) @@ -43,6 +43,7 @@ if (WITH_TESTING) add_subdirectory(paddle/v2/tests) add_subdirectory(paddle/v2/reader/tests) add_subdirectory(paddle/v2/plot/tests) + add_subdirectory(paddle/v2/framework/tests) endif() endif() install(DIRECTORY ${PADDLE_PYTHON_PACKAGE_DIR} diff --git a/python/paddle/v2/framework/__init__.py b/python/paddle/v2/framework/__init__.py new file mode 100644 index 0000000000..c942373c66 --- /dev/null +++ b/python/paddle/v2/framework/__init__.py @@ -0,0 +1 @@ +__all__ = ['proto'] diff --git a/python/paddle/v2/framework/tests/CMakeLists.txt b/python/paddle/v2/framework/tests/CMakeLists.txt new file mode 100644 index 0000000000..8cb0c5c376 --- /dev/null +++ b/python/paddle/v2/framework/tests/CMakeLists.txt @@ -0,0 +1 @@ +add_python_test(test_framework test_protobuf.py) diff --git a/python/paddle/v2/framework/tests/test_protobuf.py b/python/paddle/v2/framework/tests/test_protobuf.py new file mode 100644 index 0000000000..f0e6019199 --- /dev/null +++ b/python/paddle/v2/framework/tests/test_protobuf.py @@ -0,0 +1,26 @@ +import paddle.v2.framework.proto.op_proto_pb2 +import paddle.v2.framework.proto.attr_type_pb2 +import unittest + + +class TestFrameworkProto(unittest.TestCase): + def test_all(self): + op_proto_lib = paddle.v2.framework.proto.op_proto_pb2 + attr_type_lib = paddle.v2.framework.proto.attr_type_pb2 + op_proto = op_proto_lib.OpProto() + ipt0 = op_proto.inputs.add() + ipt0.name = "a" + ipt0.comment = "the input of cosine op" + ipt1 = op_proto.inputs.add() + ipt1.name = "b" + ipt1.comment = "the other input of cosine op" + opt = op_proto.outputs.add() + opt.name = "output" + opt.comment = "the output of cosine op" + op_proto.comment = "cosine op, output = scale*cos(a, b)" + attr = op_proto.attrs.add() + attr.name = "scale" + attr.comment = "scale of cosine op" + attr.type = attr_type_lib.FLOAT + op_proto.type = "cos" + self.assertTrue(op_proto.IsInitialized()) diff --git a/python/setup.py.in b/python/setup.py.in index dae0166487..78423614a6 100644 --- a/python/setup.py.in +++ b/python/setup.py.in @@ -9,7 +9,9 @@ packages=['paddle', 'paddle.v2.dataset', 'paddle.v2.reader', 'paddle.v2.master', - 'paddle.v2.plot'] + 'paddle.v2.plot', + 'paddle.v2.framework', + 'paddle.v2.framework.proto'] setup_requires=["requests", "numpy", @@ -29,6 +31,9 @@ setup(name='paddle', packages=packages, package_data={'paddle.v2.master': ['${paddle_master_LIB_NAME}'], }, package_dir={ - '': '${CMAKE_CURRENT_SOURCE_DIR}' + '': '${CMAKE_CURRENT_SOURCE_DIR}', + # The paddle.v2.framework.proto will be generated while compiling. + # So that package points to other directory. + 'paddle.v2.framework.proto': '${CMAKE_BINARY_DIR}/paddle/framework' }, ) From 847535f4fe6cea0b954a67fffea4c7b9ed96bd77 Mon Sep 17 00:00:00 2001 From: liaogang Date: Thu, 6 Jul 2017 15:42:29 +0800 Subject: [PATCH 29/48] FIX: propagation dependencies under linux --- cmake/generic.cmake | 69 +++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/cmake/generic.cmake b/cmake/generic.cmake index 1a4600ef4b..3900ea2604 100644 --- a/cmake/generic.cmake +++ b/cmake/generic.cmake @@ -103,38 +103,33 @@ function(merge_static_libs TARGET_NAME) foreach(lib ${libs}) list(APPEND libs_deps ${${lib}_LIB_DEPENDS}) endforeach() - list(REMOVE_DUPLICATES libs_deps) - # To produce a library we need at least one source file. - # It is created by add_custom_command below and will helps - # also help to track dependencies. - set(dummyfile ${CMAKE_CURRENT_BINARY_DIR}/${TARGET_NAME}_dummy.c) - - # Make the generated dummy source file depended on all static input - # libs. If input lib changes,the source file is touched - # which causes the desired effect (relink). - add_custom_command(OUTPUT ${dummyfile} - COMMAND ${CMAKE_COMMAND} -E touch ${dummyfile} - DEPENDS ${libs}) - - # Generate dummy staic lib - file(WRITE ${dummyfile} "const char * dummy = \"${dummyfile}\";") - add_library(${TARGET_NAME} STATIC ${dummyfile}) - target_link_libraries(${TARGET_NAME} ${libs_deps}) + if(APPLE) # Use OSX's libtool to merge archives + # To produce a library we need at least one source file. + # It is created by add_custom_command below and will helps + # also help to track dependencies. + set(dummyfile ${CMAKE_CURRENT_BINARY_DIR}/${TARGET_NAME}_dummy.c) - foreach(lib ${libs}) - # Get the file names of the libraries to be merged - set(libfiles ${libfiles} $) - endforeach() + # Make the generated dummy source file depended on all static input + # libs. If input lib changes,the source file is touched + # which causes the desired effect (relink). + add_custom_command(OUTPUT ${dummyfile} + COMMAND ${CMAKE_COMMAND} -E touch ${dummyfile} + DEPENDS ${libs}) - # Get the file name of the generated library - set(outlibfile "$") + # Generate dummy staic lib + file(WRITE ${dummyfile} "const char * dummy = \"${dummyfile}\";") + add_library(${TARGET_NAME} STATIC ${dummyfile}) + target_link_libraries(${TARGET_NAME} ${libs_deps}) - if(APPLE) # Use OSX's libtool to merge archives + foreach(lib ${libs}) + # Get the file names of the libraries to be merged + set(libfiles ${libfiles} $) + endforeach() add_custom_command(TARGET ${TARGET_NAME} POST_BUILD COMMAND rm "${CMAKE_CURRENT_BINARY_DIR}/lib${TARGET_NAME}.a" COMMAND /usr/bin/libtool -static -o "${CMAKE_CURRENT_BINARY_DIR}/lib${TARGET_NAME}.a" ${libfiles}) - else() # general UNIX: use "ar" to extract objects and re-add to a common lib + else() # general UNIX: use "ar" to extract objects and re-add to a common lib foreach(lib ${libs}) set(objlistfile ${lib}.objlist) # list of objects in the input library set(objdir ${lib}.objdir) @@ -149,13 +144,27 @@ function(merge_static_libs TARGET_NAME) DEPENDS ${lib} ${objdir} WORKING_DIRECTORY ${objdir}) - add_custom_command(TARGET ${TARGET_NAME} POST_BUILD - COMMAND ${CMAKE_AR} ru ${outlibfile} *.o - WORKING_DIRECTORY ${objdir}) + # Empty dummy source file that goes into merged library + set(mergebase ${lib}.mergebase.c) + add_custom_command(OUTPUT ${mergebase} + COMMAND ${CMAKE_COMMAND} -E touch ${mergebase} + DEPENDS ${objlistfile}) + + list(APPEND mergebases "${mergebase}") endforeach() - add_custom_command(TARGET ${TARGET_NAME} POST_BUILD - COMMAND ${CMAKE_RANLIB} ${outlibfile}) + add_library(${TARGET_NAME} STATIC ${mergebases}) + target_link_libraries(${TARGET_NAME} ${libs_deps}) + + # Get the file name of the generated library + set(outlibfile "$") + + foreach(lib ${libs}) + add_custom_command(TARGET ${TARGET_NAME} POST_BUILD + COMMAND ${CMAKE_AR} cr ${outlibfile} *.o + COMMAND ${CMAKE_RANLIB} ${outlibfile} + WORKING_DIRECTORY ${lib}.objdir) + endforeach() endif() endfunction(merge_static_libs) From 203364281ed8b86c53c520142b881f00aca5485e Mon Sep 17 00:00:00 2001 From: caoying03 Date: Thu, 6 Jul 2017 16:44:54 +0800 Subject: [PATCH 30/48] enable error clipping in FC layer. --- python/paddle/trainer/config_parser.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/python/paddle/trainer/config_parser.py b/python/paddle/trainer/config_parser.py index 370529ed97..e020be9378 100644 --- a/python/paddle/trainer/config_parser.py +++ b/python/paddle/trainer/config_parser.py @@ -13,6 +13,7 @@ # limitations under the License. from __future__ import print_function +import pdb ''' The following functions are available in the config file: @@ -761,8 +762,8 @@ class DotMulOperator(Operator): def check_dims(self): for i in range(2): - config_assert(self.operator_conf.input_sizes[i] == - self.operator_conf.output_size, + config_assert(self.operator_conf.input_sizes[ + i] == self.operator_conf.output_size, "DotMul input_size != output_size") def calc_output_size(self, input_sizes): @@ -1193,8 +1194,7 @@ def parse_image(image, input_layer_name, image_conf): def parse_norm(norm, input_layer_name, norm_conf): norm_conf.norm_type = norm.norm_type config_assert( - norm.norm_type in - ['rnorm', 'cmrnorm-projection', 'cross-channel-norm'], + norm.norm_type in ['rnorm', 'cmrnorm-projection', 'cross-channel-norm'], "norm-type %s is not in [rnorm, cmrnorm-projection, cross-channel-norm]" % norm.norm_type) norm_conf.channels = norm.channels @@ -1571,7 +1571,13 @@ class MultiClassCrossEntropySelfNormCostLayer(LayerBase): @config_layer('fc') class FCLayer(LayerBase): - def __init__(self, name, size, inputs, bias=True, **xargs): + def __init__(self, + name, + size, + inputs, + bias=True, + error_clipping_threshold=None, + **xargs): super(FCLayer, self).__init__(name, 'fc', size, inputs=inputs, **xargs) for input_index in xrange(len(self.inputs)): input_layer = self.get_input_layer(input_index) @@ -1589,6 +1595,9 @@ class FCLayer(LayerBase): format) self.create_bias_parameter(bias, self.config.size) + if error_clipping_threshold is not None: + self.config.error_clipping_threshold = error_clipping_threshold + @config_layer('selective_fc') class SelectiveFCLayer(LayerBase): @@ -3425,7 +3434,8 @@ DEFAULT_SETTING = dict( settings = copy.deepcopy(DEFAULT_SETTING) -settings_deprecated = dict(usage_ratio=1., ) +settings_deprecated = dict( + usage_ratio=1., ) trainer_settings = dict( save_dir="./output/model", From 075954c17ceaf422478961d9a5d6aaa364458415 Mon Sep 17 00:00:00 2001 From: caoying03 Date: Thu, 6 Jul 2017 17:40:58 +0800 Subject: [PATCH 31/48] follow comment. --- python/paddle/trainer/config_parser.py | 28 +++++++------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/python/paddle/trainer/config_parser.py b/python/paddle/trainer/config_parser.py index 1fed6db33c..826ba2834a 100644 --- a/python/paddle/trainer/config_parser.py +++ b/python/paddle/trainer/config_parser.py @@ -1353,7 +1353,8 @@ class LayerBase(object): device=None, active_type="", drop_rate=0., - coeff=None): + coeff=None, + error_clipping_threshold=None): config_assert('@' not in name, "layer name: %s contain special character @" % name) global g_current_submodel @@ -1387,6 +1388,9 @@ class LayerBase(object): elif g_default_device is not None: self.config.device = g_default_device + if error_clipping_threshold is not None: + self.config.error_clipping_threshold = error_clipping_threshold + for input_index in xrange(len(self.inputs)): input = self.inputs[input_index] input_config = None @@ -1571,13 +1575,7 @@ class MultiClassCrossEntropySelfNormCostLayer(LayerBase): @config_layer('fc') class FCLayer(LayerBase): - def __init__(self, - name, - size, - inputs, - bias=True, - error_clipping_threshold=None, - **xargs): + def __init__(self, name, size, inputs, bias=True, **xargs): super(FCLayer, self).__init__(name, 'fc', size, inputs=inputs, **xargs) for input_index in xrange(len(self.inputs)): input_layer = self.get_input_layer(input_index) @@ -1595,9 +1593,6 @@ class FCLayer(LayerBase): format) self.create_bias_parameter(bias, self.config.size) - if error_clipping_threshold is not None: - self.config.error_clipping_threshold = error_clipping_threshold - @config_layer('selective_fc') class SelectiveFCLayer(LayerBase): @@ -2791,13 +2786,7 @@ class TensorLayer(LayerBase): @config_layer('mixed') class MixedLayer(LayerBase): - def __init__(self, - name, - inputs, - size=0, - bias=True, - error_clipping_threshold=None, - **xargs): + def __init__(self, name, inputs, size=0, bias=True, **xargs): config_assert(inputs, 'inputs cannot be empty') super(MixedLayer, self).__init__( name, 'mixed', size, inputs=inputs, **xargs) @@ -2879,9 +2868,6 @@ class MixedLayer(LayerBase): self.config.bias_size = psize self.create_bias_parameter(bias, psize) - if error_clipping_threshold is not None: - self.config.error_clipping_threshold = error_clipping_threshold - # like MixedLayer, but no bias parameter @config_func From f2a82b16a25c2eb825ddb0a46b4966b01f248f22 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Thu, 6 Jul 2017 11:58:43 +0000 Subject: [PATCH 32/48] add print messages --- python/CMakeLists.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 361e764e25..7a57d922ef 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -17,15 +17,21 @@ add_custom_target(copy_paddle_master) SET(COPY_PADDLE_MASTER "") if(WITH_GOLANG) SET(COPY_PADDLE_MASTER "copy_paddle_master") + message("paddle_master_lib_path:" ${paddle_master_LIB_PATH}) + message("PROJ_ROOT:" ${PROJ_ROOT}) add_custom_command(TARGET ${COPY_PADDLE_MASTER} COMMAND cp ${paddle_master_LIB_PATH} ${PROJ_ROOT}/python/paddle/v2/master/ ) add_dependencies(copy_paddle_master paddle_master) endif(WITH_GOLANG) +message("paddle_master_LIB_NAME:" ${paddle_master_LIB_NAME}) +message("CMAKE_CURRENT_BINARY_DIR:" ${CMAKE_CURRENT_BINARY_DIR}) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/setup.py.in ${CMAKE_CURRENT_BINARY_DIR}/setup.py) +message("OUTPUT_DIR:" ${OUTPUT_DIR}) +message("py_env:" ${py_env}) add_custom_command(OUTPUT ${OUTPUT_DIR}/.timestamp COMMAND env ${py_env} ${PYTHON_EXECUTABLE} setup.py bdist_wheel COMMAND ${CMAKE_COMMAND} -E touch ${OUTPUT_DIR}/.timestamp From 660475b5ab1c6cc295420a527d549dc1f38ba03a Mon Sep 17 00:00:00 2001 From: gongweibao Date: Thu, 6 Jul 2017 12:14:30 +0000 Subject: [PATCH 33/48] modify to add paddle_master name --- python/CMakeLists.txt | 1 + python/setup.py.in | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 7a57d922ef..633d2b3786 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -27,6 +27,7 @@ endif(WITH_GOLANG) message("paddle_master_LIB_NAME:" ${paddle_master_LIB_NAME}) message("CMAKE_CURRENT_BINARY_DIR:" ${CMAKE_CURRENT_BINARY_DIR}) +message("CMAKE_CURRENT_SOURCE_DIR:" ${CMAKE_CURRENT_SOURCE_DIR}) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/setup.py.in ${CMAKE_CURRENT_BINARY_DIR}/setup.py) diff --git a/python/setup.py.in b/python/setup.py.in index dae0166487..9c77bed15f 100644 --- a/python/setup.py.in +++ b/python/setup.py.in @@ -27,7 +27,7 @@ setup(name='paddle', description='Parallel Distributed Deep Learning', install_requires=setup_requires, packages=packages, - package_data={'paddle.v2.master': ['${paddle_master_LIB_NAME}'], }, + package_data={'paddle.v2.master': ['libpaddle_master.so'], }, package_dir={ '': '${CMAKE_CURRENT_SOURCE_DIR}' }, From b396055499c5bd34bea5753e7ca19e18e2f7044b Mon Sep 17 00:00:00 2001 From: gongweibao Date: Thu, 6 Jul 2017 13:34:40 +0000 Subject: [PATCH 34/48] add -V --- paddle/scripts/docker/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paddle/scripts/docker/build.sh b/paddle/scripts/docker/build.sh index ab60f1a38d..0579bfcc7a 100644 --- a/paddle/scripts/docker/build.sh +++ b/paddle/scripts/docker/build.sh @@ -60,7 +60,7 @@ EOF make -j `nproc` if [ ${WITH_TESTING:-OFF} == "ON" ] && [ ${RUN_TEST:-OFF} == "ON" ] ; then pip uninstall -y py-paddle paddle || true - ctest --output-on-failure + ctest -V --output-on-failure fi From 4daa247d80a3f94b8f60fe084bd3887b4b5c698e Mon Sep 17 00:00:00 2001 From: gongweibao Date: Fri, 7 Jul 2017 01:12:48 +0000 Subject: [PATCH 35/48] rm -v --- paddle/scripts/docker/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paddle/scripts/docker/build.sh b/paddle/scripts/docker/build.sh index 0579bfcc7a..ab60f1a38d 100644 --- a/paddle/scripts/docker/build.sh +++ b/paddle/scripts/docker/build.sh @@ -60,7 +60,7 @@ EOF make -j `nproc` if [ ${WITH_TESTING:-OFF} == "ON" ] && [ ${RUN_TEST:-OFF} == "ON" ] ; then pip uninstall -y py-paddle paddle || true - ctest -V --output-on-failure + ctest --output-on-failure fi From 126e64fc830ba5b787a787fdd2e2b7f7e2ef1939 Mon Sep 17 00:00:00 2001 From: gongweibao Date: Fri, 7 Jul 2017 01:35:16 +0000 Subject: [PATCH 36/48] add cmake --- python/CMakeLists.txt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/python/CMakeLists.txt b/python/CMakeLists.txt index 633d2b3786..361e764e25 100644 --- a/python/CMakeLists.txt +++ b/python/CMakeLists.txt @@ -17,22 +17,15 @@ add_custom_target(copy_paddle_master) SET(COPY_PADDLE_MASTER "") if(WITH_GOLANG) SET(COPY_PADDLE_MASTER "copy_paddle_master") - message("paddle_master_lib_path:" ${paddle_master_LIB_PATH}) - message("PROJ_ROOT:" ${PROJ_ROOT}) add_custom_command(TARGET ${COPY_PADDLE_MASTER} COMMAND cp ${paddle_master_LIB_PATH} ${PROJ_ROOT}/python/paddle/v2/master/ ) add_dependencies(copy_paddle_master paddle_master) endif(WITH_GOLANG) -message("paddle_master_LIB_NAME:" ${paddle_master_LIB_NAME}) -message("CMAKE_CURRENT_BINARY_DIR:" ${CMAKE_CURRENT_BINARY_DIR}) -message("CMAKE_CURRENT_SOURCE_DIR:" ${CMAKE_CURRENT_SOURCE_DIR}) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/setup.py.in ${CMAKE_CURRENT_BINARY_DIR}/setup.py) -message("OUTPUT_DIR:" ${OUTPUT_DIR}) -message("py_env:" ${py_env}) add_custom_command(OUTPUT ${OUTPUT_DIR}/.timestamp COMMAND env ${py_env} ${PYTHON_EXECUTABLE} setup.py bdist_wheel COMMAND ${CMAKE_COMMAND} -E touch ${OUTPUT_DIR}/.timestamp From c78f41a331ddc181f98e5885f0aa64c29acb8182 Mon Sep 17 00:00:00 2001 From: liaogang Date: Fri, 7 Jul 2017 11:53:29 +0800 Subject: [PATCH 37/48] FIX: explicitly specify glog install path --- cmake/external/glog.cmake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmake/external/glog.cmake b/cmake/external/glog.cmake index b70e94a170..bd401faa6e 100644 --- a/cmake/external/glog.cmake +++ b/cmake/external/glog.cmake @@ -38,12 +38,14 @@ ExternalProject_Add( CMAKE_ARGS -DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS} CMAKE_ARGS -DCMAKE_C_FLAGS=${CMAKE_C_FLAGS} CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${GLOG_INSTALL_DIR} + CMAKE_ARGS -DCMAKE_INSTALL_LIBDIR=${GLOG_INSTALL_DIR}/lib CMAKE_ARGS -DCMAKE_POSITION_INDEPENDENT_CODE=ON CMAKE_ARGS -DWITH_GFLAGS=ON CMAKE_ARGS -Dgflags_DIR=${GFLAGS_INSTALL_DIR}/lib/cmake/gflags CMAKE_ARGS -DBUILD_TESTING=OFF CMAKE_ARGS -DCMAKE_BUILD_TYPE=Release CMAKE_CACHE_ARGS -DCMAKE_INSTALL_PREFIX:PATH=${GLOG_INSTALL_DIR} + -DCMAKE_INSTALL_LIBDIR:PATH=${GLOG_INSTALL_DIR}/lib -DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=ON -DCMAKE_BUILD_TYPE:STRING=Release ) From 1d2ef1db82136de8817229252774a797323f8eac Mon Sep 17 00:00:00 2001 From: fengjiayi Date: Fri, 7 Jul 2017 14:19:47 +0800 Subject: [PATCH 38/48] [draft] add registry for Op, OpProto and OpAttrChecker (#2739) * init op_registry.h * dev op_registry.h * add 'attr_checker.h', which is a draft of op attribute checker. * rename some macro parameters * 1. Use `Attribute` and `AttributeMap` instead of `OpDesc`. `AttributeMap` is a unordered_map of , and `Attribute` is a boost::variant object to hold multiple types of attribute value. 2. Use `PADDLE_ENFORCE` to print checkers' fail message. 3. Abstract default value operations to a new function: `DefaultChecker`. * rename DefaultChecker to DefaultValueSetter ZZ * Finish op_registry 1. Complete the development of interfaces between OpRegistry and Protobuf. 2. Add unit test for op_registry.h * Add demo and test of custome checker * fix merge conflict --- paddle/framework/CMakeLists.txt | 1 + paddle/framework/attr_checker.h | 119 +++++++++++++ paddle/framework/op_registry.h | 253 +++++++++++++++++++++++++++ paddle/framework/op_registry_test.cc | 122 +++++++++++++ 4 files changed, 495 insertions(+) create mode 100644 paddle/framework/attr_checker.h create mode 100644 paddle/framework/op_registry.h create mode 100644 paddle/framework/op_registry_test.cc diff --git a/paddle/framework/CMakeLists.txt b/paddle/framework/CMakeLists.txt index 970b2b9abd..4409c6feae 100644 --- a/paddle/framework/CMakeLists.txt +++ b/paddle/framework/CMakeLists.txt @@ -11,6 +11,7 @@ proto_library(op_proto SRCS op_proto.proto DEPS attr_type) cc_test(op_proto_test SRCS op_proto_test.cc DEPS op_proto protobuf) proto_library(op_desc SRCS op_desc.proto DEPS attr_type) cc_test(op_desc_test SRCS op_desc_test.cc DEPS op_desc protobuf) +cc_test(op_registry_test SRCS op_registry_test.cc DEPS op_proto op_desc) py_proto_compile(framework_py_proto SRCS attr_type.proto op_proto.proto op_desc.proto) # Generate an empty __init__.py to make framework_py_proto as a valid python module. add_custom_target(framework_py_proto_init ALL COMMAND ${CMAKE_COMMAND} -E touch __init__.py) diff --git a/paddle/framework/attr_checker.h b/paddle/framework/attr_checker.h new file mode 100644 index 0000000000..c0c33d8114 --- /dev/null +++ b/paddle/framework/attr_checker.h @@ -0,0 +1,119 @@ +#pragma once + +#include +#include +#include +#include +#include +#include "paddle/framework/enforce.h" + +namespace paddle { +namespace framework { + +typedef boost::variant, + std::vector, std::vector> + Attribute; +typedef std::unordered_map AttributeMap; + +// check whether a value(attribute) fit a certain limit +template +class LargerThanChecker { + public: + LargerThanChecker(T lower_bound) : lower_bound_(lower_bound) {} + void operator()(T& value) const { + PADDLE_ENFORCE(value > lower_bound_, "larger_than check fail"); + } + + private: + T lower_bound_; +}; + +// we can provide users more common Checker, like 'LessThanChecker', +// 'BetweenChecker'... + +template +class DefaultValueSetter { + public: + DefaultValueSetter(T default_value) : default_value_(default_value) {} + void operator()(T& value) const { value = default_value_; } + + private: + T default_value_; +}; + +// check whether a certain attribute fit its limits +// an attribute can have more than one limits +template +class TypedAttrChecker { + typedef std::function ValueChecker; + + public: + TypedAttrChecker(const std::string& attr_name) : attr_name_(attr_name) {} + + TypedAttrChecker& LargerThan(const T& lower_bound) { + value_checkers_.push_back(LargerThanChecker(lower_bound)); + return *this; + } + + // we can add more common limits, like LessThan(), Between()... + + TypedAttrChecker& SetDefault(const T& default_value) { + PADDLE_ENFORCE(default_value_setter_.empty(), + "%s can't have more than one default value!", attr_name_); + default_value_setter_.push_back(DefaultValueSetter(default_value)); + return *this; + } + + // allow users provide their own checker + TypedAttrChecker& AddCustomChecker(const ValueChecker& checker) { + value_checkers_.push_back(checker); + return *this; + } + + void operator()(AttributeMap& attr_map) const { + if (!attr_map.count(attr_name_)) { + // user do not set this attr + PADDLE_ENFORCE(!default_value_setter_.empty(), + "Attribute '%s' is required!", attr_name_); + // default_value_setter_ has no more than one element + T val; + (default_value_setter_[0])(val); + attr_map[attr_name_] = val; + } + Attribute& attr = attr_map.at(attr_name_); + T& attr_value = boost::get(attr); + for (const auto& checker : value_checkers_) { + checker(attr_value); + } + } + + private: + std::string attr_name_; + std::vector value_checkers_; + std::vector default_value_setter_; +}; + +// check whether op's all attributes fit their own limits +class OpAttrChecker { + typedef std::function AttrChecker; + + public: + template + TypedAttrChecker& AddAttrChecker(const std::string& attr_name) { + attr_checkers_.push_back(TypedAttrChecker(attr_name)); + AttrChecker& checker = attr_checkers_.back(); + return *(checker.target>()); + } + + void Check(AttributeMap& attr_map) const { + for (const auto& checker : attr_checkers_) { + checker(attr_map); + } + } + + private: + std::vector attr_checkers_; +}; + +} // namespace framework +} // namespace paddle diff --git a/paddle/framework/op_registry.h b/paddle/framework/op_registry.h new file mode 100644 index 0000000000..81241b5342 --- /dev/null +++ b/paddle/framework/op_registry.h @@ -0,0 +1,253 @@ +#pragma once + +#include "paddle/framework/attr_checker.h" + +//#include "paddle/framework/op_base.h" +#include "paddle/framework/op_desc.pb.h" +#include "paddle/framework/op_proto.pb.h" + +namespace paddle { +namespace framework { + +//==================For test================// +class OpBase { + public: + std::vector inputs_; + std::vector outputs_; + AttributeMap attr_map_; + + virtual std::string Run() const = 0; + virtual ~OpBase() {} +}; +//=========================================// + +// helper class to set attribute type +struct AttrTypeHelper { + template + static void SetAttrType(AttrProto* attr); + + static Attribute GetAttrValue(const AttrDesc& attr_desc) { + switch (attr_desc.type()) { + case paddle::framework::AttrType::INT: { + return attr_desc.i(); + } + case paddle::framework::AttrType::FLOAT: { + return attr_desc.f(); + } + case paddle::framework::AttrType::STRING: { + return attr_desc.s(); + } + case paddle::framework::AttrType::INTS: { + std::vector val(attr_desc.ints_size()); + for (int i = 0; i < attr_desc.ints_size(); ++i) { + val[i] = attr_desc.ints(i); + } + return val; + } + case paddle::framework::AttrType::FLOATS: { + std::vector val(attr_desc.floats_size()); + for (int i = 0; i < attr_desc.floats_size(); ++i) { + val[i] = attr_desc.floats(i); + } + return val; + } + case paddle::framework::AttrType::STRINGS: { + std::vector val(attr_desc.strings_size()); + for (int i = 0; i < attr_desc.strings_size(); ++i) { + val[i] = attr_desc.strings(i); + } + return val; + } + } + PADDLE_ENFORCE(false, "Unknown OpDesc::AttrDesc::type !"); + return boost::blank(); + } +}; + +template <> +void AttrTypeHelper::SetAttrType(AttrProto* attr) { + attr->set_type(paddle::framework::AttrType::INT); +} + +template <> +void AttrTypeHelper::SetAttrType(AttrProto* attr) { + attr->set_type(paddle::framework::AttrType::FLOAT); +} + +template <> +void AttrTypeHelper::SetAttrType(AttrProto* attr) { + attr->set_type(paddle::framework::AttrType::STRING); +} + +template <> +void AttrTypeHelper::SetAttrType>(AttrProto* attr) { + attr->set_type(paddle::framework::AttrType::INTS); +} + +template <> +void AttrTypeHelper::SetAttrType>(AttrProto* attr) { + attr->set_type(paddle::framework::AttrType::FLOATS); +} + +template <> +void AttrTypeHelper::SetAttrType>(AttrProto* attr) { + attr->set_type(paddle::framework::AttrType::STRINGS); +} + +// this class not only make proto but also init attribute checkers. +class OpProtoAndCheckerMaker { + public: + OpProtoAndCheckerMaker(OpProto* proto, OpAttrChecker* op_checker) + : proto_(proto), op_checker_(op_checker) {} + + protected: + void AddInput(const std::string& name, const std::string& comment) { + auto input = proto_->mutable_inputs()->Add(); + *(input->mutable_name()) = name; + *(input->mutable_comment()) = comment; + } + + void AddOutput(const std::string& name, const std::string& comment) { + auto output = proto_->mutable_outputs()->Add(); + *(output->mutable_name()) = name; + *(output->mutable_comment()) = comment; + } + + template + TypedAttrChecker& AddAttr(const std::string& name, + const std::string& comment) { + auto attr = proto_->mutable_attrs()->Add(); + *(attr->mutable_name()) = name; + *(attr->mutable_comment()) = comment; + AttrTypeHelper::SetAttrType(attr); + return op_checker_->AddAttrChecker(name); + } + + void AddType(const std::string& op_type) { proto_->set_type(op_type); } + + void AddComment(const std::string& comment) { + *(proto_->mutable_comment()) = comment; + } + + OpProto* proto_; + OpAttrChecker* op_checker_; +}; + +class OpRegistry { + typedef std::function OpCreator; + + public: + template + static void RegisterOp(const std::string& op_type) { + creators_[op_type] = []() { return new OpType; }; + OpProto& op_proto = protos_[op_type]; + OpAttrChecker& op_checker = op_checkers_[op_type]; + ProtoMakerType(&op_proto, &op_checker); + PADDLE_ENFORCE(op_proto.IsInitialized() == true, + "Fail to initialize %s's OpProto !", op_type); + } + + static OpBase* CreateOp(const OpDesc& op_desc) { + std::string op_type = op_desc.type(); + OpBase* op = (creators_.at(op_type))(); + (op->inputs_).resize(op_desc.inputs_size()); + for (int i = 0; i < op_desc.inputs_size(); ++i) { + (op->inputs_)[i] = op_desc.inputs(i); + } + (op->outputs_).resize(op_desc.outputs_size()); + for (int i = 0; i < op_desc.outputs_size(); ++i) { + (op->outputs_)[i] = op_desc.outputs(i); + } + for (int i = 0; i < op_desc.attrs_size(); ++i) { + const AttrDesc& ith_attr = op_desc.attrs(i); + std::string name = ith_attr.name(); + (op->attr_map_)[name] = AttrTypeHelper::GetAttrValue(ith_attr); + } + const OpAttrChecker& op_checker = op_checkers_.at(op_type); + op_checker.Check(op->attr_map_); + return op; + } + + private: + static std::unordered_map creators_; + static std::unordered_map protos_; + static std::unordered_map op_checkers_; +}; + +std::unordered_map> OpRegistry::creators_; +std::unordered_map OpRegistry::protos_; +std::unordered_map OpRegistry::op_checkers_; + +template +class OpRegisterHelper { + public: + OpRegisterHelper(std::string op_type) { + OpRegistry::RegisterOp(op_type); + } +}; + +#define REGISTER_OP(__op_class, __op_maker_class, __op_type) \ + class __op_class##Register { \ + private: \ + const static OpRegisterHelper<__op_class, __op_maker_class> reg; \ + }; \ + const OpRegisterHelper<__op_class, __op_maker_class> \ + __op_class##Register::reg(#__op_type); + +// Demos + +class CosineOp : public OpBase { + public: + virtual std::string Run() const { + std::string msg = "CosineOp runs! scale = " + + std::to_string(boost::get(attr_map_.at("scale"))); + return msg; + } +}; + +class CosineOpProtoAndCheckerMaker : public OpProtoAndCheckerMaker { + public: + CosineOpProtoAndCheckerMaker(OpProto* proto, OpAttrChecker* op_checker) + : OpProtoAndCheckerMaker(proto, op_checker) { + AddInput("input", "input of cosine op"); + AddOutput("output", "output of cosine op"); + AddAttr("scale", "scale of cosine op") + .SetDefault(1.0) + .LargerThan(0.0); + AddType("cos"); + AddComment("This is cos op"); + } +}; + +REGISTER_OP(CosineOp, CosineOpProtoAndCheckerMaker, cos_sim) + +class MyTestOp : public OpBase { + public: + virtual std::string Run() const { + std::string msg = + "MyTestOp runs! test_attr = " + + std::to_string(boost::get(attr_map_.at("test_attr"))); + return msg; + } +}; + +class MyTestOpProtoAndCheckerMaker : public OpProtoAndCheckerMaker { + public: + MyTestOpProtoAndCheckerMaker(OpProto* proto, OpAttrChecker* op_checker) + : OpProtoAndCheckerMaker(proto, op_checker) { + AddInput("input", "input of cosine op"); + AddOutput("output", "output of cosine op"); + auto my_checker = [](int i) { + PADDLE_ENFORCE(i % 2 == 0, "'test_attr' must be even!"); + }; + AddAttr("test_attr", "a simple test attribute") + .AddCustomChecker(my_checker); + AddType("my_test_op"); + AddComment("This is my_test op"); + } +}; + +REGISTER_OP(MyTestOp, MyTestOpProtoAndCheckerMaker, my_test_op) + +} // namespace framework +} // namespace paddle diff --git a/paddle/framework/op_registry_test.cc b/paddle/framework/op_registry_test.cc new file mode 100644 index 0000000000..17849ca019 --- /dev/null +++ b/paddle/framework/op_registry_test.cc @@ -0,0 +1,122 @@ +#include "paddle/framework/op_registry.h" +#include + +TEST(OpRegistry, CreateOp) { + paddle::framework::OpDesc op_desc; + op_desc.set_type("cos_sim"); + op_desc.add_inputs("aa"); + op_desc.add_outputs("bb"); + + auto attr = op_desc.mutable_attrs()->Add(); + attr->set_name("scale"); + attr->set_type(paddle::framework::AttrType::FLOAT); + attr->set_f(3.3); + + paddle::framework::OpBase* op = + paddle::framework::OpRegistry::CreateOp(op_desc); + std::string debug_str = op->Run(); + std::string str = "CosineOp runs! scale = " + std::to_string(3.3); + ASSERT_EQ(str.size(), debug_str.size()); + for (size_t i = 0; i < debug_str.length(); ++i) { + ASSERT_EQ(debug_str[i], str[i]); + } +} + +TEST(OpRegistry, IllegalAttr) { + paddle::framework::OpDesc op_desc; + op_desc.set_type("cos_sim"); + op_desc.add_inputs("aa"); + op_desc.add_outputs("bb"); + + auto attr = op_desc.mutable_attrs()->Add(); + attr->set_name("scale"); + attr->set_type(paddle::framework::AttrType::FLOAT); + attr->set_f(-2.0); + + bool caught = false; + try { + paddle::framework::OpBase* op __attribute__((unused)) = + paddle::framework::OpRegistry::CreateOp(op_desc); + } catch (paddle::framework::EnforceNotMet err) { + caught = true; + std::string msg = "larger_than check fail"; + const char* err_msg = err.what(); + for (size_t i = 0; i < msg.length(); ++i) { + ASSERT_EQ(err_msg[i], msg[i]); + } + } + ASSERT_TRUE(caught); +} + +TEST(OpRegistry, DefaultValue) { + paddle::framework::OpDesc op_desc; + op_desc.set_type("cos_sim"); + op_desc.add_inputs("aa"); + op_desc.add_outputs("bb"); + + paddle::framework::OpBase* op = + paddle::framework::OpRegistry::CreateOp(op_desc); + std::string debug_str = op->Run(); + float default_value = 1.0; + std::string str = "CosineOp runs! scale = " + std::to_string(default_value); + ASSERT_EQ(str.size(), debug_str.size()); + for (size_t i = 0; i < debug_str.length(); ++i) { + ASSERT_EQ(debug_str[i], str[i]); + } +} + +TEST(OpRegistry, CustomChecker) { + paddle::framework::OpDesc op_desc; + op_desc.set_type("my_test_op"); + op_desc.add_inputs("ii"); + op_desc.add_outputs("oo"); + + // attr 'test_attr' is not set + bool caught = false; + try { + paddle::framework::OpBase* op __attribute__((unused)) = + paddle::framework::OpRegistry::CreateOp(op_desc); + } catch (paddle::framework::EnforceNotMet err) { + caught = true; + std::string msg = "Attribute 'test_attr' is required!"; + const char* err_msg = err.what(); + for (size_t i = 0; i < msg.length(); ++i) { + ASSERT_EQ(err_msg[i], msg[i]); + } + } + ASSERT_TRUE(caught); + + // set 'test_attr' set to an illegal value + auto attr = op_desc.mutable_attrs()->Add(); + attr->set_name("test_attr"); + attr->set_type(paddle::framework::AttrType::INT); + attr->set_i(3); + caught = false; + try { + paddle::framework::OpBase* op __attribute__((unused)) = + paddle::framework::OpRegistry::CreateOp(op_desc); + } catch (paddle::framework::EnforceNotMet err) { + caught = true; + std::string msg = "'test_attr' must be even!"; + const char* err_msg = err.what(); + for (size_t i = 0; i < msg.length(); ++i) { + ASSERT_EQ(err_msg[i], msg[i]); + } + } + ASSERT_TRUE(caught); + + // set 'test_attr' set to a legal value + op_desc.mutable_attrs()->Clear(); + attr = op_desc.mutable_attrs()->Add(); + attr->set_name("test_attr"); + attr->set_type(paddle::framework::AttrType::INT); + attr->set_i(4); + paddle::framework::OpBase* op = + paddle::framework::OpRegistry::CreateOp(op_desc); + std::string debug_str = op->Run(); + std::string str = "MyTestOp runs! test_attr = " + std::to_string(4); + ASSERT_EQ(str.size(), debug_str.size()); + for (size_t i = 0; i < debug_str.length(); ++i) { + ASSERT_EQ(debug_str[i], str[i]); + } +} \ No newline at end of file From 50e29bac38f485dca831b62ddcc40da2f38521ff Mon Sep 17 00:00:00 2001 From: Luo Tao Date: Fri, 7 Jul 2017 15:39:00 +0800 Subject: [PATCH 39/48] mistaken: Folk -> Fork in develop branch --- doc_theme/templates/layout.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc_theme/templates/layout.html b/doc_theme/templates/layout.html index 65e61c5f29..9fca69dc4e 100644 --- a/doc_theme/templates/layout.html +++ b/doc_theme/templates/layout.html @@ -101,7 +101,7 @@