Skip to content

Comments

Clean flaky code#1264

Merged
Yangqing merged 2 commits intoBVLC:devfrom
Yangqing:dev
Oct 12, 2014
Merged

Clean flaky code#1264
Yangqing merged 2 commits intoBVLC:devfrom
Yangqing:dev

Conversation

@Yangqing
Copy link
Member

(1) Whether char is signed or unsigned is not defined by c++ standard, so it makes more sense to use int8_t to get sign values.

(2) signbit returns a non-zero value when the sign bit is set, and we can't implicitly assume that it will be 1. This will cause problem when we directly assign it to e.g. a float value. Need to static cast the value to bool first.

(3) Explicit instantiation is not needed when the whole function is defined in the header file.

(4) Don't do EXPECT_EQ for float values. Always assume that there is numerical instability.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe EXPECT_FLOAT_EQ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, changed.

@jeffdonahue
Copy link
Contributor

cool, LGTM

Yangqing added a commit that referenced this pull request Oct 12, 2014
@Yangqing Yangqing merged commit e53aafa into BVLC:dev Oct 12, 2014
@Yangqing Yangqing mentioned this pull request Oct 15, 2014
@shelhamer shelhamer mentioned this pull request Oct 17, 2014
RazvanRanca pushed a commit to RazvanRanca/caffe that referenced this pull request Nov 4, 2014
@yulijun1234
Copy link

Makefile:427: target '.build_release/src/caffe/util/math_functions' doesn't match the target pattern
make: *** No rule to make target 'include/caffe/util/math_functions', needed by '.build_release/src/caffe/common.o'. Stop.

I have changed as mentioned above,but it comes with this problem. Can you give me some advice?

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