chsin commented on a change in pull request #9809: register optimizers only
once in CPP-Package
URL: https://github.com/apache/incubator-mxnet/pull/9809#discussion_r169368848
##########
File path: cpp-package/include/mxnet-cpp/optimizer.hpp
##########
@@ -125,13 +125,16 @@ inline float Optimizer::GetWD_(int index) {
}
inline Optimizer* OptimizerRegistry::Find(const std::string& name) {
- MXNETCPP_REGISTER_OPTIMIZER(sgd, SGDOptimizer);
- MXNETCPP_REGISTER_OPTIMIZER(ccsgd, SGDOptimizer); // For backward
compatibility
- MXNETCPP_REGISTER_OPTIMIZER(rmsprop, RMSPropOptimizer);
- MXNETCPP_REGISTER_OPTIMIZER(adam, AdamOptimizer);
- MXNETCPP_REGISTER_OPTIMIZER(adagrad, AdaGradOptimizer);
- MXNETCPP_REGISTER_OPTIMIZER(adadelta, AdaDeltaOptimizer);
- MXNETCPP_REGISTER_OPTIMIZER(signum, SignumOptimizer);
+ if (cmap().empty()) {
Review comment:
Optimizers are all registered at once in the original CPP-Package code and
in the Python code using decorators. In the original code
[#5251](https://github.com/apache/incubator-mxnet/blob/21b2da07f6ff10669b7649860b913e17e6184708/cpp-package/include/mxnet-cpp/optimizer.hpp),
they were all registered at once in the file, so when the program was
initialized, all optimizers would already be registered (which is the case in
Python), not in the first call to the function Find, but that caused multiple
definition errors, so in #5545, the were all registered at once in the first
call to Find, which caused the inability to have more than one optimizer bc
they would be re-registered with each call, so this change makes sure they are
registered once. Why check every optimizer separately when they should all be
registered at the same time and only once?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
