Investigate and prepare general purpose Makefile

[kudkudak]

If possible should include Makevars so that we don't get desynchronized

If possible should be able to build tests so that we can remove tests/cpp/Makefile

Could be CMake, not necessarily Makefile

[ktalik]

More info: #162

[ktalik]

GMUM.R compilation is OK but CPP tests are failing... Can someone take look at this? I don't know why it can't find Rcpp headers:

https://gist.github.com/ktalik/a69c12942740227fab33

(branch task-163, 4b393b3)

[kudkudak]

Those are R headers, not Rcpp header. Make sure makevars flags are being used in Makefile (I think this works on my branch so make sure you copied correctly Makevars).

R headers are pretty easy to get using R, so you can call Rscript to retrieve them (as in Makevars)

[ktalik]

As in commit message, 1 of 2 GNG tests is passing. @kudkudak is investigating error output: http://pastebin.com/eALr91vn (segmentation fault)

Starting CEC tests now.

[kudkudak]

Please merge GNG as if this error didn't exist. I will look into it over the weekend, but this failure is not connected to the tests themselves

[ktalik]

As in commits, tests part of Makefile is ready.

Meanwhile I started to think about library compilation. Why we want to do this? Since library compilation is provided by devtools I really don't see a point of doing this. Please explain me your needs so that I would know what to do and in what purpose.

[kudkudak]

I don't want to do this now as we have more pressing issues (SVMLight for instance), but:

• now we have to recompile whole library when one file is changed because R CMD INSTALL not handling that (at least I have this problem with GNG) - a good Makefile would include dependencies
• it might very useful to be able to compile parts of library to some side targets, for instance we might want to compile only GNG. Following this lines we might want target that will clean only GNG, etc.
• We might need faster compilation for developing C++ without attaching R (this is in fact 99% of compilations I was doing in GNG) and I had to write my own Makefile for that because R linking is pretty slow

So we can close this issue today (just give me a moment to look at the test compilation)

[kudkudak]

Comments to the makefile itself

• All overrides seem redundant. It even doesnt work on my machine as I don't have -larmadillo (and it worked just after erasing all overrides except -gtest -gtest_main -g and -s). I don't understand O1 and O2 - those are cancelling each other
• We should have separate binaries for each test, test_all is bad because there are some long running tests in the suite. I think it can be easily achieved?

EDIT: I forgot gtest_all is GoogleTest binary. Cool - so if you can add comment somewhere that you can execute single tests. Usage is given by

./test_all -h

I think it might be non-obvious as it was to me ;)

Maybe rename it to test_runner - as it suggests it has some options

Btw: GNG is fixed, it can be pulled from GNG@dev

[ktalik]

All overrides seem redundant

You were right about libraries. Only GTest's are needed. O1 and O2 too, they were just a copy-pastes of the previous Makefile. Same as -g and -s but I'm leaving -g, becouse maybe someone will need it (@sacherus?).

Maybe rename it to test_runner - as it suggests it has some options

I allowed myself to rename it to run_tests since it is more command-style (what a funny word).

All changes are in commit and are tested. Heading to GNG.

PS:

there are some long running tests in the suite

I think long tests should be separated from unit tests. We should have some ./run_unit_tests executable that will tests possibly largest code coverage in one execution and should last reasonable time.

[kudkudak]

Unfortunately for exactly unit testing machine learning algorithms sometimes need longer running tests, and this still unit tests its correctness.

We can compile everything into one binary, because it has many options for running tests. So makefile can create a script that uses this run_tests to select simple unit tests and I would suggest doing that

[ktalik]

OK, so lets have some convention in naming test cases and tests itself in order to filter them. For example in TwoESVMTest.TestPreprocessor we have TwoESVMTest which is a test case and TestPreprocessor which is a test (via GTest Documentation).

Most common convention is to name a tests case from a tested object (below example: Factorial -> FactorialTest). I think we could do it in transparent way by naming tests according to they type (in this case: Factorial -> FactorialUnitTest and e.g. Factorial -> FactorialIntegrationTest) but I'm not sure what are our test types and maybe we should disscuss that later.

[kudkudak]

I wouldnt care too much about this we can just have a script that will have baked in all the names. But you can enforce a convention if you have a complete idea (maybe on wiki page).

[kudkudak]

• @igorsieradzki @marcinMarcin Please change test file extension to .cpp from .cc for consistency with our source flle extension which is .cpp not .cc. I have changed that on this branch already.
• I have pulled gng sources (btw I just learned git checkout branch is a selective merge :p) and everything works
• In terms of naming we have two names group and test name. Group should be starting with subpackage name, and test should be starting with long if is a long test, this will be enough for now. We can change it later if we end up with hundreds of tests :D
• I made few minor changes, for instance cleaning was cleaning also gmum.R package objects which is kind of not expected from test Makefile
• I added constructing shell scripts for running longer and shorter unit tests. For now I just excluded my unit tests from short runner :p
• I tried to add CEC tests, but turned out that the tests are not ready :(

If everything is fine we can close it. We should move to next tasks asap

Sorry forgot to reference issue from commits

[ktalik]

I have changed that on this branch already.

Why are you requesting from them a change that you've already done?

We can change it later if we end up with hundreds of tests :D

... You of course meant that we should do that from beginning until we end up with that number.

I made few minor changes, for instance cleaning was cleaning also gmum.R package objects which is kind of not expected from test Makefile

We must revert that since, as you requested, this Makefile will also build a gmum.r library...

I tried to add CEC tests, but turned out that the tests are not ready :(

Yes, I dealt with that las week.

[kudkudak]

If they change that on their branch it will be easy to merge.

This makefile is not building gmum.R library and won't be building before release, so let's keep it this way..

We have to deal with more important issues now (for instance repository size) so we will come back to this disucssion after release if that's ok :), so let's close this issue

[ktalik]

Tests division fixes and improvements:

• Script generation didn't work: in order to parse newline character use printf instead of echo
• I divided tests to primary (fast and used in CI) and secondary (slow and precise)
• You can define many secondary tests types. For now I renamed BasicTests to NumericTest since excluding "basic tests" from "primary tests" sounded really funny ;)

I think thats it.

[ktalik]

@marcinMarcin, @kjurek , please checkout and pull branch task-163 and take a look at the current stage:

1. CEC tests are compiling and linking into main executable
2. If we run them separatedly e.g. ./run_tests --gtest_filter=Simple_1* they are doing OK
3. If we run all tests together: ./run_primary_tests.sh, there is a... yes, segfault (only few last lines output included).

Please guys (or someone) take a look at this...

[ktalik]

Done with the help of @kjurek