Skip to content

Conversation

@ktlichkid
Copy link

@ktlichkid ktlichkid commented May 9, 2018

Resolved #10512

@ktlichkid ktlichkid force-pushed the fix-10512 branch 4 times, most recently from 1dc8153 to 2d498b1 Compare May 15, 2018 05:43
@ktlichkid ktlichkid changed the title [WIP]Adding has_data_op Added has_data_op May 15, 2018
@ktlichkid ktlichkid requested a review from pkuyym May 15, 2018 11:56
@pkuyym pkuyym requested a review from guoshengCS May 16, 2018 03:44
Copy link
Contributor

@pkuyym pkuyym left a comment

Choose a reason for hiding this comment

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

@@ -0,0 +1,70 @@
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

2016 --> 2018

limitations under the License. */

#include "paddle/fluid/operators/has_data_op.h"
#include <string>
Copy link
Contributor

Choose a reason for hiding this comment

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

exchange line15 with line16

void Make() override {
// inputs and outputs stored in proto
AddInput("X", "(LoDTensor) the LoDTensor to check");
AddOutput("Out", "(LoDTensor) the ouput of has_data_op");
Copy link
Contributor

Choose a reason for hiding this comment

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

ouput --> output
has_data_op --> HasDataOp
Please add a dot at end of each sentence.

PADDLE_ENFORCE(ctx->HasOutput("Out"),
"Output(Out) of HasDataOp should not be null.");
ctx->SetOutputDim("Out", {1});
ctx->ShareLoD("X", "Out");
Copy link
Contributor

Choose a reason for hiding this comment

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

LoD of Out shouldn't inherit from X, in fact, Out has no LoD.

@ktlichkid ktlichkid closed this May 16, 2018
@ktlichkid ktlichkid deleted the fix-10512 branch August 8, 2018 10:46
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.

2 participants