Skip to content

Comments

Remove enforce demangle#4002

Merged
Xreki merged 1 commit intoPaddlePaddle:developfrom
reyoung:feature/remove_enforce_demagle
Sep 11, 2017
Merged

Remove enforce demangle#4002
Xreki merged 1 commit intoPaddlePaddle:developfrom
reyoung:feature/remove_enforce_demagle

Conversation

@reyoung
Copy link
Collaborator

@reyoung reyoung commented Sep 11, 2017

It is buggy in some Linux because the unique_ptr will be free however
the std::string trying to use that char*.

Moreover, it is no need to demangle for error log by Paddle.
Just use c++filt or other shell utilities to do this.

Fix #3864
Fix #3967
Fix #3858
Fix #3865

@reyoung reyoung force-pushed the feature/remove_enforce_demagle branch from a57139e to dad5421 Compare September 11, 2017 04:25
It is buggy in some Linux because the unique_ptr will be free however
the std::string trying to use that char*.

Moreover, it is no need to demangle for error log by Paddle.
Just use `c++filt` or other shell utilities to do this.
@reyoung reyoung requested a review from Xreki September 11, 2017 04:26
Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

Please remember to add "Fix XXX" in the first comment before merging it.

Also, I still don't quite understand the reason of the bug. Should be better to explain more in the issue.

Copy link
Contributor

@Xreki Xreki left a comment

Choose a reason for hiding this comment

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

LGTM++
I have tested this PR. Stack information can be printed when PADDLE_ENFORCE failed in both C++ and Python code on Linux.

@Xreki Xreki merged commit 4fbc03d into PaddlePaddle:develop Sep 11, 2017
@jacquesqiao
Copy link
Member

测试了下,
删掉unique_ptr之后,只保留

inline std::string demangle(std::string name) { return name; }

依然报同样的错误,所以不是unique_ptr的问题,是别的地方的问题。

@chengduoZH
Copy link
Contributor

Fix #3935

@reyoung reyoung deleted the feature/remove_enforce_demagle branch October 2, 2017 18:19
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.

5 participants