MCMC/HMC refactor

[Bob Carpenter]

Michael --- this all looks GREAT. Way better than the
way the code's currently organized. All the suggestions
below are very minor.

Michael is using a very different coding style than the
rest of Stan -- how much do we care about this kind of
thing? I'd volunteer to restyle it, but I don't want to
just do that unilaterally.

Here are some specific comments organized by file.

mcmc/mcmc_base.hpp
------------------
Break this file into two files, mcmc_sampler.hpp and sample.hpp
to follow the class defs.

What's the acceptance statistic and why is it part of a sample?
Isn't acceptance a property of a proposal sample relative to
a previous sample?

I'm ok with using "cont"/"disc" instead of "real"/"int",
but it should be consistent. (Yes, I realize it's all discrete
floating point reps underneath for reals, but Andrew 1 didn't
like "double", which is how I originally had it.)

mcmc/hmc_base.hpp
-----------------
Break out ps_point into its own file named ps_point.hpp
We're moving to finer grained and easier to navigate files
in the new math refactor, and I think it's a good idea.

Would it be useful to generalize away from teh normal
distribution for generating momenta? (I realize a fatter
tailed proposal doesn't help much, but it seems an easy
thing to generalize if it'll ever be needed for eval, etc.)

Looks like you got the RNG refs in there already.

mcmc/hamiltonian.hpp
--------------------
Break out into base_hamiltonian.hpp, unit_e_point.hpp,
and unit_metric.hpp.

What's the purpose of defining templated member functions
outside of the class? (And I didn't mean that question
rhetorically --- I'm genuinely curious.)

I can see the pain here with the std::vector to Eigen
conversion. Let's clean all that up next (which should be
a whole heckuva lot easier with this refactor).

What's with the empty comments at the bottom, like "DEFINE
HAMILTONIANS HERE"?

By the time this gets pushed, we should get rid of all commented
out code.


mcmc/chains.hpp
---------------
This doesn't look like it's changed much, but we should get rid of
that block of comments at the bottom. If they're to-do items,
they should go on the to-do list.

mcmc/adapter.hpp
----------------
stepsize_adapter class to file stepsize_adapter.hpp. (I'll let
you work out the rest of these moves).

This looks super clean compared to what we had before.

Do you really need setters or could you get away with a rich
constructor? I'd greatly prefer the latter because it's easier
to reason about the consistency of state with immutable objects.
Then you can get rid of the setters and make the member variables
private. If you do allow setters, there needs to be a check on
consistent values (such as some of these values being required
to be finite and positive).

mcmc/integrator.hpp
-------------------
Break into files named by class.

You can get rid of the protected: block in leapfrog -- there's
nothing in it. Ditto the private block of expl_leapfrog.


mcmc/hamiltonian.hpp
--------------------
Break into files named by class.

I think we should avoid mixing the underscore style and camel case,
so I'd rename "sampleP" to "sample_p" for consistency. (Overall,
the naming looks great and I take naming very seriously.)

Also some seemingly unanchored comments at the bottom.


mcmc/nuts.hpp
-------------
Break into files named by class.

These implementations look awesome.



In conclusion, I love the refactor. With this and what Daniel's
doing on the base math classes, this is all going to be a whole
lot easier to understand.

- Bob

[Ben Goodrich]

On Thursday, March 28, 2013 3:28:32 PM UTC-4, Bob Carpenter wrote:

Michael --- this all looks GREAT.  Way better than the
way the code's currently organized.  All the suggestions
below are very minor.

I just wanted to ask a question: Is it possible to do the SoftMax thing based on the LDL' decomposition of the Hessian rather than the eigendecomposition? In either case, the determinant is just the product of the diagonal elements of the middle matrix, but I think the LDL' decomposition is faster than the (rather basic) self-adjoint eigendecomposition in Eigen. The only thing is that the former also does pivoting, so it is really a PLDL'P' decomposition where P is a permutation matrix that needs to be kept track of.

Ben

[Ben Goodrich]

On Thursday, March 28, 2013 3:51:08 PM UTC-4, Ben Goodrich wrote:

On Thursday, March 28, 2013 3:28:32 PM UTC-4, Bob Carpenter wrote:

Michael --- this all looks GREAT.  Way better than the
way the code's currently organized.  All the suggestions
below are very minor.

I just wanted to ask a question: Is it possible to do the SoftMax thing

I meant SoftAbs.

Ben

[Michael Betancourt]

Nope. The problem is that you're really trying to do matrix exponentials, which LDLT doesn't facilitate, and then do gradients of the decomposition, and I'm not familiar with any work on the derivatives of L or D.

> --
> You received this message because you are subscribed to the Google Groups "stan development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Michael Betancourt]

> Michael --- this all looks GREAT. Way better than the
> way the code's currently organized. All the suggestions
> below are very minor.

Most of them are errors, like the sampleP name, that I thought I had already fixed or simply missed. I'll address any ambiguities inline.

The breaking up the files was also on my mind. Do we want to keep to a strict one class/one file schema or can I put VERY related classes together. The only example I can think of right now would be unit_metric_hmc and adapt_unit_metric_hmc, since the latter is only a small extension of the first.

> Michael is using a very different coding style than the
> rest of Stan -- how much do we care about this kind of
> thing? I'd volunteer to restyle it, but I don't want to
> just do that unilaterally.

Which elements of the style? I tried to stay consistent with the style in the rest of the files, save for some sparse white space here and there which I had to throw in to quickly identify different blocks of code, and I'm happy to fix things up.

I did shed a tear every time I had to delete a linebreak between a if/for and the opening brace.

> What's the acceptance statistic and why is it part of a sample?
> Isn't acceptance a property of a proposal sample relative to
> a previous sample?

If we think of each "sample" as actually a transition in a Markov chain then the acceptance statistic (acceptance probability in the Metropolis case, tree-sampling probability in the NUTS case) is an important element of the sample. On a more practical side, it's important to pass along for adaptation and helpful for diagnostics (for example right now we only implicitly use the mean of this statistic, assuming that the stepsize adaptation has converged, but the variance can also be useful in diagnosing wacky geometries).

> I'm ok with using "cont"/"disc" instead of "real"/"int",
> but it should be consistent. (Yes, I realize it's all discrete
> floating point reps underneath for reals, but Andrew 1 didn't
> like "double", which is how I originally had it.)

cont/disc seemed much more natural without adding too many characters. Was there anywhere in the mcmc code that I failed to keep things consistent, or are you referring to the model code?

> Would it be useful to generalize away from teh normal
> distribution for generating momenta? (I realize a fatter
> tailed proposal doesn't help much, but it seems an easy
> thing to generalize if it'll ever be needed for eval, etc.)

This was one place where I compromised because I didn't fully understand Boost's RNG and I couldn't imagine anyone trying non-quadratic kinetic energies anytime soon. I'd like to keep the Hamiltonians stateless, so would this be acceptable?

- Pass the rand_int to sample_p by reference.
- Create the random distribution on the fly to generate variates inline.

The only issue is how expensive the random distribution construction is.

> What's the purpose of defining templated member functions
> outside of the class? (And I didn't mean that question
> rhetorically --- I'm genuinely curious.)

Just habit from the h/cpp days. I can put them back in if that's at all preferable.

> What's with the empty comments at the bottom, like "DEFINE
> HAMILTONIANS HERE"?
>
> By the time this gets pushed, we should get rid of all commented
> out code.

Yup, just comments to myself during the refactor. Will not be present when attempting to merge.

> mcmc/chains.hpp
> ---------------
> This doesn't look like it's changed much, but we should get rid of
> that block of comments at the bottom. If they're to-do items,
> they should go on the to-do list.

Didn't actually touch this guy yet, but I'll clean it up.

> Do you really need setters or could you get away with a rich
> constructor? I'd greatly prefer the latter because it's easier
> to reason about the consistency of state with immutable objects.
> Then you can get rid of the setters and make the member variables
> private. If you do allow setters, there needs to be a check on
> consistent values (such as some of these values being required
> to be finite and positive).

Personally I prefer clear constructors and settings because I prefer using the objects in a create/tweak schema. But here I'd actually fight for explicit setters, as it would allow for multi-stage adaptation without creating new instances.

-Mike

[Bob Carpenter]

On 3/28/13 4:59 PM, Michael Betancourt wrote:

> The breaking up the files was also on my mind. Do we want to keep to a strict one class/one file schema or can I put VERY related classes together. The only example I can think of right now would be unit_metric_hmc and adapt_unit_metric_hmc, since the latter is only a small extension of the first.

Now that we've started doing it elsewhere, I like one
class per file. (Maybe it's my java background.) It makes
the arrangement of dependencies and testing easier and it's
easier to browse.

>> Michael is using a very different coding style than the
>> rest of Stan -- how much do we care about this kind of
>> thing? I'd volunteer to restyle it, but I don't want to
>> just do that unilaterally.
>
> Which elements of the style? I tried to stay consistent with the style in the rest of the files,

You've mostly got rid of the open-paren-only lines (thanks!), but there are
still some left.

There was also a big ///// Fortran-style comment

> save for some sparse white space here and there which I had to throw in to quickly identify different blocks of code, and I'm happy to fix things up.

Yes, lots of vertical whitespace, and then
some one-liners like:

ps_point(int n, int m): q(n), r(m), p(Eigen::VectorXd::Zero(n)) {};

I think you're also the only one who starts argument lists on the
next line even when there's plenty of horizontal space left:

hmc_base<M, P, H, I, BaseRNG>::hmc_base(
M &m,
BaseRNG& rng)

Overall, our style hasn't been very consistent. So there's not much
of a target.

I sort of like the way you lay out constructors with the
semicolon on the previous line.

> I did shed a tear every time I had to delete a linebreak between a if/for and the opening brace.

It saved me just as many, so thanks.

>> What's the acceptance statistic and why is it part of a sample?
>> Isn't acceptance a property of a proposal sample relative to
>> a previous sample?
>
> If we think of each "sample" as actually a transition in a Markov chain then the acceptance statistic (acceptance probability in the Metropolis case, tree-sampling probability in the NUTS case) is an important element of the sample. On a more practical side, it's important to pass along for adaptation and helpful for diagnostics (for example right now we only implicitly use the mean of this statistic, assuming that the stepsize adaptation has converged, but the variance can also be useful in diagnosing wacky geometries).

That's what I suspected.

Conceptually, I don't think an acceptance probability is
part of a sample per se. As you point out, its interpretation
and contents is going to be very sampler specific. If we
had an exact sampler then there wouldn't even be an acceptance
statistic.

In Metropolis, accept probability is a relation between two
parameter configurations.

There's also the issue of "sample" vs. potential sample.
And we wound up standardizing on "draw" in the external doc
because of the exact vs. MCMC sampling and the notion of
effective sample size.

If everything we have now has some use for it and it's easy
enough to generalize, we can always refactor later. I'd
actually rather wait until we have a need to refactor than
to do it on spec in the hopes of anticipating something we
haven't implemented yet.

>> I'm ok with using "cont"/"disc" instead of "real"/"int",
>> but it should be consistent. (Yes, I realize it's all discrete
>> floating point reps underneath for reals, but Andrew 1 didn't
>> like "double", which is how I originally had it.)
>
> cont/disc seemed much more natural without adding too many characters. Was there anywhere in the mcmc code that I failed to keep things consistent, or are you referring to the model code?

Just the model code. No, the MCMC code's consistent.
It's not a huge problem if it doesn't match the other
code. I had inconsistencies in the code because we
changed the original "double" to "real" at some point,
so a lot of the internals still say "double". I always
find it a bit confusing. But no big deal.

>> Would it be useful to generalize away from teh normal
>> distribution for generating momenta? (I realize a fatter
>> tailed proposal doesn't help much, but it seems an easy
>> thing to generalize if it'll ever be needed for eval, etc.)
>
> This was one place where I compromised because I didn't fully understand Boost's RNG and I couldn't imagine anyone trying non-quadratic kinetic energies anytime soon. I'd like to keep the Hamiltonians stateless, so would this be acceptable?

Sure. See above -- we can always generalize later if
we need it. Boost's RNG isn't object oriented, it's
concept oriented, which is why you have to always template
out the RNG rather than provide some kind of base class
reference argument type.

> - Pass the rand_int to sample_p by reference.
> - Create the random distribution on the fly to generate variates inline.
>
> The only issue is how expensive the random distribution construction is.

You pay a constructor overhead, but it's cheap because there's
not much data -- just the parameters of the distribution. They
do check bounds on the parameters, though (like that variance is
positive and the mean is finite).

>> What's the purpose of defining templated member functions
>> outside of the class? (And I didn't mean that question
>> rhetorically --- I'm genuinely curious.)
>
> Just habit from the h/cpp days. I can put them back in if that's at all preferable.

We might as well do them all one way or the other. Some were
defined in the classes and some outside. I like them inside,
but I think that's another Java holdover for me. I can see
the elegance of just having the header part defined. It's just
with templates we can't really separate the declarations and
definitions.

I have been thinking about compile efficiency. For that,
with templated code, we need explicit instantiations to compile
and link against and header files to include. But let's worry
about that after this refactor because it's orthogonal functionally.

>> mcmc/chains.hpp
>> ---------------
>> This doesn't look like it's changed much, but we should get rid of
>> that block of comments at the bottom. If they're to-do items,
>> they should go on the to-do list.
>
> Didn't actually touch this guy yet, but I'll clean it up.
>
>> Do you really need setters or could you get away with a rich
>> constructor? I'd greatly prefer the latter because it's easier
>> to reason about the consistency of state with immutable objects.
>> Then you can get rid of the setters and make the member variables
>> private. If you do allow setters, there needs to be a check on
>> consistent values (such as some of these values being required
>> to be finite and positive).
>
> Personally I prefer clear constructors and settings because I prefer using the objects in a create/tweak schema.
> But here I'd actually fight for explicit setters, as it would allow for multi-stage adaptation
> without creating new instances.

New instances are cheap. Managing state's a headache.
Whatever you think's best here.

If you keep the mutable desgin, please add checks for consistency
on any public setters. You can provide extra methods for friends
if you need to. We really need to do this in other places in the code.

- Bob

[Michael Betancourt]

> You've mostly got rid of the open-paren-only lines (thanks!), but there are
> still some left.

> some one-liners like:
>
> ps_point(int n, int m): q(n), r(m), p(Eigen::VectorXd::Zero(n)) {};
>
> I think you're also the only one who starts argument lists on the
> next line even when there's plenty of horizontal space left:
>
> hmc_base<M, P, H, I, BaseRNG>::hmc_base(
> M &m,
> BaseRNG& rng)
>
> Overall, our style hasn't been very consistent. So there's not much
> of a target.

More inadvertent mistakes. I'll try to fix them up.

> Conceptually, I don't think an acceptance probability is
> part of a sample per se. As you point out, its interpretation
> and contents is going to be very sampler specific. If we
> had an exact sampler then there wouldn't even be an acceptance
> statistic.
>
> In Metropolis, accept probability is a relation between two
> parameter configurations.
>
> There's also the issue of "sample" vs. potential sample.
> And we wound up standardizing on "draw" in the external doc
> because of the exact vs. MCMC sampling and the notion of
> effective sample size.

Yeah, sample doesn't really make sense -- we're talking about elements in a Markov chain. But in any case, we can always talk about an "acceptance statistic", even in the limit of independent samples when it asymptotes to 1.

> Sure. See above -- we can always generalize later if
> we need it. Boost's RNG isn't object oriented, it's
> concept oriented, which is why you have to always template
> out the RNG rather than provide some kind of base class
> reference argument type.

That's what I liked about RandomLib -- you passed the RNG by reference to each variate generator. Anyways, should be an easy enough fix.

> New instances are cheap. Managing state's a headache.
> Whatever you think's best here.
>
> If you keep the mutable desgin, please add checks for consistency
> on any public setters. You can provide extra methods for friends
> if you need to. We really need to do this in other places in the code.

Will do.

[Bob Carpenter]

On 3/29/13 12:26 AM, Michael Betancourt wrote:

>> Overall, our style hasn't been very consistent. So there's not much
>> of a target.
>
> More inadvertent mistakes. I'll try to fix them up.

I really didn't want to make a big deal out of this. It's
a class bike-shed issue!

>> Conceptually, I don't think an acceptance probability is
>> part of a sample per se. As you point out, its interpretation
>> and contents is going to be very sampler specific. If we
>> had an exact sampler then there wouldn't even be an acceptance
>> statistic.
>>
>> In Metropolis, accept probability is a relation between two
>> parameter configurations.
>>
>> There's also the issue of "sample" vs. potential sample.
>> And we wound up standardizing on "draw" in the external doc
>> because of the exact vs. MCMC sampling and the notion of
>> effective sample size.
>
> Yeah, sample doesn't really make sense -- we're talking about elements in a Markov chain. But in any case, we can always talk about an "acceptance statistic", even in the limit of independent samples when it asymptotes to 1.

This can lead to problematic designs. I did this in a few
places in the software for our company and it never failed
to trip users up (example: a probability method on text
spans that were sometimes extracted with a regex --- what
value do you give it so that users aren't confused?).
In this case, they'll run a regular sampler, get an object
with an "acceptance statistic" (your scare quotes, not mine!) and
wonder what it means.

The design obstacle is that for efficient C++ code, you don't want
any virtual methods (Java's all virtual all the time). That means
you have to write the code polymorphically (with templates) rather
than object oriented. The API problem of polymorphism is that
there's no within-the-language way to declare (as opposed to define
implicitly in whatever code uses the templates) what concepts are
required on the implementation of the class instantiating the
template.

In NUTS, we really have multiple acceptance
statistics --- there's the slicing then the sampling along
the path. That also argues for templating out the acceptance
stat. But then it becomes hard to use it when all different
classes implement it differently.

Anyway, you (Michael) are likely to be the one coding against
the API you're designing for the diagnostics.

>> Sure. See above -- we can always generalize later if
>> we need it. Boost's RNG isn't object oriented, it's
>> concept oriented, which is why you have to always template
>> out the RNG rather than provide some kind of base class
>> reference argument type.
>
> That's what I liked about RandomLib -- you passed the RNG by reference to each variate generator. Anyways, should be an easy enough fix.

It's not optimally efficient, but it's very clean (stateless).
Peter wrote most of the code --- I just did the normal
as an example.

>> New instances are cheap. Managing state's a headache.
>> Whatever you think's best here.
>>
>> If you keep the mutable desgin, please add checks for consistency
>> on any public setters. You can provide extra methods for friends
>> if you need to. We really need to do this in other places in the code.
>
> Will do.

Thanks.

- Bob

[Andrew Hunter]

On Fri, Mar 29, 2013 at 8:26 AM, Bob Carpenter <ca...@alias-i.com> wrote:
> The design obstacle is that for efficient C++ code, you don't want
> any virtual methods (Java's all virtual all the time).

Honestly, I think it's easy to get carried away here. On a decent and
a modern machine, virtual function calls overhead can be well under
three nanoseconds. Seriously (at least, if they're (dynamically)
predictable, which they are in this case, the branch predictor can do
perfectly well and the overhead is very, very hard to notice.)
Templating everything you do is not free; it makes code much harder to
read and follow, and is the primary cause of terrible compilation
times.

On my (fairly beefy) laptop, `make test-all` took something north of
48 hours. I really do recommend as part of the refactoring efforts
that we eliminate templates where they're not really necessary; if
you're avoiding a virtual function call, it better be something tiny,
inlined, and called more than once every, say, five hundred
instructions.

[Bob Carpenter]

On 3/29/13 1:44 PM, Andrew Hunter wrote:

> I really do recommend as part of the refactoring efforts
> that we eliminate templates where they're not really necessary; if
> you're avoiding a virtual function call, it better be something tiny,
> inlined, and called more than once every, say, five hundred
> instructions.

Sounds like a good idea.

We'd especially like to get model compilation time down because it's
often longer than the time to run a model. I'm less worried about
the libs and parser that get compiled once per version for users,
though it'd certainly be nice to speed that up for us developers.

More below.

> On Fri, Mar 29, 2013 at 8:26 AM, Bob Carpenter <ca...@alias-i.com> wrote:
>> The design obstacle is that for efficient C++ code, you don't want
>> any virtual methods (Java's all virtual all the time).
>
> Honestly, I think it's easy to get carried away here.

I agree. Which is why I was suggesting we didn't do it
for this case. In this case, I don't even think we need
a virtual design yet.

Most of our templating is in order to support one of three
things:

a. specifying arguments that are written polymorphically
rather than with a suitable base class, such as the boost
RNGs,

b. dealing with the polymorphism presented by our different
scalars: double, stan::agrad::var, and soon, stan::agrad::fvar
for forward-mode auto-diff, and also by the vectorization of
functions where we use views, or

c. the polymorphism implicit in the Eigen matrix libs
and Boost math libs.

For b and c, we could get away with a bunch of overloaded
non-template functions on our sides. In some cases, like for
the probability functions, it'd be prohibitive (there are
literally hundreds of possible instantiations that could get
called in a model).

As for (c), the libraries we use are heavily templated, but
we only use a few instantiations underneath.

The design cost would be duplicated functions for different
types in some cases.

> On a decent and
> a modern machine, virtual function calls overhead can be well under
> three nanoseconds. Seriously (at least, if they're (dynamically)
> predictable, which they are in this case, the branch predictor can do
> perfectly well and the overhead is very, very hard to notice.)

And if the branch isn't predictable statically, we can't
template it away anyway!

> Templating everything you do is not free; it makes code much harder to
> read and follow, and is the primary cause of terrible compilation
> times.
>
> On my (fairly beefy) laptop, `make test-all` took something north of
> 48 hours.

48 hours? Yikes. Our observation is that if you parallelize (-j4 on
my recent generation Macbook Pro with four cores), the tests take
about the following amounts of time:

basic unit tests: 5 minutes

distribution unit tests: 20 minutes

model functional tests: 60 minutes

That's running everything -O3. The model tests can take longer
run at lower optimization.

We have a buildbot set up to do continuous tests, but the model tests
and distribution tests are now set up to run daily rather than versus
every change in the code base.

- Bob

[Andrew Hunter]

On Fri, Mar 29, 2013 at 11:37 AM, Bob Carpenter <ca...@alias-i.com> wrote:
> 48 hours? Yikes. Our observation is that if you parallelize (-j4 on
> my recent generation Macbook Pro with four cores), the tests take
> about the following amounts of time:
>
> basic unit tests: 5 minutes
>
> distribution unit tests: 20 minutes
>
> model functional tests: 60 minutes
>

Uh, really? OK, perhaps I'm doing something wrong. But on my 2013
macbook pro (4 cores/8 threads 2.3 GHz i7, 16 GB) ram, even with make
-j, a full test-all took 17 hours. Perhaps I'm doing something wrong,
but are you sure that's what you see?

My reported "g++": i686-apple-darwin11-llvm-g++-4.2 (Xcode's
version)...perhaps you're using a different compiler?

[Ben Goodrich]

17 hours seems excessive, but I would guess that even with 16GB of RAM in might have to swap with -j. In any event, using clang++ with -j8 should be much faster.

Ben

 

[Daniel Lee]

As Ben has mentioned, using clang++ is much faster. And g++ 4.6 is going to be faster than 4.2. I'm actually surprised it all compiled with g++ 4.2.

I think I told Bob the wrong numbers earlier. Especially after the refactoring, but it definitely shouldn't take 48 hours.

With regards to test targets, we have:
* test-unit: the unit tests (excluding distribution tests, unless they've already been generated) ~1100 tests

* test-distribution: the distribution tests. We've got ~180k tests here.

* test-models: tests the models we have built against known values. We use these to make sure our end-to-end process still works as well as verifying that our sampling is still returning reasonable values. Our unit tests do test whether or not code is being generated properly, but this really focuses on the results from sampling.

* test-all: just calls the above targets.

We usually stick to running the unit-tests while developing. If we're playing with distributions, we'll also run the distribution tests. The model tests should be run if you're playing with samplers, derivatives of distributions, etc. But for the most part, we rely on our continuous integration server to tell us whether the models are broken. You can see the results here:
http://d1m1s1b1.stat.columbia.edu:8080/

Daniel

--

[Bob Carpenter]

On 3/29/13 9:37 PM, Andrew Hunter wrote:
> On Fri, Mar 29, 2013 at 11:37 AM, Bob Carpenter <ca...@alias-i.com> wrote:
>> 48 hours? Yikes. Our observation is that if you parallelize (-j4 on
>> my recent generation Macbook Pro with four cores), the tests take
>> about the following amounts of time:
>>
>> basic unit tests: 5 minutes
>>
>> distribution unit tests: 20 minutes
>>
>> model functional tests: 60 minutes
>>
>
> Uh, really? OK, perhaps I'm doing something wrong. But on my 2013
> macbook pro (4 cores/8 threads 2.3 GHz i7, 16 GB) ram, even with make
> -j, a full test-all took 17 hours.

We all (well, Daniel, Andrew, Jiqiang and I, at least)
have the exact same notebook. I'll fire up some tests
with different compilers and see how long it takes.

We definitely want to get the compile times down.

I think the unit tests are compile-time bound whereas
the model functional tests are execution-time bound (the
latter run multiple Markov chains and test the results
against known answers).

I wrote 5 minutes for basic unit tests with clang++ -O0,
which compiles a bit faster than g++, especially g++ -O3.
I've never run test-all; the numbers above were how long
Daniel said everything took (assuming I heard right, that is).

The buildbot is a Windows machine and I'm not exactly sure
how Daniel has it configured.

Ben's tried all sorts of compilers under Linux, but I don't
know what the overall speed test times were.

> Perhaps I'm doing something wrong,
> but are you sure that's what you see?
>
> My reported "g++": i686-apple-darwin11-llvm-g++-4.2 (Xcode's
> version)...perhaps you're using a different compiler?

I'll fire off some tests and see how long they take on
my machine with g++ and clang++ from Xcode with different
optimizations. The latest Xcode clang++ compilers seem to
execute our code about as fast as the recent g++ from Xcode.
We've found the Macports or other more recent g++ versions to
be faster, but we've pretty much stuck with Xcode versions
on the Mac.

Any suggestions would be appreciated.

- Bob

[Andrew Hunter]

FYI, Clang made it a lot faster for me --the full test suite was <3
hours with parallelization. This is hardly surprising, it's known to
be a much faster compiler. I also tried gcc-4.7 from ports, which was
faster than XCode, though not as nice as clang.

I'm going to look into removing some of the more egregious
templatization where I can. I bet it'll help.

[Bob Carpenter]

On 3/30/13 12:14 PM, Andrew Hunter wrote:
> FYI, Clang made it a lot faster for me --the full test suite was <3
> hours with parallelization. This is hardly surprising, it's known to
> be a much faster compiler. I also tried gcc-4.7 from ports, which was
> faster than XCode, though not as nice as clang.
>
> I'm going to look into removing some of the more egregious
> templatization where I can. I bet it'll help.

Great. Let us know what you're untemplatizing and how.
(Not to check your work, but so we can see how to do it
better next time.)

The only issue I know of ahead of time is that we need to make
sure not to lose instantiations for alternative scalar types
where we don't have appropriate specializations.

Another issue where we've been toying with simplifying dramatically
is in error handling policy arguments. They're everywhere in
our distributions, and either need to get pushed into our
math libs so that they also have configurable behavior, or simply
removed. The reason we haven't done this yet is that we know
some styles, like Google's, frown on exceptions, yet we're using
them to signal error conditions and locations in a way that would
be very very inconvenient to do any other way.

Although the grammars are only compiled once, I think
the top-level classes could be hardcoded for the iterator
type that's templated everywhere. Otheter than that, I'm curious
what can be untemplated.

- Bob