Skip to content
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.

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

Travis CI rebased #681

Merged
merged 5 commits into from
Jul 14, 2014
Merged

Travis CI rebased #681

merged 5 commits into from
Jul 14, 2014

Conversation

jeffdonahue
Copy link
Contributor

Hi, I've taken PR #667 (thanks @huyng!) and rebased it onto some commits cutting down on the duplication between CPU and GPU tests to make it easier and more reliable to run only the CPU tests, and cut down on duplicated code throughout the test suite. I added a make runtestnogpu target to run only the tests that do not use the GPU (had to append CPUGPU to some tests that use both CPU and GPU and can't be separated).

I made a few changes to the .travis.yml in d95aea8 for CPU-only testing and notification by email only going to the committer. Others can see build status on the PR or travis feed.

@jeffdonahue
Copy link
Contributor Author

whoohoo, PR integration working (and build passed!)

@shelhamer
Copy link
Member

I'm so excited for this! Please squash before merge.

@@ -38,7 +38,7 @@ TEST_SRCS := $(filter-out $(TEST_MAIN_SRC), $(TEST_SRCS))
TEST_CU_SRCS := $(shell find src/$(PROJECT) -name "test_*.cu")
GTEST_SRC := src/gtest/gtest-all.cpp
# TEST_HDRS are the test header files
Copy link
Member

Choose a reason for hiding this comment

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

Why HDR? Everywhere else is HXX.

@shelhamer
Copy link
Member

Nice extension of the testing types and arranging into include/caffe.

@jeffdonahue
Copy link
Contributor Author

Thanks for taking a look @shelhamer! Squashed history, changed TEST_HDRS to TEST_HXX_SRCS in Makefile, and removed CUSTOM_CXX.

jeffdonahue and others added 5 commits July 14, 2014 02:17
Add a test param to test both CPU and GPU (with both float and double
Dtypes).
Makefile.config instead of CUSTOM_CXX, as Travis exports CXX
as the compiler env variable name.
-Change TEST_HDFS -> TEST_HXX_SRCS.
-Add (CPU-only) test, lint, warn and parallel (-j 4) to travis CI build.
-Add /usr/local/lib to travis config (seems needed for LMDB).
-Put export in "before_script"; disable clang build -- doesn't work on Linux.
-Cache Ubuntu apt packages.
-Install bc package to hopefully suppress "bc: not found" errors
-Get apt packages before_install as suggested by Travis official docs
-Remove specified email address and IRC notifications (emails are sent
to the committer by default; others can view build results in public
Travis feed, on pull requests, etc.).
exit with a non-zero code -- this fixes that.
@shelhamer
Copy link
Member

Sidenote: much of the test time is downloading and installing the CUDA requirements plus doing the nvcc compilation, so this should act more lively once the device abstraction and CPU-only build are done.

shelhamer added a commit that referenced this pull request Jul 14, 2014
continuous integration for build, test, and lint by Travis CI
@shelhamer shelhamer merged commit cdb6014 into dev Jul 14, 2014
@shelhamer
Copy link
Member

Open 24/7

@shelhamer shelhamer deleted the travisci branch July 14, 2014 09:27
@jeffdonahue
Copy link
Contributor Author

sweet, thanks for the quick merge

@shelhamer
Copy link
Member

Wait, I remember why I did CUSTOM_CXX now. At least on OS X the CXX ?= method breaks because it picks up the value c++ from somewhere even though it's not in my environment as reported by env.

@jeffdonahue can you revert the change to the Makefile to bring back the CUSTOM_CXX logic and make Travis set the set CUSTOM_CXX (e.g. sed the Makefile.config or call CUSTOM_CXX=g++ make instead)?

@longjon
Copy link
Contributor

longjon commented Jul 14, 2014

@shelhamer, right, I believe CXX and a few others are set to defaults by
make itself for use in implicit rules, see
https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html.
On Jul 14, 2014 6:58 AM, "Evan Shelhamer" notifications@github.com wrote:

Wait, I remember why I did CUSTOM_CXX now. At least on OS X the CXX ?=
method breaks because it picks up the value c++ from somewhere even
though it's not in my environment as reported by env.

Can you revert the change to CXX and make Travis set the Makefile.config
CUSTOM_CXX field instead (e.g. run a sed command or whatever)?


Reply to this email directly or view it on GitHub
#681 (comment).

@jeffdonahue
Copy link
Contributor Author

Re-added CUSTOM_CXX stuff to Makefile and Makefile.config.example in #694. I didn't change the travis build logic yet to use it yet because I'm not sure how to get the separate, per-compiler, builds behavior working using the explicit env variable export or Makefile.config change, rather than the Travis compiler field (feel free to take this on if you'd like). I'm not sure about make itself setting CXX and other variables; when I had the clang build enabled (can try yourself by uncommenting the - clang line in .travis.yml) it was failing so clearly the CXX setting was having some effect.

@longjon
Copy link
Contributor

longjon commented Jul 14, 2014

@jeffdonahue travis sets the CXX environment variable, which make picks up to set its own CXX.

Witness:

$ echo $CXX

$ cat > Makefile
all:  
    echo $(CXX)

$ make
echo g++
g++

$ CXX=clang++ make
echo clang++
clang++

$ cat > Makefile
CXX ?= clang++
all:
    echo $(CXX)

$ make
echo g++
g++

See https://www.gnu.org/software/make/manual/html_node/Environment.html#Environment.

@jeffdonahue
Copy link
Contributor Author

I see -- so make lets the CXX env var setting override its default, but specifying CXX as a Makefile variable doesn't affect anything because it just keeps the default setting when it runs into a line that tries to set it to something else? Interesting...

@longjon
Copy link
Contributor

longjon commented Jul 14, 2014

You can override the value with a :=, but if you try to specify it with a ?= line, yes. In effect make imports your environment and then has its own ?= lines for a bunch of default variables.

@jeffdonahue
Copy link
Contributor Author

Oh okay, that makes sense -- thanks for the explanation.

mitmul pushed a commit to mitmul/caffe that referenced this pull request Sep 30, 2014
continuous integration for build, test, and lint by Travis CI
RazvanRanca pushed a commit to RazvanRanca/caffe that referenced this pull request Nov 4, 2014
continuous integration for build, test, and lint by Travis CI
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