Skip to content

Conversation

@reyoung
Copy link
Collaborator

@reyoung reyoung commented Aug 9, 2017

  • Make PADDLE_ENFORCE_EQ supports custom class, like DDim
* Make `PADDLE_ENFORCE_EQ` supports custom class, like DDim
@reyoung reyoung requested review from gangliao and qingqing01 August 9, 2017 05:23

// std::to_string might fill zero after float value, like 1.2000
for (size_t i = 0; i < my_to_string.size(); ++i) {
ASSERT_EQ(my_to_string[i], std_to_string[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

可以去掉std::to_string,直接改成"10", "1.2"来比较吗? 而不是加#if !defined(ANDROID) && !defined(__ANDROID__) 来避免

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

其实也行。。那我这么干吧。

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.

Copy link
Contributor

@qingqing01 qingqing01 left a comment

Choose a reason for hiding this comment

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

LGTM, except the naming for constexpr

#include "paddle/string/to_string.h"
#include <gtest/gtest.h>

constexpr char OUT_STR[] = "User Defined Output";
Copy link
Contributor

Choose a reason for hiding this comment

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

https://google.github.io/styleguide/cppguide.html#Constant_Names

Variables declared constexpr or const, and whose value is fixed for the duration of the program, are named with a leading "k" followed by mixed case. For example:

const int kDaysInAWeek = 7; 
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.

@reyoung reyoung merged commit 37d461d into PaddlePaddle:develop Aug 9, 2017
@reyoung reyoung deleted the feature/use_iostream_for_enforce_to_string branch October 2, 2017 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants