Implement distributed training save model, improve master.NewClient i… #2973
Conversation
helinwang
force-pushed
the
save_model_1
branch
2 times, most recently
from
c90c085
to
a69230b
Compare
go/master/c/client.go
Outdated
| } | ||
|
|
||
| if need { | ||
| return 1 |
There was a problem hiding this comment.
should we define const status code instead of numbers? even though this status needs to pass across language border, numbers always seem tricky.
| @@ -142,7 +142,7 @@ func NewService(idx int, interval time.Duration, path string, client *EtcdClient | |||
| } | |||
|
|
|||
| // InitParam initializes a parameter. | |||
| func (s *Service) InitParam(paramWithConfigs ParameterWithConfig, dummy *int) error { | |||
| func (s *Service) InitParam(paramWithConfigs ParameterWithConfig, _ *int) error { | |||
There was a problem hiding this comment.
cool, golang support anonymous parameter in RPC Call.
| path = os.path.join(path, trainer_id) | ||
| path = os.path.join(path, "model.tar") | ||
|
|
||
| mkdir_p(path) |
There was a problem hiding this comment.
since mkdir_p is used once by save model, can we put it into here as an inner function?
There was a problem hiding this comment.
@dzhwinter I am not familiar with Python, in Go typically this kind of function is placed like this (top level inside the file) to reduce indentation. Just curious why in Python inner function is preferred, is it because every function is public in Python?
There was a problem hiding this comment.
No, the inner function is not the Pythonic way because more indentation is needed, only closure and factory function as an inner function is preferred.
I thought that treat system command like mkdir as public function may be not good in paddle cloud.
But as your comment mentions that, put it at the top level inside the file will reduce indentation. Both of them is fine to me.
There was a problem hiding this comment.
I see, thanks @dzhwinter , let's keep it this way since there is no real "public" or "private" function in Python (unless use inline function as you mentioned). Typically "public" is declared (but Python does not enforce ones that are not declared private) by __all__ = ["save_model", "load_model"], which is already in this file.
go/master/c/client.go
Outdated
| // paddle_request_save_model requests the master server to approve the | ||
| // caller to save the model. | ||
| // | ||
| // returns 1 if the save the model request is approved, 0 if does the |
…nterface
Fixes: #2512
Sorry this has not been tested yet, but I think it's ready to review.