Skip to content

Conversation

@hedaoyuan
Copy link
Contributor

No description provided.

@hedaoyuan hedaoyuan requested a review from NHZlX August 10, 2017 09:11
@NHZlX
Copy link
Contributor

NHZlX commented Aug 10, 2017

More and more test files are added to paddle/function, for users, it looks very messy, should we consider to building a folder ?

@NHZlX
Copy link
Contributor

NHZlX commented Aug 10, 2017

The refinement of the test of convolution function looks great to me.

@hedaoyuan
Copy link
Contributor Author

More and more test files are added to paddle/function, for users, it looks very messy, should we consider to building a folder ?

Put the test file and source file in one directory is a new convention. You can see in the platform, operators, framework the test file and source file are also placed in a directory.

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.

赞!删了这么多代码!

有一个 C-style cast,应该改成C++ style 的。

CHECK_EQ(strideW(), 1);

// TODO(hedaoyuan): There has some bug when batchSize > 1 and groups_ > 1.
CHECK_EQ(groups_, (size_t)1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

(size_t)1 ==> size_t(1) or static_cast<size_t>(1), according to Google C++ style https://google.github.io/styleguide/cppguide.html#Casting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing out. Done.

@hedaoyuan hedaoyuan merged commit d87c0b1 into PaddlePaddle:develop Aug 11, 2017
heavengate pushed a commit to heavengate/Paddle that referenced this pull request Aug 16, 2021
* fix bug when benchmark=True

* split warmup and repeat
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