Update expression tests code generation #2419
Conversation
|
@t4c1 can you give @rok-cesnovar the way we would run this is: And then this to print the function names that are compatible: It takes a long time so this is something we'd put in a GHA or something (definitely not run every merge -- it has to compile one file for every signature). Anyway I still gotta convert this all back to python2 and maybe shuffle the files a bit and add docs, so WIP (Edit: not really ready for full, careful review). |
…into feature/varmat-signatures
Ooo, okay that is convenient thanks. |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
|
@t4c1 This is ready for review again |
|
Any idea why the new expr. tests take 2x the time as the previous expressions tests? I am not saying its a deal breaker as its 10 minutes, just wondering what changed? |
I think the code generation is less efficient (double the arguments are initialized). I'm not sure. I did a test and the new code gen generates 52008 tests. The old code generates 52021 tests. |
|
@t4c1 I updated the comments. Re: benchmarking, I want to merge this on its own, and then the next pull is a script to automatically detect which functions can compile with varmat variables or not (it is just 2 scripts). I haven't looked at the benchmarking script yet. It probably wouldn't be too much trouble to clean it up (maybe a day to convert?) while we're on the topic of code generation. |
Any idea why there is a difference at all? I think we should understand what is happening here. |
Yeah I was curious too. The list of functions affected is is_cholesky_factor, is_matching_dims, multinomial_log, multinomial_lpmf Edit: Also multinomial_rng It looks like the old code is doing codegen for the same signatures twice somehow. Like here are two functions it generates: TEST(ExpressionTestPrim, is_cholesky_factor0) {
Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic> arg_mat0 = stan::test::make_arg<Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>>();
auto res_mat = stan::math::is_cholesky_factor(arg_mat0);
Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic> arg_expr0 = stan::test::make_arg<Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>>();
int counter0 = 0;
stan::test::counterOp<double> counter_op0(&counter0);
auto res_expr = stan::math::is_cholesky_factor(arg_expr0.block(0,0,1,1).unaryExpr(counter_op0));
EXPECT_STAN_EQ(res_expr, res_mat);
EXPECT_LE(counter0, 1);
}
TEST(ExpressionTestPrim, is_cholesky_factor1) {
Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic> arg_mat0 = stan::test::make_arg<Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>>();
auto res_mat = stan::math::is_cholesky_factor(arg_mat0);
Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic> arg_expr0 = stan::test::make_arg<Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic>>();
int counter0 = 0;
stan::test::counterOp<double> counter_op0(&counter0);
auto res_expr = stan::math::is_cholesky_factor(arg_expr0.block(0,0,1,1).unaryExpr(counter_op0));
EXPECT_STAN_EQ(res_expr, res_mat);
EXPECT_LE(counter0, 1);
}I assume this means that one of the sets of special tests is causing the functions to get added twice. |
|
Looks like signatures = get_signatures()
len(signatures)
26982
len(set(signatures))
26976New code pushes everything through sets so it doesn't show this. Edit: Duplicates are: ['multinomial_log(array[] int, vector) => real\n'
'gp_dot_prod_cov(array[] real, array[] real, real) => matrix\n'
'is_cholesky_factor(matrix) => int', 'is_matching_dims(matrix, matrix) => int'
'multinomial_rng(vector, int) => array[] int\n'
'multinomial_lpmf(array[] int, vector) => real\n']Edit: One of these things must be duplicated more than once for the difference in length to be 6 |
|
Here's new code vs. old code. New code is more lines. Isn't obvious to me why it takes so much longer, but I guess all the little New code: TEST(ExpressionTestRev, multinomial_lpmf3) {
/*
multinomial_lpmf(array[] int, vector) => real
*/
int int0 = 1;
std::vector<decltype(int0)> array0 = {int0};
auto simplex1 = stan::test::make_simplex<Eigen::Matrix<stan::math::var, Eigen::Dynamic, 1>>(0.4, 1);
int simplex1_expr2_counter = 0;
stan::test::counterOp<stan::math::var> simplex1_expr2_counter_op(&simplex1_expr2_counter);
auto simplex1_expr2 = simplex1.segment(0,1).unaryExpr(simplex1_expr2_counter_op);
int int3 = 1;
std::vector<decltype(int3)> array3 = {int3};
auto simplex4 = stan::test::make_simplex<Eigen::Matrix<stan::math::var, Eigen::Dynamic, 1>>(0.4, 1);
auto result5 = stan::math::eval(stan::math::multinomial_lpmf(array0,simplex1_expr2));
auto result6 = stan::math::eval(stan::math::multinomial_lpmf(array3,simplex4));
EXPECT_STAN_EQ(result5,result6);
EXPECT_LEQ_ONE(simplex1_expr2_counter);
auto summed_result7 = stan::math::eval(stan::test::recursive_sum(result5));
auto summed_result8 = stan::math::eval(stan::test::recursive_sum(result6));
auto sum_of_sums9 = stan::math::eval(stan::math::add(summed_result7,summed_result8));
stan::test::grad(sum_of_sums9);
stan::test::expect_adj_eq(array3,array0);
stan::test::expect_adj_eq(simplex4,simplex1_expr2);
stan::math::recover_memory();
}Old code: TEST(ExpressionTestRev, multinomial_log1) {
std::vector<int> arg_mat0 = stan::test::make_arg<std::vector<int>>();
Eigen::Matrix<stan::math::var, Eigen::Dynamic, 1> arg_mat1 = stan::test::make_simplex<Eigen::Matrix<stan::math::var, Eigen::Dynamic, 1>>();
auto res_mat = stan::math::multinomial_log(arg_mat0, arg_mat1);
std::vector<int> arg_expr0 = stan::test::make_arg<std::vector<int>>();
Eigen::Matrix<stan::math::var, Eigen::Dynamic, 1> arg_expr1 = stan::test::make_simplex<Eigen::Matrix<stan::math::var, Eigen::Dynamic, 1>>();
int counter1 = 0;
stan::test::counterOp<stan::math::var> counter_op1(&counter1);
auto res_expr = stan::math::multinomial_log(arg_expr0, arg_expr1.segment(0,1).unaryExpr(counter_op1));
EXPECT_STAN_EQ(res_expr, res_mat);
EXPECT_LE(counter1, 1);
(stan::test::recursive_sum(res_mat) + stan::test::recursive_sum(res_expr)).grad();
EXPECT_STAN_ADJ_EQ(arg_expr0,arg_mat0);
EXPECT_STAN_ADJ_EQ(arg_expr1,arg_mat1);
} |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
|
@t4c1 ready for review again |
|
I looked at the comparison of old and new tests. All those |
|
@t4c1 Ready again |
When I call a function I evaluate the results to make sure all the returns are not expressions (and do something unexpected). Since the python here isn't generating performance sensitive code, I preferred to hide expressions with evals (edited). This might change if we did the benchmarks, but it is easy enough to change.
I just did the first thing that came to mind. Coulda done it as a function. Could have also initialized an IntVariable and called a two argument macro, but this seemed to fit well enough with how we do other bits of code. |
We don't have scalar expressions. So these evals never do anything.
I dont like it. Calling two argument macro makes most sense to me. I don't think we define macros anywhere else just to hide one argument. |
Switched
Right, but I wanted |
|
Oh, I see. I just thought that might have been the reason for the slow down of expression tests. |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
Summary
I took the varmat test stuff out of this. It is currently just a cleanup to the code generation for the expressions tests (and this will feed into the varmat stuff). It makes the pull smaller.
Tests
None yet
Release notes
Checklist
Math issue How to add static matrix? #1805
Copyright holder: Columbia University
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested