Skip to content

Conversation

@reyoung
Copy link
Collaborator

@reyoung reyoung commented Jun 28, 2017

Basically from caffe2::logging.h, but only expose PADDLE_ENFORCE
interface.

Basically from caffe2::logging.h, but only expose `PADDLE_ENFORCE` interface.
@reyoung reyoung force-pushed the feature/add_enforce branch from 09dcc7a to 3a119ef Compare June 28, 2017 08:25
@@ -0,0 +1,116 @@
/*
Copy link
Member

@jacquesqiao jacquesqiao Jun 28, 2017

Choose a reason for hiding this comment

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

The Copyright format is not right, please use this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@@ -0,0 +1,25 @@
#include <gtest/gtest.h>
Copy link
Member

@jacquesqiao jacquesqiao Jun 28, 2017

Choose a reason for hiding this comment

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

add Copyright info.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

all_msg_ = sout.str();
}

const char* what() const noexcept override { return all_msg_.c_str(); }
Copy link
Member

Choose a reason for hiding this comment

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

why not just return a std::string

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because it is the std::exception interface.

Copy link
Member

Choose a reason for hiding this comment

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

got it! thanks~

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.

LGTM


namespace details {

inline void MakeStringInternal(std::ostringstream& stream) {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should replace MakeString by string::Printf in the near future. I have a PR working on string::Printf.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, and wait for CI. After passing CI tests, this PR will be merged.

@reyoung reyoung merged commit f86b3ec into PaddlePaddle:develop Jun 29, 2017
@reyoung reyoung deleted the feature/add_enforce branch July 13, 2017 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants