Feature/2431 lang tuples step1

[mitzimorris]

Submission Checklist

• Run unit tests: ./runTests.py src/test/unit
• Run cpplint: make cpplint
• Declare copyright holder and open-source license: see below

Summary

Refactor of stan/lang AST needed in order to extend Stan language to tuples. See issue #2431 for details.

Intended Effect

Everything should work exactly as before.

How to Verify

Run all unit tests.

Side Effects

Changes to the AST nodes required changes to the generator code. Previously the AST contained a set of variant types for variable declarations, with the result that the generator code required many vistor classes to generate the series of c++ statements which correspond to a single Stan language variable declaration or compound declaration-definition. In rewriting the generator methods, I changed the generated code itself in order to remove duplicate checks on container sizes and improve overall readability. Significant changes to the generated code:

1. Previously first all variable dimensions were validated, then variables were initialized. Now validation is followed directly by initialization, which means that some failing programs may fail differently.

2. Double literal variables now store the actual string value from the program. Previously, generated c++ code used the stored double value, e.g. double literal "5.1" would be initialized to not exactly 5.1, depending on the compiler. Now the string value "5.1" is generated.

3. spaces after commas in args, to improve readibility.

Documentation

N/A.

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Columbia University

By submitting this pull request, the copyright holder is agreeing to 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/)

[syclik]

any idea why this would fail and not be picked up? https://travis-ci.org/stan-dev/stan/jobs/405140359#L670 Either way, we're probably touching on a compiler bug. Meaning we'll need to be able to run that exact compiler in order to hunt this one down.

…

On Wed, Jul 18, 2018 at 9:20 AM Mitzi Morris ***@***.***> wrote: works fine on my machine, and works on travis except for 2 compilers https://travis-ci.org/stan-dev/stan/builds/405140355?utm_source=github_status&utm_medium=notification — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2498 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAZ_F8k1-HVCxaiC_e7QpkrHahRMAZBDks5uHzUKgaJpZM4S6eCd> .

[syclik]

I take it back. I just started clean from your branch and I see a failure: $ ./runTests.py src/test/unit/variational/advi_multivar_no_constraint_test.cpp ... (lots of compilation herre) ------------------------------------------------------------ test/unit/variational/advi_multivar_no_constraint --gtest_output="xml:test/unit/variational/advi_multivar_no_constraint.xml" Running main() from gtest_main.cc [==========] Running 2 tests from 1 test case. [----------] Global test environment set-up. [----------] 2 tests from advi_test [ RUN ] advi_test.multivar_no_constraint_fullrank unknown file: Failure C++ exception with description "Exception: vector[uni] assign range: accessing element out of range. index 1 out of range; expecting index to be between 1 and 0 (in 'src/test/test-models/good/variational/multivariate_no_constraint.stan' at line 6) " thrown in the test body. [ FAILED ] advi_test.multivar_no_constraint_fullrank (0 ms) [ RUN ] advi_test.multivar_no_constraint_meanfield unknown file: Failure C++ exception with description "Exception: vector[uni] assign range: accessing element out of range. index 1 out of range; expecting index to be between 1 and 0 (in 'src/test/test-models/good/variational/multivariate_no_constraint.stan' at line 6) " thrown in the test body. [ FAILED ] advi_test.multivar_no_constraint_meanfield (0 ms) [----------] 2 tests from advi_test (0 ms total) [----------] Global test environment tear-down [==========] 2 tests from 1 test case ran. (0 ms total) [ PASSED ] 0 tests. [ FAILED ] 2 tests, listed below: [ FAILED ] advi_test.multivar_no_constraint_fullrank [ FAILED ] advi_test.multivar_no_constraint_meanfield 2 FAILED TESTS test/unit/variational/advi_multivar_no_constraint --gtest_output="xml:test/unit/variational/advi_multivar_no_constraint.xml" failed exit now (07/18/18 09:32:55 EDT)

…

On Wed, Jul 18, 2018 at 9:23 AM Daniel Lee ***@***.***> wrote: any idea why this would fail and not be picked up? https://travis-ci.org/stan-dev/stan/jobs/405140359#L670 Either way, we're probably touching on a compiler bug. Meaning we'll need to be able to run that exact compiler in order to hunt this one down. On Wed, Jul 18, 2018 at 9:20 AM Mitzi Morris ***@***.***> wrote: > works fine on my machine, and works on travis except for 2 compilers > > > https://travis-ci.org/stan-dev/stan/builds/405140355?utm_source=github_status&utm_medium=notification > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#2498 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AAZ_F8k1-HVCxaiC_e7QpkrHahRMAZBDks5uHzUKgaJpZM4S6eCd> > . >

[mitzimorris]

src/test/unit/services/util/generate_transitions_test.cpp:7:10: fatal error: 'test/test-models/good/optimization/rosenbrock.hpp' file not found

that could be a missing dependency in the make/tests makefile - I'll check.

[syclik]

It's a red herring. The model in the advi test is failing to run. I think it's a bug. Look at this Stan program: src/test/test-models/good/variational/multivariate_no_constraint.stan If you copy that into CmdStan with this branch, it doesn't run. $ ./multivariate_no_constraint sample method = sample (Default) sample num_samples = 1000 (Default) num_warmup = 1000 (Default) save_warmup = 0 (Default) thin = 1 (Default) adapt engaged = 1 (Default) gamma = 0.050000000000000003 (Default) delta = 0.80000000000000004 (Default) kappa = 0.75 (Default) t0 = 10 (Default) init_buffer = 75 (Default) term_buffer = 50 (Default) window = 25 (Default) algorithm = hmc (Default) hmc engine = nuts (Default) nuts max_depth = 10 (Default) metric = diag_e (Default) metric_file = (Default) stepsize = 1 (Default) stepsize_jitter = 0 (Default) id = 0 (Default) data file = (Default) init = 2 (Default) random seed = 3033418282 output file = output.csv (Default) diagnostic_file = (Default) refresh = 100 (Default) Exception: vector[uni] assign range: accessing element out of range. index 1 out of range; expecting index to be between 1 and 0 (in 'multivariate_no_constraint.stan' at line 6)

…

On Wed, Jul 18, 2018 at 9:42 AM Mitzi Morris ***@***.***> wrote: src/test/unit/services/util/generate_transitions_test.cpp:7:10: fatal error: 'test/test-models/good/optimization/rosenbrock.hpp' file not found that could be a missing dependency in the make/tests makefile - I'll check. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2498 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAZ_FzTw96g6IQy2K83Pl21Fn9dyvt3pks5uHzsWgaJpZM4S6eCd> .

[mitzimorris]

right - that is the error.

but regarding the other, it does seem that file make/tests needs a fix as well -

src/test/unit/services/util/generate_transitions_test.cpp: src/test/test-models/good/services/test_lp.hpp src/test/test-models/good/optimization/rosenbrock.hpp

[syclik]

well... the big issue is that this is actually a problem and it can be reproduced using the Apple XCode clang++. Is that enough info to track it down?

…

On Wed, Jul 18, 2018 at 9:48 AM Mitzi Morris ***@***.***> wrote: right - that is the error. but regarding the other, it does seem that file make/tests needs a fix as well - src/test/unit/services/util/generate_transitions_test.cpp: src/test/test-models/good/services/test_lp.hpp src/test/test-models/good/optimization/rosenbrock.hpp — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2498 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAZ_F3PTm_dZrYoDtsn0QoDN4N_wewM_ks5uHzySgaJpZM4S6eCd> .

[mitzimorris]

thanks @syclik - that's a legit error - investigating

[seantalts]

Hey, this broke existing models and literally doesn't return all of the parameters for the SIR model. I thought this was a Math issue first, see stan-dev/math#972

Should we revert while this undergoes more testing?

[mitzimorris]

yes, absolutely.

[mitzimorris]

@seantalts which models besides SIR model are problematic?

[bob-carpenter]

Where are the tests that uncovered this?

[seantalts]

I don't know the full extent of models that have issues because my test program is currently stopping at the first error like this it encounters (I'm trying to fix that but I'm tied up all day with Rok and Erik).

The tests run here: http://d1m1s1b1.stat.columbia.edu:8080/blue/organizations/jenkins/CmdStan%20Performance%20Tests/detail/master/214/pipeline/

The repo (with instructions) is here: https://github.com/stan-dev/performance-tests-cmdstan

[mitzimorris]

there's an error with the indexing logic - for the SIR model, list of parameters looks like:

y.1.1,y.2.1,y.3.1,y.4.1

where it should be

y.1.1, y.1.2, y.1.3, y.1.4,

the underlying problem is that we need more tests on generator - the Stan test suite currently doesn't do much end-to-end testing, other than tests for advi.
the benchmark tests are useful as good tests on the generator - some subset of them should be added to the Stan test suite.

[seantalts]

Nice find. Just to copy slightly relevant info from the other thread:
I don't know the correct name, but these tests also test that HMC is still recovering means within 30% of the old estimate. Similar to the stat comp benchmarks.

[mitzimorris]

I've made a lot of mistakes in doing this PR. I should have had the discipline to leave the generated .hpp code exactly as is and not change the output to be more readable and save that for a second PR. mea culpa.

As a way forward, I'm writing a test plan. The specific bug that the benchmarks test uncovered is that the new generator code is getting mixed up on the order of array indexes. This isn't surprising - the whole point of this PR is to refactor the way that dimensions on container types are represented in the AST. Here is what I have so far w/r/t a test plan for current bug:

Multi-dimensional block-level variables in Stan programs.

Block variables can have constraints/specialized types;
constraints enforced in program block in which they occur.
Block variables are declared with sizes.

Example: matrix<lower=1,upper=1>[2,3] ar_mat[4,5];

Verify that refactor contains tests for all AST variable decl nodes and type:

• variable ar_mat is block_var_decl

• block_var_decl.type() = array_block_type

• array_block_type.element_type() is array_block_type

• array_dims = 2

• array_lens = [4,5]

• array_block_type contains matrix_type

• matrix_type has bounds

• matrix_type M_ (arg1) = 2

• matrix_type N_ (arg2) = 3

Multi-dimensional local variables in Stan programs.

Local variables are declared with sizes.

functions {
 function void foo() {
  matrix[2,3] local_ar_mat[4,5];
 }
}

Verify that refactor contains tests for all AST variable decl nodes and type:

• variable ar_mat is local_var_decl

• local_var_decl.type() = array_local_type

• array_local_type.element_type() is array_local_type

• array_dims = 2

• array_lens = [4,5]

• array_local_type contains matrix_type

• matrix_type M_ (arg1) = 2

• matrix_type N_ (arg2) = 3

Multi-dimensional bare variables in Stan programs

Bare type array variables are declared with number of dimensions.
Bare type variables don't have constraints or sizes.
Used in function declarations

functions {
 function void foo(matrix ar_mat[,]) {
   int a = 0;
 }
}

Program 1: data block only, single variable decl:

data {
 matrix<lower=1,upper=1>[2,3] ar_mat[4,5];
 }

Parser tests: AST for entire program as expected

Generator tests:

check .hpp file for:

• generated code for variable declation
• generated code for variable initialization

program end-to-end test:

• run program, good inputs
• run program, inputs wrong shape
• run program, inputs correct shape, violate bounds constraint

Program 2: transformed data block only, single variable decl:

transformed data {
 matrix<lower=1,upper=1>[2,3] ar_mat[4,5];
}

check .hpp file for:

• generated code for variable declation
• generated code for variable initialization

program end-to-end test:

• run program - should run

Programs 3,4,5 : parameters / transformed parameters /generated quantities block only:

parameters {
 matrix<lower=1,upper=1>[2,3] ar_mat[4,5];
}

check .hpp file for:

• generated code for variable declation
• generated code for variable initialization

program end-to-end test:

• run program - should run
• output should list all parameters in order:
  ar_mat.1.1.1.1, ar_mat.1.1.1.2, ... ar_mat.4.5.2.2, ar_mat.4.5.2.3

is there a GitHub way to add test plans to PRs?
comments on this one?

[mitzimorris]

filed issue #2604 - fixing on branch bugfix/2406-generator-multidim-params