golang pserver use OptimizerConfig.proto #3358
Conversation
| // NOTE: convert V1 OptimizatioinConfig proto to OptimizerConfig if | ||
| // OptimizerConfig not set. This makes golang pserver compatible with | ||
| // handy V1 demos. Support SGD only. | ||
| // TODO: Remove these lines when golang pserver is production ready |
There was a problem hiding this comment.
we really need to simplify this mess in the future. sgdConfigV2 , optimizerConfigNew_ , almost a nightmare to maintain.
python/paddle/v2/optimizer.py
Outdated
|
|
||
| # create new optimizer with proto config, add new optimizer here | ||
| self.__opt_conf_new__ = OptimizerConfig() | ||
| dict_to_protobuf(kwargs, self.__opt_conf_new__) |
There was a problem hiding this comment.
Is the manually parsed optimizer configuration protobuf message human readable?
how about replacing them with built-in python API text_format?
https://stackoverflow.com/questions/33557965/print-human-friendly-protobuf-message
It can be printed in a pretty format, facilitate the future debug.
| for (int i = 0; i < parameterSize(); ++i) { | ||
| OptimizerConfig optConfigPerParameter; |
There was a problem hiding this comment.
Sorry that you have to write this...
It's hard to maintain two version of the same thing. We added config V2 because V1 is a giant proto file which contains everything, which is too hard to understand and blocks the pserver optimizer implementation.
| @@ -31,6 +31,13 @@ def main(): | |||
|
|
|||
| # create optimizer of new remote updater to pserver | |||
| optimizer = paddle.optimizer.Momentum(momentum=0, learning_rate=1e-3) | |||
| optimizer.set_remote_optimizer_config( | |||
There was a problem hiding this comment.
I think we should not add a new API to the Python interface. The new API seems created because there are some implementation difficulties that we have. But from the user's perspective, a single optimizer concept seems to suffice. We probably should not introduce two concepts: remote optimizer and local optimizer to the user.
Curious how hard would it be if we keep the current Python interface, but convert the parsed V1 proto to V2 proto internally? (seems you have already written most of the code to do so).
There was a problem hiding this comment.
Well, converting V1 config to V2 may lose some of the configuration fields, but it's still possible. We always intend to keep the user interface simple, will add the conversions.
But I think we still should keep set_remote_optimizer_config or rename to set_optimizer_config for detailed optimizer configs.
There was a problem hiding this comment.
In OptimizerConfig, each "parameter" object have a single configuration details, each "parameter" can have different algorithm like "SGD" or "Adagrad".
In V1 TrainerConfig.proto configs the global optimization algorithm for all parameters, and in ParameterConfig.proto set config for each "parameter", but only learning_rate, decay_rate and momentum values.
There was a problem hiding this comment.
I see, thanks for clarifying! Now I understand that V1 proto is not as capable as V2 proto.
Sorry for insisting on this, but I think we should not have an graph configuration API that work only for cloud training but not for local training. I am open for discussion, but I think the graph configuration API needs to be consistent: works both locally and on cloud.
Maybe when the times come that we need the capability of v2 proto (e.g., different optimizer on different parameters), we should make it work locally as well.
There was a problem hiding this comment.
- Does "graph configuration" means the V2 proto configuration python API?
- V2 config python API only work when using the new pserver. I think we can just remove the python API for just less confusing, and in most cases people always config a "global" optimizer algorithm for all parameters(layers), and do specific configuration for only some of the parameters.
Do you agree with removing python API only?
There was a problem hiding this comment.
Sorry by V2 proto I meant OptimizerConfig.proto, V1 proto I meant TrainerConfig.proto and ParameterConfig.proto.
- I meant V2 API that we released earlier this year, not the V2 parameter config (OptimizerConfig.proto) .
- Just for clarity, I think the V2 API work with the old parameter server as well. Yes, OptimizerConfig.proto only works for the new pserver. (Sorry too much different V2 here...)
Yes agree on removing Python API only, for less confusion.
… new_pserver_updater_proto
| parameterClient_ = | ||
| paddle_new_etcd_pserver_client((char *)pserverSpec_.c_str()); | ||
| parameterClient_ = paddle_new_etcd_pserver_client( | ||
| (char *)pserverSpec_.c_str(), FLAGS_trainer_id == 0); |
There was a problem hiding this comment.
Can you rebase / pull with latest develop? paddle_new_etcd_pserver_client now only takes one argument.
python/paddle/v2/optimizer.py
Outdated
| __all__ = [ | ||
| 'Momentum', 'Adam', 'Adamax', 'AdaGrad', 'DecayedAdaGrad', 'AdaDelta', | ||
| 'RMSProp', 'ModelAverage', 'L2Regularization' | ||
| 'RMSProp', 'ModelAverage', 'L2Regularization', 'OptimizerConfig' |
There was a problem hiding this comment.
Is the import and OptimizerConfig in this line still necessary?
python/paddle/v2/optimizer.py
Outdated
| @@ -27,6 +40,7 @@ def __impl__(): | |||
| __impl__) | |||
| self.__opt_conf__ = swig_api.OptimizationConfig.createFromProto( | |||
| self.__opt_conf_proto__) | |||
| self.__opt_conf_new__ = None | |||
There was a problem hiding this comment.
Is this variable still necessary?
… new_pserver_updater_proto
Fix #3328
New golang pserver use
OptimizerConfig.prototo configure pserver side.This enables using both "old" configs using "TrainerConfig.proto" and new optimizer config using "OptimizerConfig.proto", ParameterConfig settings will overwrite the global config for variant parameter configuation