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

fix two error message #32039

Merged
merged 6 commits into from
Apr 6, 2021
Merged

fix two error message #32039

merged 6 commits into from
Apr 6, 2021

Conversation

Kqnonrime
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

fix two error message

@paddle-bot-old
Copy link

paddle-bot-old bot commented Apr 2, 2021

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

jzhang533
jzhang533 previously approved these changes Apr 2, 2021
Copy link
Contributor

@jzhang533 jzhang533 left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -104,7 +104,10 @@ void ScatterAssign(const platform::DeviceContext& ctx, const Tensor& src,
for (int i = 1; i < src_dims.size(); i++)
PADDLE_ENFORCE_EQ(src_dims[i], dst_dims[i],
platform::errors::InvalidArgument(
"src shape and dst shape should match"));
"src's shape and dst's shape should match."
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 建议使用完整单词表述,src, dst, dim这种都不是标准单词
  • 建议首字母大写
  • 这个shape和dim表示的是同一个意思,建议统一都用dimension
  • 这里是第几维度不一致,表述要准确
  • 后面的对比句子句法有误

比如可以改为:The dimensions of the source tensor and target tensor should match, but received source tensor's %d-th dimension is %d, target tensor's %d-th dimension is %d.

@@ -148,7 +151,10 @@ void ScatterAssignAdd(const framework::ExecutionContext& ctx, const Tensor& src,
for (int i = 1; i < src_dims.size(); i++)
PADDLE_ENFORCE_EQ(src_dims[i], dst_dims[i],
platform::errors::InvalidArgument(
"src shape and dst shape should match"));
"src's shape and dst's shape should match,"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上,请完善表述

@@ -101,14 +101,19 @@ class UnStackGradOp : public framework::OperatorWithKernel {
void InferShape(framework::InferShapeContext *ctx) const override {
PADDLE_ENFORCE_GT(ctx->Inputs(framework::GradVarName("Y")).size(), 0,
platform::errors::InvalidArgument(
"Number of Inputs(Y@Grad) must be larger than 0"));
"Number of Inputs(Y@Grad) must be larger than 0."
Copy link
Contributor

@chenwhql chenwhql Apr 2, 2021

Choose a reason for hiding this comment

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

这里是不太自然的,其实要表达的意思就是的输入为空吧,目前这个比较绕,直接告诉用户这里是空就好了, The Inputs(Y@Grad) of unstack operator are empty.

OP_INOUT_CHECK(ctx->HasOutput(framework::GradVarName("X")), "Output", "X",
"UnStackGrad");
auto input_dims = ctx->GetInputsDim(framework::GradVarName("Y"));
for (size_t i = 1; i < input_dims.size(); ++i) {
PADDLE_ENFORCE_EQ(input_dims[i], input_dims[0],
platform::errors::InvalidArgument(
"Dims of all Inputs(Y@Grad) must be the same"));
"Dims of all Inputs(Y@Grad) must be the same,"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上,请参考前面的评论完善表述

Copy link
Contributor Author

Choose a reason for hiding this comment

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

收到,谢谢指导,已改正!

Copy link
Contributor

@swtkiwi swtkiwi left a comment

Choose a reason for hiding this comment

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

LGTM

@swtkiwi swtkiwi merged commit 9e8f903 into PaddlePaddle:develop Apr 6, 2021
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.

4 participants