add init_gflags interface #5193
Conversation
paddle/pybind/pybind.cc Outdated
| | ||
| std::once_flag gflags_init_flag; | ||
| | ||
| void InitGflags(std::vector<std::string> argv_vec) { |
There was a problem hiding this comment.
The parameter type should be a reference.
paddle/pybind/pybind.cc Outdated
| | ||
| std::once_flag gflags_init_flag; | ||
| | ||
| void InitGflags(std::vector<std::string> argv_vec) { |
There was a problem hiding this comment.
argv_vec => argv. The v in argv has been "vectors".
| return generator.fetch_add(1); | ||
| } | ||
| | ||
| std::once_flag gflags_init_flag; |
There was a problem hiding this comment.
I remember that we have a convention from Google Style Guide -- global variables must be of POD type.
There was a problem hiding this comment.
@QiJune for the alternative, you can use pthread API for this global variable
http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_once.html
There was a problem hiding this comment.
@reyoung pthread_once is used as follows:
pthread_once(pthread_once_t *once_control, void (*init_routine)(void)); We have to pass a vector of string. So it's not suitable.
@wangkuiyi Yes, but once_flag has no deconstructor. It behaves like a POD. And I have not found another better way to write this code yet.
paddle/pybind/pybind.cc Outdated
| | ||
| std::once_flag gflags_init_flag; | ||
| | ||
| void InitGflags(std::vector<std::string> argv_vec) { |
There was a problem hiding this comment.
I think we can add InitGFlags into a new .cc files so could we prevent from making this pybind.cc infinitely long. @reyoung had an idea how to separate pybind.cc into multiple files.
There was a problem hiding this comment.
Yes, we actually have some initialization work to do before executing the neural network, such as glog/gflags settings, and device count/information detection. I will add a TODO in this code and move it to init.cc later.
| import core | ||
| __all__ = ['proto'] | ||
| argv = ['paddle'] | ||
| if os.getenv('FLAGS_fraction_of_gpu_memory_to_use'): |
There was a problem hiding this comment.
Parsing from environment variables is not needed since gflags has already done that.
3 participants
Fix #5194