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

lookup_table_v2_op_xpu report errors;test=kunlun #28064

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

yinhaofeng
Copy link
Contributor

PR types

Bug fixes

PR changes

OPs

Describe

Fixed error message

@paddle-bot-old
Copy link

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

@@ -31,11 +31,13 @@ class LookupTableV2XPUKernel : public framework::OpKernel<T> {
auto *table_var = context.InputVar("W");
PADDLE_ENFORCE_EQ(
(std::is_same<DeviceContext, platform::XPUDeviceContext>::value), true,
platform::errors::InvalidArgument("Unsupported place!"));
platform::errors::PreconditionNotMet(
Copy link
Contributor

Choose a reason for hiding this comment

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

是不是提示下这里只支持XPUPlace更清晰

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改


PADDLE_ENFORCE_EQ(table_var->IsType<LoDTensor>(), true,
platform::errors::InvalidArgument(
"idx in LookupTableV2XPUKernel should be LoDTensor"));
"Tensor holds the wrong type,idx in "
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

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.

已修改

"idx_numel in LookupTableV2XPUKernel should not "
"greater than int32_t::max."));
platform::errors::OutOfRange(
"idx_numel greater than int32_t::max. please check "
Copy link
Contributor

Choose a reason for hiding this comment

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

idx还是ids?idx_numel是用户输入的变量吗?如果不是的话,就是我们内部C++变量,建议改成用户更容易接受的描述,比如把变量缩写拆开,比如 number of element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改,拆成了number of ids

"idx in LookupTableV2GradXPUKernel should be LoDTensor"));
PADDLE_ENFORCE_EQ(table_var->IsType<LoDTensor>(), true,
platform::errors::InvalidArgument(
"Tensor holds the wrong type,idx in "
Copy link
Contributor

Choose a reason for hiding this comment

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

这里可能不太好改,但是需要斟酌一下,Tensor holds the wrong type,但正确type确实LoDTensor?这不是同级概念,令人困惑

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

"idx_numel in LookupTableV2GradXPUKernel should not "
"greater than int32_t::max."));
platform::errors::OutOfRange(
"idx_numel greater than int32_t::max. please check "
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

@fuyinno4 fuyinno4 merged commit 2cb1ecb into PaddlePaddle:develop Oct 19, 2020
yinhaofeng added a commit to yinhaofeng/Paddle that referenced this pull request Oct 20, 2020
* lookup_table_v2_op_xpu report errors;test=kunlun

* lookup_table_v2_op_xpu report errors;test=kunlun
fuyinno4 pushed a commit that referenced this pull request Oct 20, 2020
* lookup_table_v2_op_xpu report errors;test=kunlun

* lookup_table_v2_op_xpu report errors;test=kunlun
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.

3 participants