-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
create grad_var when run Backward pass #4796
Conversation
… improve-backward
… improve-backward
… improve-backward
paddle/framework/backward.cc
Outdated
@@ -375,10 +376,18 @@ std::vector<std::unique_ptr<OpDescBind>> MakeBlockBackward( | |||
backward_descs.insert(backward_descs.begin() + p.first + 1, | |||
std::move(p.second)); | |||
} | |||
|
|||
// insert fill one op for target | |||
std::unique_ptr<OpDescBind> fill_one_op(new OpDescBind( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be in AppendBackward
. That logic is wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
paddle/framework/backward.cc
Outdated
auto backward_op_descs = | ||
MakeBlockBackward(program_desc, root_block_idx, no_grad_var_names); | ||
auto& forw_op_descs = program_desc.Block(root_block_idx)->ops_; | ||
auto backward_op_descs = MakeBlockBackward(program_desc, root_block_idx, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target does not need to pass to MakeBlockBackward
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
paddle/framework/backward.cc
Outdated
auto backward_op_descs = MakeBlockBackward(program_desc, root_block_idx, | ||
target, no_grad_var_names); | ||
auto root_block = program_desc.Block(root_block_idx); | ||
auto& forward_op_descs = root_block->ops_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forward_op_descs
--> ops
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
paddle/framework/backward.cc
Outdated
for (size_t index = backward_start_index; index < forward_op_descs.size(); | ||
++index) { | ||
auto& op = forward_op_descs.at(index); | ||
for (auto& input : op->Inputs()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Input does not need to be created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
paddle/framework/backward.cc
Outdated
auto& forw_op_descs = program_desc.Block(root_block_idx)->ops_; | ||
auto backward_op_descs = MakeBlockBackward(program_desc, root_block_idx, | ||
target, no_grad_var_names); | ||
auto root_block = program_desc.Block(root_block_idx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not only root_block need create vars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
paddle/framework/backward.cc
Outdated
root_block->NewVar(fill_one_op_out); | ||
|
||
// create grad_var for all blocks in this program | ||
for (size_t block_index = 0; block_index < program_desc.Size(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all block is generated by backward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
project: #4679
fix: #4767