Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Op in operator registry to another library, like OpInfo #3583

Closed
reyoung opened this issue Aug 19, 2017 · 0 comments
Closed

Move Op in operator registry to another library, like OpInfo #3583

reyoung opened this issue Aug 19, 2017 · 0 comments
Assignees

Comments

@reyoung
Copy link
Collaborator

reyoung commented Aug 19, 2017

OpInfo is stored in OpRegistry in op_register.{h/cc}. However, Operator needs its OpInfo to know which input or output is duplicated, which output is intermediate. OpRegistry needs to register Operator. There is a cycle dependency here.

In the same time, it seems we should move Kernels to OpInfo as its field. Since our OpInfo's field can be nullptr.

Also, we should make the global OpInfo fit Google C++ style. https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables .

// in .h
struct OpInfo {
...
};
extern std::unordered_map<string, OpInfo>& GlobalOpInfo();

// in .cpp
// Never destroy OpInfo, Leave it to operator system. Declare it as a pointer
// to avoid complex global value initialization in C++, since global value initialization
// in C++ is out of order.
static std::unordered_map<string, OpInfo>* g_op_info = nullptr;
std::unordered_map<string, OpInfo> GlobalOpInfo() {
  if (g_op_info == nullptr) {
    // No need to consider multi-thread situation
    g_op_info = new std::unordered_map<string, OpInfo>();
  }
  return *g_op_info;
}

If it should be done, I could give a PR in next day. Help is also welcome.

reyoung added a commit to reyoung/Paddle that referenced this issue Aug 20, 2017
Fix cycle dependencies, Fix PaddlePaddle#3583.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants