Skip to content

CPU only tests #478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
shelhamer merged 6 commits into from
Jun 25, 2014
Merged

CPU only tests #478

shelhamer merged 6 commits into from
Jun 25, 2014

Conversation

@kloudkl
Copy link
Contributor

@kloudkl kloudkl commented Jun 7, 2014

To enable running the tests on machines with only the CPU, the GPU related codes have to be separated.

Related issue: #468

@kloudkl kloudkl mentioned this pull request Jun 7, 2014
Closed
@kloudkl kloudkl changed the title Cpu only tests CPU only tests Jun 7, 2014
Yangqing
Yangqing reviewed Jun 9, 2014
View reviewed changes
src/caffe/test/test_hdf5_output_layer.cpp Outdated
Copy link
Member

@Yangqing Yangqing Jun 9, 2014

Choose a reason for hiding this comment

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

Could you return this to the original one-line format?

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces

@Yangqing
Copy link
Member

Yangqing commented Jun 9, 2014

Looks good to me, thanks for cleaning the CPU and GPU test :)

Yangqing
Yangqing reviewed Jun 9, 2014
View reviewed changes
src/caffe/common.cpp Outdated
Copy link
Member

@Yangqing Yangqing Jun 9, 2014

Choose a reason for hiding this comment

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

Since we tend to stick to the Google style guide, kindly rename this to

static bool g_curand_availability_logged = false;

and likewise below.

@kloudkl
Copy link
Contributor Author

kloudkl commented Jun 10, 2014

Both issues have been fixed. Ready to be merged.

@Yangqing
Copy link
Member

Yangqing commented Jun 10, 2014

@shelhamer Once the coldbrew master agrees, let's merge :)

shelhamer
shelhamer reviewed Jun 13, 2014
View reviewed changes
src/caffe/test/test_syncedmem.cpp Outdated
Copy link
Member

@shelhamer shelhamer Jun 13, 2014

Choose a reason for hiding this comment

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

Why was this test dropped? the recovered_value test that follows is the one actually checking the sync, and the memset should always succeed, but tests are tests.

@shelhamer
Copy link
Member

shelhamer commented Jun 13, 2014

@kloudkl thanks for separating the tests. I had one trivial comment, and once that's addressed we'll merge.

Thanks for pointing out this PR @Yangqing.

@kloudkl kloudkl mentioned this pull request Jun 17, 2014
Merged
@kloudkl
Copy link
Contributor Author

kloudkl commented Jun 17, 2014

@shelhamer, the memtest issue is resolved. Please review again. Thanks!

shelhamer added a commit that referenced this pull request Jun 25, 2014
@shelhamer
Merge pull request #478 from kloudkl/cpu_only_tests
c826e52 
CPU only tests
@shelhamer shelhamer merged commit c826e52 into BVLC:dev Jun 25, 2014
@shelhamer
Copy link
Member

shelhamer commented Jun 25, 2014

Thanks @kloudkl! ./build/test/test_all.testbin --gtest_filter=*CPU* runs only CPU tests.

mitmul pushed a commit to mitmul/caffe that referenced this pull request Sep 30, 2014
@shelhamer
Merge pull request BVLC#478 from kloudkl/cpu_only_tests
2e52bfa 
CPU only tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kloudkl @Yangqing @shelhamer