Implement master client for reading training tasks #2468
Conversation
helinwang
requested review from
Yancey1989,
wangkuiyi,
jacquesqiao,
gongweibao,
typhoonzero and
dzhwinter
go/master/client.go
Outdated
| if err != nil { | ||
| // TODO(helin): wait before move on with next | ||
| // getTask call. | ||
| log.Println(err) |
There was a problem hiding this comment.
Can we all use glog instead of log, so we can do glog.Errorf or glog.Infof to separate info log and error log. Use glog.V(10).Infof to log more detailed debug logs
There was a problem hiding this comment.
Good idea! glog does not work with flag package other than official "flag", can we use https://github.com/sirupsen/logrus instead?
There was a problem hiding this comment.
Nice lib! This lib seems also able to send logs to "logstash" in json format, this is very useful when run jobs on kubernetes, we can collect logs and search logs using "EFK"
There was a problem hiding this comment.
@typhoonzero Thanks for mentioning logstash and EFK, good to know about them!
|
|
||
| s := recordio.NewRangeScanner(f, &chunk.Index, -1, -1) | ||
| for s.Scan() { | ||
| c.ch <- s.Record() |
There was a problem hiding this comment.
This line will block util some reader read the record from this channel. How to deal with "batches" like paddle.dataset.batch(some_reader(), 32)? Will adding channel buffers like c.ch = make(chan []byte, batch_size) increase the efficiency?
There was a problem hiding this comment.
We can use paddle.reader.buffered for this purpose. Still I think that's a good point. Will add.
go/master/python/setup.py
Outdated
| @@ -0,0 +1,19 @@ | |||
| from setuptools import setup, Distribution | |||
There was a problem hiding this comment.
Why we need to add a single python package to install master_client?
There was a problem hiding this comment.
I am not familiar with Python packaging, can you let me know what's the better way to do it? I will make the change. Thanks!
There was a problem hiding this comment.
Thinking about it again, I am planning to put it together within paddle Python package, integrated with paddle's CMake.
helinwang
force-pushed
the
master_dispatch
branch
3 times, most recently
from
4ce639e to
cadb481
Compare
helinwang
force-pushed
the
master_dispatch
branch
from
0b437c2 to
8742441
Compare
| 'plot', | ||
| 'evaluator', | ||
| 'image', | ||
| 'master', |
There was a problem hiding this comment.
The python module master stands for "master client" or "job master client". Using name "master" may be confused for someone reading the code.
There was a problem hiding this comment.
Because when user use the package, it will be
import paddle.v2 as paddle
client = paddle.master.client("localhost:3000", 30)
record = client.NextRecord()I was thinking since there is a client after package name (paddle.master.client), maybe it's clear that it means master's client?
I am actually not very familiar with Python's naming convention, do you have any suggestion?
There was a problem hiding this comment.
Cool, I see. Usage like paddle.master.client is definitely fine.
No description provided.