SVD batch and incremental tests are susceptible to random failures

[saketkc]

In context of https://github.com/Homebrew/homebrew-science/pull/1819

See: http://bot.brew.sh/job/Homebrew%20Science%20Pull%20Requests/1159/version=mavericks/testReport/junit/brew-test-bot/mavericks/install_Homebrew_science_mlpack/

I do not have a Mac to replicate this error, but this is preventing from mlpack being updated on homebrew.

tmp/mlpack-zmG7xJ/mlpack-mlpack-1.0.12/src/mlpack/tests/svd_batch_test.cpp:98: fatal error in "SVDBatchMomentumTest": critical check RMSE_2 <= RMSE_1 failed [0.9185867037696458 > 0.91157712642902167]
/tmp/mlpack-zmG7xJ/mlpack-mlpack-1.0.12/src/mlpack/tests/svd_batch_test.cpp:149: fatal error in "SVDBatchRegularizationTest": critical check RMSE_2 <= RMSE_1 failed [0.91787583628169389 > 0.91157712642902167]
/tmp/mlpack-zmG7xJ/mlpack-mlpack-1.0.12/src/mlpack/tests/svd_incremental_test.cpp:123: fatal error in "SVDIncompleteIncrementalRegularizationTest": critical check RMSE_2 < RMSE_1 failed [0.92918024945339284 >= 0.92886091107661617]

[dpo]

I can reliably reproduce those test errors on a Mac running 10.9.5.

[rcurtin]

As an update, I looked into the issue. It's not actually Mac-specific. Instead, the issue is more of a bad tolerance in those three failing tests. If I vary the random seed (which is, in these cases, set at compilation time), I sometimes get tests that pass and sometimes get tests that fail. For now what I would do in your packaging situation is to apply a patch that comments out those tests (either the entire test in svd_batch_test.cpp and svd_incremental_test.cpp, or just the lines that call BOOST_REQUIRE_*). I'm relatively sure the algorithms themselves will still work. Many of the mlpack tests are aimed more at ensuring that a code update does not break the functionality of the method, instead of ensuring that mlpack's algorithms work on every possible platform (in general, this is a lot harder to test and also failures of this type happen a lot less often).

I'm going to leave the ticket open until I fix the tests in master, but what I've written should be good enough to solve the Homebrew issue, I think.

[rcurtin]

Okay, so I think you can adapt the patch in 73b48dd to mlpack 1.0.12 and use that in Homebrew, and that should fix your issue.

[saketkc]

The patch wouldn't work straight away, since there are commits in between that and 1.0.12 tagged release. Do you plan to make a minor release, 1.0.13 maybe?

[rcurtin]

I don't plan to make a minor release for a change like this that doesn't actually affect functionality. You can easily take the changeset 73b48dd and adapt it to 1.0.12, since the actual meat of the code in svd_batch_test.cpp and svd_incremental_test.cpp is virtually identical in 1.0.12 and master. If there is a 1.0.13 release, I will incorporate this fix, of course, but there will need to be larger improvements to the overall package or some other more compelling reason. In this particular case you should have no problem building a patch to use with your Homebrew package.

[saketkc]

Fails on lion http://bot.brew.sh/job/Homebrew%20Science%20Pull%20Requests/1414/version=mountain_lion/testReport/junit/brew-test-bot/mountain_lion/install_Homebrew_science_mlpack/

[rcurtin]

This is not related to the issue you originally opened. Please open a new issue and detail what is going on more completely. I don't have a Mac, I don't use a Mac, I have little desire to own a Mac, and so I don't generally know what is going on when you simply tell me that things are failing.

Further, you aren't applying any patch like I suggested, so even if it did compile, you would still have the original problems.

[saketkc]

Ryan, I am applying the patch. I don't own a Mac either(I have already indicated this several times). It looks like it might be a clang issue.

I am going to probably close that PR. Thanks for your suggestions.

[rcurtin]

I think that the issue may be a clang issue, but again, I'd rather discuss that in a different issue for the sake of anyone who comes searching later.

As for applying the patch, what you are actually doing there is downloading mlpack master 73b48dd, which is significantly different from mlpack 1.0.12, and labeling it as mlpack 1.0.12. What I meant, originally, was that you can use git to generate a patch (something like this):

git create-patch 73b48dd

and then you can modify that patch until it applies to the mlpack-1.0.12 source. Then, as a step in the build process, you apply the patch you created (patch -p0 < the_patch.patch), and the test errors are fixed.