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 SQRT/LAST/FIRST strategy for Seqpool #4788

Merged
merged 10 commits into from
Oct 16, 2017
Merged

add SQRT/LAST/FIRST strategy for Seqpool #4788

merged 10 commits into from
Oct 16, 2017

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented Oct 13, 2017

solve part2 of #4186
MAX strategy will be in next PR.

@luotao1 luotao1 changed the title add SQRT/LAST/FIRST strategy for sequence_pool_opSeqpool add SQRT/LAST/FIRST strategy for Seqpool Oct 13, 2017
AddOutput(
"Out",
"A float LoDTensor, the variable-length output of SequencePoolOp.");
AddInput("X", "A LoDTensor, the variable-length input of SequencePoolOp");
Copy link
Contributor

Choose a reason for hiding this comment

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

 AddInput("X", 
          "(LoDTensor), the variable-length input of SequencePoolOp");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"A float LoDTensor, the variable-length output of SequencePoolOp.");
AddInput("X", "A LoDTensor, the variable-length input of SequencePoolOp");
AddOutput("Out",
"A LoDTensor, the variable-length output of SequencePoolOp.");
Copy link
Contributor

Choose a reason for hiding this comment

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

  AddOutput("Out",
            "(Tensor), the output of SequencePoolOp is a Tensor, which does not contain LoD infomation.");

这里支持双层吗? 输出还变长吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done。由于目前还没写到双层,这里先修改成Tensor。等以后加入双层特征后,再对应修改注释。

@@ -98,6 +109,10 @@ class SequencePoolGradKernel : public framework::OpKernel<T> {
int64_t w = in->numel() / dims[0];

in_g->mutable_data<T>(context.GetPlace());
if (strategy > 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if ( strategy == LAST || strategy == FIRST || strategy == MAX) {
  // ..
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done。Max的时候,也不需要先置0.(处理方式见下一个PR)

Copy link
Contributor

@qingqing01 qingqing01 left a comment

Choose a reason for hiding this comment

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

Need more detailed doc.

AVERAGE, SUM, SQRT, MAX, LAST, FIRST

Except for the simple example, these arguments should also be explained.

@luotao1
Copy link
Contributor Author

luotao1 commented Oct 16, 2017

I will add more explanation in next PR.

@luotao1 luotao1 merged commit 440ad99 into PaddlePaddle:develop Oct 16, 2017
@luotao1 luotao1 deleted the seqpool branch October 16, 2017 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants