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

OpInfo should contain Input/Output name set #3829

Closed
reyoung opened this issue Sep 3, 2017 · 2 comments
Closed

OpInfo should contain Input/Output name set #3829

reyoung opened this issue Sep 3, 2017 · 2 comments
Assignees

Comments

@reyoung
Copy link
Collaborator

reyoung commented Sep 3, 2017

Currently, our operators use the parameter name of input/output to get the input/output variable names. For example, we are using Input("X"), Input("W"), and Input("B") in fully connected operator to get the data, weight matrix and bias. But using string is easy to get wrong, our framework should check whether that parameter name is registered in OpInfo or not. The parameter name is saved in OpProto.Var.name.

We can do this by searching the parameter name in OpProto directly. However, this implementation has several issues:

  1. The time complexity for validating a parameter name directly by OpProto is O(n), which n is the number of inputs/outputs since it is stored in a linear list.
  2. Gradient operators do not have OpProto.

The simpler way is to add input_params and output_params sets to OpInfo as follow:

struct OpInfo {
  set<string> input_params;
  set<string> output_params;
};

The OpInfo should be constructed while registering operator. It works well for both forwarding and backward operators.

@JiayiFeng
Copy link
Collaborator

JiayiFeng commented Sep 3, 2017

We have recorded operators' inputs/outputs information in OpProto, so I think it's not necessary to record it again in OpInfo. If users don't assign any variables to some inputs/outputs, just leave the corresponding name vector in Op object empty.

@reyoung
Copy link
Collaborator Author

reyoung commented Sep 3, 2017

@Canpio That's right.

@reyoung reyoung closed this as completed Sep 3, 2017
heavengate pushed a commit to heavengate/Paddle that referenced this issue Aug 16, 2021
…Paddle#3844)

* [dev] rbox update2 (PaddlePaddle#3828)

* set lr for 4 card as default, and update

* fix error
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

3 participants