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

Network implement #2730

Merged
merged 24 commits into from
Jul 11, 2017
Merged

Network implement #2730

merged 24 commits into from
Jul 11, 2017

Conversation

dzhwinter
Copy link
Contributor

@dzhwinter dzhwinter commented Jul 4, 2017

No description provided.

/**
* @brief Infer shapes of all inputs and outputs of operators.
*/
virtual bool InferShape(Scope *scope) override;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些 bool 都改成 Error 吧

@Superjomn Superjomn requested review from reyoung and jacquesqiao and removed request for reyoung and jacquesqiao July 4, 2017 04:59
Error PlainNet::InferShape(Scope* scope) {
for (auto& op : ops_) {
// wrong shape
auto err = op.InferShape();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use Error now, please use PADDLE_ENFORCE instead.

@Superjomn
Copy link
Contributor

Superjomn commented Jul 7, 2017

Several interfaces may be needed:

RNN need a proto or string as proto to create a StepNet, so need a method to serialize the net to proto or string:

void Serialize(NetDesc* net_desc) = 0

Forward or backward using Net.Run seems difficult because end_idx is hard to determine, so following methods may be needed:

void Forward() = 0;
void Backward() = 0;

@dzhwinter @reyoung

@Superjomn
Copy link
Contributor

Superjomn commented Jul 7, 2017

Do AddOptimizerOps and AddBackwardOps must be called in order?
AddBackwardOps first, AddOptimizerOps second?

@dzhwinter @reyoung @jacquesqiao

@jacquesqiao
Copy link
Member

AddOptimizerOps should call after AddBackwardOps

@QiJune
Copy link
Member

QiJune commented Jul 7, 2017

The AddBackwardOps will add all backward related operators. So, we have to find a way to tell the input and output variables of each backward operator.
And the Optimizer will take parameter/gradient/momentum/... as input and update the parameter.
The Optimizer operator will take calculate one pair of parameter and gradient. But the Optimizer will take all.
Since, we also have to make checkpoint of a Net parameters, I suggest we put forward a concept called Model.
The Model has some maps which records the relationship of parameters/gradients. And learning rate, batch_step can also be found in Model.
The Model has a method called CheckPoint, and will serialize its data member to disk.

@Superjomn
Copy link
Contributor

After a discussion, the current decision is as follows:

  • add a temporary solution for Net's backward @dzhwinter
  • no need to add Serialize currently, use AddOp to build a step net for RNN.
  • Model is a user-related API, currently is not needed.

Copy link
Contributor

@Superjomn Superjomn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGM

@dzhwinter
Copy link
Contributor Author

check in for @Superjom 's rnn op dependency.

@dzhwinter dzhwinter merged commit 71d196c into PaddlePaddle:develop Jul 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants