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

add keep_kids flag for executor #11524

Merged

Conversation

jacquesqiao
Copy link
Member

No description provided.

@jacquesqiao jacquesqiao requested a review from reyoung June 17, 2018 03:52
Copy link
Collaborator

@reyoung reyoung left a comment

Choose a reason for hiding this comment

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

The kids should not be kept.

  • If users add a FetchOp, the device context will be synchronized. Every operator should be completed after Run.
  • If users do not add a FetchOp, we should wait for the device context instead of keeping children scope. (We may add code to sync device for this situation.)

@emailweixu
Copy link
Collaborator

@reyoung In while_op.cc, the the step scopes created in the scope passed to executor.run needs to be kept so that they can be accessed by while_grad_op. If kids are dropped, how to achieve this?
https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/operators/while_op.cc#L63

@reyoung
Copy link
Collaborator

reyoung commented Jun 20, 2018

In control operators, the create_local_scopes will be false. So the executor in WhileOp will not destroy any scopes. The scope will be destroyed after the whole program has been executed.

@reyoung
Copy link
Collaborator

reyoung commented Jun 20, 2018

In the original design of Executor, if create_local_scopes=False, the local scopes will not be destroyed.

if (create_local_scope) {
scope->DeleteScope(local_scope);
}

However, it seems the create_vars flag change this behaviour on our develop branch.

if (create_vars && create_local_scope) {
scope->DeleteScope(local_scope);
} else {
// Delete the local scopes created in operators.
scope->DropKids();
}

@emailweixu
Copy link
Collaborator

This needs to be fixed. Otherwise nested while_op does not work. What should be the right way to fix it?

Copy link
Collaborator

@reyoung reyoung left a comment

Choose a reason for hiding this comment

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

Merge this HotFix first. I will write a issue to record this problem

@jacquesqiao jacquesqiao merged commit 05a9277 into PaddlePaddle:develop Jun 20, 2018
@reyoung
Copy link
Collaborator

reyoung commented Jun 20, 2018

The problem is recorded in this issue #11597

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

Successfully merging this pull request may close these issues.

None yet

3 participants