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

Error msg/polish tensor error msg #26976

Merged

Conversation

hbwx24
Copy link
Contributor

@hbwx24 hbwx24 commented Sep 3, 2020

PR types

Function optimization

PR changes

Others

Describe

Polish error message in tensor implementions

@paddle-bot-old
Copy link

paddle-bot-old bot commented Sep 3, 2020

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

@paddle-bot-old
Copy link

paddle-bot-old bot commented Sep 3, 2020

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@hbwx24 hbwx24 closed this Sep 3, 2020
@hbwx24 hbwx24 reopened this Sep 3, 2020
@hbwx24 hbwx24 closed this Sep 3, 2020
@hbwx24 hbwx24 reopened this Sep 3, 2020
@hbwx24 hbwx24 closed this Sep 3, 2020
@hbwx24 hbwx24 reopened this Sep 3, 2020
@hbwx24 hbwx24 closed this Sep 3, 2020
@hbwx24 hbwx24 deleted the error_msg/polish_tensor_error_msg branch September 3, 2020 07:40
@hbwx24 hbwx24 restored the error_msg/polish_tensor_error_msg branch September 3, 2020 07:42
@hbwx24 hbwx24 reopened this Sep 3, 2020
@hbwx24 hbwx24 closed this Sep 3, 2020
@hbwx24 hbwx24 reopened this Sep 3, 2020
@hbwx24 hbwx24 closed this Sep 3, 2020
@hbwx24 hbwx24 reopened this Sep 3, 2020
@hbwx24 hbwx24 closed this Sep 4, 2020
@hbwx24 hbwx24 reopened this Sep 4, 2020
Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

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

Good job!

PADDLE_ENFORCE_NOT_NULL(holder_, platform::errors::PreconditionNotMet(
"The `holder_` of Tensor is nullptr. "
"Tensor holds no memory. "
"Call Tensor::mutable_data first."));
Copy link
Contributor

Choose a reason for hiding this comment

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

first -> firstly may be better

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

"first to re-allocate memory.\n"
"or maybe the required data-type mismatches the data already stored.");
platform::errors::PreconditionNotMet(
"Tensor's dims_ is out of bound."
Copy link
Contributor

@chenwhql chenwhql Sep 4, 2020

Choose a reason for hiding this comment

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

dims_ -> dimension may be better, users may not understand dims_, we need to show users standard English word

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

"or maybe the required data-type mismatches the data already stored.");
platform::errors::PreconditionNotMet(
"Tensor's dims_ is out of bound."
"Tensor's dims_ must be equal or lesser than the size of memory."
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe size of its memory is better?

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

platform::errors::PreconditionNotMet(
"Tensor's dims_ is out of bound."
"Tensor's dims_ must be equal or lesser than the size of memory."
"But received Tensor's dims_ = d%, memory's size = %d.",
Copy link
Contributor

@chenwhql chenwhql Sep 4, 2020

Choose a reason for hiding this comment

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

I think = can directly replaced by is, try to use English word

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

platform::errors::PreconditionNotMet(
"The Tensor's numel must be "
"equal or larger than zero. "
"Please check Tensor::dims, or Tensor::Resize has been "
Copy link
Contributor

Choose a reason for hiding this comment

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

user may not kown the Tensor::dims and Tensor::Resize, because they are interal method, we can only tell users the The tensor's element number is illegal, it must be equal or larger than zero, but actually it's %d, and tensor shape is [%s]

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

PADDLE_ENFORCE_GE(requested_size, size);
PADDLE_ENFORCE_GE(requested_size, size,
platform::errors::InvalidArgument(
"The requested memory size is less than the tensor."
Copy link
Contributor

Choose a reason for hiding this comment

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

make sure less than and lesser than, which is correct grammar?

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

PADDLE_ENFORCE_GE(requested_size, size,
platform::errors::InvalidArgument(
"The requested memory size is less than the tensor."
"But received the requested memory size is d%, "
Copy link
Contributor

Choose a reason for hiding this comment

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

only need one blank, and maybe the can be removed

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

this->holder_, "Cannot invoke mutable data if current hold nothing.");
PADDLE_ENFORCE_NOT_NULL(this->holder_,
platform::errors::PreconditionNotMet(
"The `holder_` of Tensor is nullptr."));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can adjust the holder_, because users also doesn't know what is holder_, when holder is null, the tensor is not be initialized, maybe we can tell user The tensor is not initialized.?

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

PADDLE_ENFORCE_GE(
begin_idx, 0,
platform::errors::OutOfRange("The start row index must be greater than 0."
"But received the start index is d%.",
Copy link
Contributor

Choose a reason for hiding this comment

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

multiple blank between received the

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done, thx

PADDLE_ENFORCE_LT(
begin_idx, end_idx,
"The start row index must be lesser than the end row index.");
platform::errors::InvalidArgument(
"The start row index must be lesser than the end row index."
Copy link
Contributor

Choose a reason for hiding this comment

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

same above lesser than or less than?

Copy link
Contributor Author

@hbwx24 hbwx24 Sep 4, 2020

Choose a reason for hiding this comment

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

Done,thx

@hbwx24 hbwx24 closed this Sep 5, 2020
@hbwx24 hbwx24 reopened this Sep 5, 2020
@@ -0,0 +1,3 @@
Start testing: Sep 04 06:09 UTC
Copy link
Contributor

Choose a reason for hiding this comment

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

these two temporary filles need to be removed

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, thx.

PADDLE_ENFORCE_GE(
numel(), 0,
platform::errors::PreconditionNotMet(
"The Tensor's element number must be equal or larger than zero. "
Copy link
Contributor

Choose a reason for hiding this comment

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

larger -> greater?

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, thx.

Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

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

LGTM

@chenwhql chenwhql merged commit 13804ed into PaddlePaddle:develop Sep 8, 2020
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