Parallel reduce_sum

[rok-cesnovar]

This is a draft and a very brute force of making a prototype for parallel reduce_sum.

Any and all comments are welcome.

The main things is that I now duplicate the functors, the duplicate has the pstream as the third argument.

The other thing is supporting functions with variadic arguments.

// f is the reducer function
real f(int start, int end, sliced_type sub_slice_container, ...);

// parallel reduce
real reduce_sum(real(int, int, sliced_type, ...) f,
                int grainsize, sliced_type large_container, ...);

... can be 0 to N arguments of any type.
For now I just brute force my way to support 0 to 3 arguments of all base types and their 1D arrays.

[rok-cesnovar]

This model compiles in cmdstan for now:

functions {
  real my_func(int start, int end, real[] y_slice, real mu, real sigma) {
    return normal_lpdf(y_slice | mu, sigma);
  }
}

parameters {
  real a[5];
}

model {
  target += reduce_sum(my_func, a, 1, 0.0, 1.0);
}

The other model I tried to compile was this one https://gist.github.com/rok-cesnovar/832f9752fc2a8ba7ae7b3da3eb5f2743

but that spits out a bunch for ambigious functions warnings that I currently dont understand yet, example shown in the gist comment.

[rok-cesnovar]

v0.1 stanc3 with parallel_reduce binary for linux if anyone wants to test (unzip and move binary in cmdstan).
stanc.zip

[bbbales2]

OOoooooon it!

[bbbales2]

I got:

./stanc: /lib/x86_64-linux-gnu/libm.so.6: version `GLIBC_2.29' not found (required by ./stanc)

When I tried to run the binary. How hard is it to build stanc?

[rok-cesnovar]

It does take some installing of all the required ocaml, dune, etc. But after you have that its a piece of cake. Weird that it doesnt run. I will check how Jenkins builds it for the release and we can try again.

[rok-cesnovar]

Please try this. I think this should run.
v0.1.1
stanc.zip

[rok-cesnovar]

I goofed the original functor arguments, it doesnt hurt the your example with normal. Cleaned that up now.

The hierachiral reduce models still doesnt compile in c++ tho.

stanc_parallel_reduce_sum_v0.2.zip

p.s.: dont get your hopes up too much. Learning on the job :)

[bbbales2]

Yeah I got a basic normal running. Those errors you posted look like the result of me not defining the count_var_impl_ correctly -- I hit those in the ODE stuff and can fix em' easy.

Edit: Of course, it could be something else

[bbbales2]

Alright I think it's working for a basic normal example for me :D. I'll see if I can get the poisson thing to compile.

Yo Bob said the print function would have the variadic argument stuff in the stanc3 compiler. Is that something you can copy-paste over to this?

[bbbales2]

parallel_reduce_test_model compiles for me.

[bbbales2]

I posted some code over here: https://gist.github.com/rok-cesnovar/832f9752fc2a8ba7ae7b3da3eb5f2743

You might be able to add something like require_not_t<is_var<T>>..., accumulate_adjoints, count_var_impl_, and save_varis_ and make the error you're getting go away. I don't know if my compiler is right to not complain or yours is right to complain (vs. the C++ standard).

[bbbales2]

I got the same errors you did on gcc and added the require_not_ts and it worked.

[rok-cesnovar]

Yo Bob said the print function would have the variadic argument stuff in the stanc3 compiler. Is that something you can copy-paste over to this?

Yeah, tried looking at that, but print isnt implemented as the rest of them so not trivial for me. Might be for the real gurus :) Will try if this brute force approach looks promising.

parallel_reduce_test_model compiles for me.

Nice!

I posted some code over here:

Thanks, saw the comment yeah. I am using g++ 8.3. Just switched to clang and that works and that is fine with me :)

[bbbales2]

Thanks, saw the comment yeah. I am using g++ 8.3. Just switched to clang and that works and that is fine with me :)

I'll push the changes to math.

The example model is running for me and I'm getting different results for STAN_NUM_THREADS=8 and STAN_NUM_THREADS=1 so debug time wooo!

[bbbales2]

Oh wait. It generates data randomly. Lemme generate the data in R. I might be lying.

[bbbales2]

Alright with generated data Rhat = 1 so they're doing the same thing. I'm gonna go fix the indexing bug with start/end.

[rok-cesnovar]

@nhuurre Thanks again! I think I resolved all the comments apart from the one about functors where I have a question.

@bbbales2 I updated the test model with your changes. The updated error message is now:

Ill-typed arguments supplied to function 'reduce_sum'. Available arguments:
(int, int, T[,], ...) => real, int, T[,], ...
(int, int, T[,,], ...) => real, int, T[,,], ...
(int, int, T[,,,], ...) => real, int, T[,,,], ...
(int, int, T[,,,,], ...) => real, int, T[,,,,], ...
(int, int, T[,,,,,], ...) => real, int, T[,,,,,], ...
(int, int, T[,,,,,,], ...) => real, int, T[,,,,,,], ...
(int, int, T[,,,,,,,], ...) => real, int, T[,,,,,,,], ...
Where T is any one of int, real, vector, row_vector or matrix.
Instead supplied arguments of incompatible type: (int, real, real[], real, real) => real, real[], int, real

[rok-cesnovar]

I think I resolved all the open comments now, apart from the question on whether we want to be strict on the user defined function return type or not.

I will leave that one to @wds15 and @bbales2 to decide.

[wds15]

Just one thing: The error message says as a first line:

(int, int, T[,], ...) => real, int, T[,], ...

, but what about the signature

(int, int, T[], ...) => real, int, T[], ...

? Did I miss that? I mean, it's possible to instantiate this thing with an array of a scalar ( std::vector<var>), for example.

I am really looking forward to checkout develop from cmdstan and then have this shiny new reduce_sum available.

BTW... for the future we could think about handling sums of reduce_sum as

reduce_sum() + reduce_sum()

in an efficient way. That would be rat to do at some point. I got this thought when looking at the review comments here.

[rok-cesnovar]

The error message says as a first line

Ah, it was an off-by-one error in printing the error message (vector of 1 dimension means zero commas not 1 :) ). Fixed now. Thanks for pointing that out.

Once this passes I will start another build of the binaries.

[rok-cesnovar]

Place

STANC3_TEST_BIN_URL = https://jenkins.mc-stan.org/job/stanc3-test-binaries/57/artifact
in your make/local on cmdstan develop to test the latest binaries.

[wds15]

For me this looks all great now.

[wds15]

Just ran again one of my test models on this - speeds up nicely.

[rok-cesnovar]

The deadline to get this in for 2.23 is end of monday anywhere on earth. Does anyone have any more comments or requests? Would love to see this get in. But obviously not gonna pressure if anyone objects. Thanks!

[wds15]

Indeed, it would be awesome to have this land in 2.23... Tagging @nhuurre , @rybern and @seantalts ... hopefully someone got time to look at this.

Happy easter!

[nhuurre]

Looks good to me. I have a couple of suggestions but they're just optional style changes.

[rok-cesnovar]

Thanks @nhuurre !

Your comments were are very much on point. I addressed all of them.

Really excited to get this in and see it in action.
Thanks @wds15 @bbbales2 and @SteveBronder for the Math backend, the majority of work was done there. Thanks @nhuurre and @rybern for all the comments and advices! Very nice team effort!