Suggestion: enforce unique GoogleTest names

[alashworth]

Description

Many tests in Stan Math have duplicate test names. You can check this with the appropriate regex | sort | uniq pipe, (e.g. rg -INo "TEST(_F|_P)?(\w*, \w*)" | sort | uniq -d)

Some tests also have the same name that are part of a test fixture, and some that are not. This is an error in GTest.

Expected:

All unit tests should have unique names.

Reason:

The GTest test runner supports filtering and running based on test name. This is also true for the CMake CTest runner. In general, when you run all the unit tests, having unique test names is necessary once you move beyond compiling individual test executables through runTests.py in order to easily see what tests are passing and failing. Also, it's a compilation error for two GTest tests to have the same name.

Eventually, I would like to support jumbo (unity) builds of the unit tests. Having unique test names is a prerequisite.

I used to maintain my own patches to the sources here, but it was an automated thing and the names were not chosen well, just the existing ones with 1 or 2 appended to the test, so I have discarded them. I would like to post this issue here just to let people know that if they make new tests, to please make sure the names are unique.

[rok-cesnovar]

I am not completely sure I understand what you mean. A few questions to clarify this One question:

Lets say we have a test
TEST(MathMatrix, inverse_exception)

Is the problem that we have the exact same TEST(MathMatrix, inverse_exception) in an unrelated file?

If that is the case we should clean that up, definitely. And then add the suggested regex | sort check to runChecks.py so PRs cant get merged if they have non-unique names.

[alashworth]

Yes. The unit tests currently have many unrelated* unit tests in different translation units with the same GTest name (i.e. MathMatrix, inverse_exception).

*The tests are actually related usually, some variant on some Stan functionality. Somebody probably cut and paste to make these tests, and then did a few edits.

It's a problem for me when I aggregate all the test results. I get a bunch of tests with the same name appearing in my logs.

Also that regex might not be correct. There are lots of tests with numbers in them. Maybe \w should not be used.

[alashworth]

Current result of running that regex on my copy of Stan math (might be outdated):

TEST(AgradAutoDiff, gradient)
TEST(AgradFwdAbs, abs_NaN)
TEST(AgradFwdAtan2, nan)
TEST(AgradFwdBesselFirstKind, bessel_first_kind_NaN)
TEST(AgradFwdBesselSecondKind, bessel_second_kind_NaN)
TEST(AgradFwdBinaryLogLoss, binary_log_loss_NaN)
TEST(AgradFwdBinomialCoefficientLog, nan)
TEST(AgradFwdFdim, nan)
TEST(AgradFwdFma, fma_NaN)
TEST(AgradFwdFmax, nan)
TEST(AgradFwdFmin, nan)
TEST(AgradFwdFmod, nan)
TEST(AgradFwdFvar, insertion_operator)
TEST(AgradFwdGammaQ, nan)
TEST(AgradFwdHypot, nan)
TEST(AgradFwdLbeta, nan)
TEST(AgradFwdLmgamma, lmgamma_NaN)
TEST(AgradFwdLogDiffExp, nan)
TEST(AgradFwdLogFallingFactorial, nan)
TEST(AgradFwdLogMix, log_mix_NaN)
TEST(AgradFwdLogRisingFactorial, nan)
TEST(AgradFwdLogSumExp, nan)
TEST(AgradFwdMatrixSum, ffd_vector)
TEST(AgradFwdModifiedBesselFirstKind, modified_bessel_first_kind_NaN)
TEST(AgradFwdModifiedBesselSecondKind, modified_bessel_second_kind_NaN)
TEST(AgradFwdMultiplyLog, nan)
TEST(AgradFwdOwensT, nan)
TEST(AgradFwdPow, nan)
TEST(AgradFwdSign, Fvar)
TEST(AgradMatrix, value_of_rec)
TEST(AgradRev, check_varis_on_stack)
TEST(AgradRev, unaryNot)
TEST(AgradRev, value_of_rec)
TEST(AgradRevErrorHandlingMatrix, checkPosDefiniteMatrix_nan)
TEST(AgradRevErrorHandlingScalar, CheckConsistentSizeVarCheckVectorized)
TEST(AgradRevErrorHandlingScalar, CheckFiniteVarCheckVectorized)
TEST(AgradRevErrorHandlingScalar, CheckPositive)
TEST(AgradRevMatrix, check_varis_on_stack)
TEST(AgradRevMatrix, columns_dot_self)
TEST(AgradRevMatrix, crossprod)
TEST(AgradRevMatrix, cumulative_sum)
TEST(AgradRevMatrix, diagMatrix)
TEST(AgradRevMatrix, dot_product_dv)
TEST(AgradRevMatrix, dot_product_dv_vec)
TEST(AgradRevMatrix, dot_product_vd)
TEST(AgradRevMatrix, dot_product_vd_vec)
TEST(AgradRevMatrix, dot_product_vv)
TEST(AgradRevMatrix, dot_product_vv_vec)
TEST(AgradRevMatrix, dot_self_vec)
TEST(AgradRevMatrix, fill)
TEST(AgradRevMatrix, fillDouble)
TEST(AgradRevMatrix, initializeVariable)
TEST(AgradRevMatrix, log_deteriminant_exception)
TEST(AgradRevMatrix, log_determinant_grad)
TEST(AgradRevMatrix, meanStdVector)
TEST(AgradRevMatrix, multiply_vector_int)
TEST(AgradRevMatrix, multiplyLowerTriSelfTranspose)
TEST(AgradRevMatrix, multiplyLowerTriSelfTransposeGrad1)
TEST(AgradRevMatrix, multiplyLowerTriSelfTransposeGrad2)
TEST(AgradRevMatrix, multiplyLowerTriSelfTransposeGrad3)
TEST(AgradRevMatrix, prod)
TEST(AgradRevMatrix, sdStdVector)
TEST(AgradRevMatrix, softmax)
TEST(AgradRevMatrix, tcrossprod)
TEST(AgradRevMatrix, tcrossprodGrad1)
TEST(AgradRevMatrix, tcrossprodGrad2)
TEST(AgradRevMatrix, tcrossprodGrad3)
TEST(AgradRevMatrix, UserCase1)
TEST(AgradRevMatrix, varianceStdVector)
TEST(ErrorHandlingMatrix, checkNonzeroSizeMatrix)
TEST(ErrorHandlingMatrix, checkNonzeroSizeMatrix_nan)
TEST(ErrorHandlingMatrix, checkSizeMatch)
TEST(ErrorHandlingMatrix, checkVectorMatrix)
TEST(ErrorHandlingMatrix, checkVectorMatrix_nan)
TEST(ErrorHandlingScalar, CheckFinite_nan)
TEST(ErrorHandlingScalar, CheckGreater_nan)
TEST(ErrorHandlingScalar, CheckGreaterOrEqual_nan)
TEST(ErrorHandlingScalar, CheckLessOrEqual_nan)
TEST(ErrorHandlingScalar, CheckNonnegative_nan)
TEST(ErrorHandlingScalar, CheckPositive)
TEST(ErrorHandlingScalar, CheckPositive_nan)
TEST(ErrorHandlingScalar, CheckPositiveFinite_nan)
TEST(MathFunctions, fma_nan)
TEST(MathFunctions, is_any_nan_variadic)
TEST(MathFunctions, log_sum_exp)
TEST(MathFunctions, sign)
TEST(MathFunctions, sub_nan)
TEST(MathMatrix, diagPostMultiplyException)
TEST(MathMatrix, diagPreMultiply2_dv)
TEST(MathMatrix, diagPreMultiplyException)
TEST(MathMatrix, failing_pre_20)
TEST(mathMatrix, getLp)
TEST(MathMatrix, initMatrix)
TEST(MathMatrix, inverse_exception)
TEST(MathMatrix, matrix_exp_100x100)
TEST(MathMatrix, matrix_exp_1x1)
TEST(MathMatrix, matrix_exp_25x25)
TEST(MathMatrix, matrix_exp_2x2)
TEST(MathMatrix, matrix_exp_2x2_1)
TEST(MathMatrix, matrix_exp_2x2_2)
TEST(MathMatrix, matrix_exp_3x3)
TEST(MathMatrix, matrix_exp_exceptions)
TEST(MathMatrix, matrix_exp_multiply_exception)
TEST(MathMatrix, matrix_exp_pade_1x1)
TEST(MathMatrix, matrix_exp_pade_2x2)
TEST(MathMatrix, matrix_exp_pade_3x3)
TEST(MathMatrix, qr_Q)
TEST(MathMatrix, qr_R)
TEST(MathMatrix, quadFormDiagException)
TEST(MathMatrix, scale_matrix_exp_multiply_exception)
TEST(MathMatrix, softmax)
TEST(MathMatrix, softmax_exception)
TEST(MathMatrix, sortAscEigenRowVecNan)
TEST(MathMatrix, sortAscEigenVecNan)
TEST(MathMatrix, sortAscStdVecNan)
TEST(MathMatrix, sortDescEigenRowVecNan)
TEST(MathMatrix, sortDescEigenVecNan)
TEST(MathMatrix, sortDescStdVecNan)
TEST(MathMatrix, value_of)
TEST(MathMatrix, value_of_rec)
TEST(MathMeta, index_type)
TEST(MathMeta, is_std_vector)
TEST(MathMeta, is_var_or_arithmetic_simple)
TEST(MathMeta, primitive)
TEST(MathMeta, value_type)
TEST(MathPrimMat, calculations)
TEST(MathPrimMat, calculations_ard)
TEST(MathPrimMat, check_dim_mismatch)
TEST(MathPrimMat, domain_err_training_sig_l_gamma)
TEST(MathPrimMat, domain_error_training_sig_l)
TEST(MathPrimMat, nan_error_training_sig_l)
TEST(MathPrimMat, nan_error_training_sig_l_gamma)
TEST(MathPrimMat, vec_double_gp_matern52_cov1)
TEST(MathPrimMat, zero_size)
TEST(mathPrimScalProbWiener, illegal_tau_gt_y)
TEST(MetaTraits, contains_std_vector_false)
TEST(MetaTraits, contains_vector)
TEST(MetaTraits, contains_vector_true)
TEST(MetaTraits, containsFvar)
TEST(MetaTraits, get)
TEST(MetaTraits, is_fvar)
TEST(MetaTraits, is_var)
TEST(MetaTraits, is_vector)
TEST(MetaTraits, isConstant)
TEST(MetaTraits, isConstantStruct)
TEST(MetaTraits, length)
TEST(MetaTraits, length_mvt)
TEST(MetaTraits, partials_return_type)
TEST(MetaTraits, partials_type)
TEST(MetaTraits, size_of)
TEST(MetaTraits, StdVectorBuilder_false_false)
TEST(MetaTraits, StdVectorBuilder_true_false)
TEST(MetaTraits, StdVectorBuilder_type_check)
TEST(MetaTraits, VectorBuilder_false_false)
TEST(MetaTraits, VectorBuilder_false_true)
TEST(MetaTraits, VectorBuilder_true_false)
TEST(MetaTraits, VectorBuilder_true_true)
TEST(MetaTraits, VectorBuilderHelper_false_false)
TEST(MetaTraits, VectorBuilderHelper_false_true)
TEST(MetaTraits, VectorBuilderHelper_true_false)
TEST(MetaTraits, VectorBuilderHelper_true_true)
TEST(MultiNormal, TestGradFunctionalVectorized)
TEST(prob_transform, lkj_cov_matrix_free_exception)
TEST(prob_transform, lkj_cov_matrix_rt)
TEST(ProbBernoulliLogit, log_matches_lpmf)
TEST(ProbDistributionsLkjCorr, Sigma)
TEST(ProbDistributionsLkjCorr, testHalf)
TEST(ProbDistributionsLkjCorr, testIdentity)
TEST(ProbDistributionsLogNormal, chiSquareGoodnessFitTest)
TEST(ProbDistributionsNegBinomial, chiSquareGoodnessFitTest)
TEST(ProbDistributionsNegBinomial, chiSquareGoodnessFitTest2)
TEST(ProbDistributionsNegBinomial, chiSquareGoodnessFitTest3)
TEST(ProbDistributionsNegBinomial, error_check)
TEST(ProbDistributionsParetoType2, chiSquareGoodnessFitTest)
TEST(StanAgradRevOde_integrate_ode, harmonic_oscillator_error)
TEST(StanAgradRevOde_integrate_ode, harmonic_oscillator_finite_diff)
TEST(StanMath_integrate_1d, test_integer_arguments)
TEST(StanMathOdeIntegrateODEGrad, double_var)
TEST(StanMathOdeIntegrateODEGrad, var_double)
TEST(StanMathOdeIntegrateODEGrad, var_var)
TEST_F(agrad_distributions_multi_normal, Propto)
TEST_F(agrad_distributions_multi_normal, ProptoMu)
TEST_F(agrad_distributions_multi_normal, ProptoMuSigma)
TEST_F(agrad_distributions_multi_normal, ProptoSigma)
TEST_F(agrad_distributions_multi_normal, ProptoY)
TEST_F(agrad_distributions_multi_normal, ProptoYMu)
TEST_F(agrad_distributions_multi_normal, ProptoYSigma)
TEST_F(agrad_distributions_multi_normal_multi_row, Propto)
TEST_F(agrad_distributions_multi_normal_multi_row, ProptoMu)
TEST_F(agrad_distributions_multi_normal_multi_row, ProptoMuSigma)
TEST_F(agrad_distributions_multi_normal_multi_row, ProptoSigma)
TEST_F(agrad_distributions_multi_normal_multi_row, ProptoY)
TEST_F(agrad_distributions_multi_normal_multi_row, ProptoYMu)
TEST_F(agrad_distributions_multi_normal_multi_row, ProptoYSigma)
TEST_F(ErrorHandlingScalar_out_of_range, double)

[rok-cesnovar]

Great, thank you for the list.

I understand its currently limited to your case, but I imagine it could be enforced on the Stan Math level. I will clean this up shortly for the current Math codebase.

How faster is the compile time for a jumbo build compared to what we do now? Do you have any ballpark figures?

[alashworth]

It's much faster, so long as you have the memory to support it. Like at least 50%, maybe more.

It's too much work to to have it until the new Stan compiler comes, because of difficulties with the Stan language module and Boost Spirit (I tried...). But you can have something similar for your development work by using the ZapCC Clang compiler fork (https://github.com/yrnkrn/zapcc). It caches template instantiations for you - it's really kind of awesome.

[syclik]

@alashworth, can you verify that this has a chance of working on Windows? We've had to break apart tests into multiple translation units for Windows due to the limitations of the compiler. (It's really bad... it fails silently under some conditions... it will generate an executable, but it won't have anything in it.)

[alashworth]

Much like the distribution tests, a full jumbo build will not be possible. It is possible to automatically adjust this per-platform, though. Especially once the project starts being built with CMake - there is functionality to do this already baked in in certain widely-used 3rd-party modules (Cotire - https://github.com/sakra/cotire). Also the CMake develop branch is gaining support for PCH and unity builds soon too.

I am very aware of things Stan does on Windows 🙅‍♂. My workstation is a Windows machine, so I test first on MinGW, then spin up a virtual machine to test on Linux.