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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants