stan-dev / stan

Stan development repository. The master branch contains the current release. The develop branch contains the latest stable development. See the Developer Process Wiki for details.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
2.56k stars 365 forks source link

mods to bin/print and sampling behavior #138

Closed bob-carpenter closed 11 years ago

bob-carpenter commented 11 years ago
mbrubake commented 11 years ago

Regarding thin vs keep, I'd prefer staying with thin. It's relatively standard terminology even if it is semantically somewhat misleading.

betanalpha commented 11 years ago

@bob-carpenter, what do you mean by "unnormalized doubles"?

bob-carpenter commented 11 years ago

Oops -- I mean "denormalized", not "unnormalized". See:

http://www.cplusplus.com/reference/limits/numeric_limits/

There's a minimum normalized value (with mantissa/significand between 0.5 and 1 or something like that --- the idea is that you don't want to lose precision with a bunch of leading zeroes). Then there's a minimum denormalized value, which is smaller.

Where we had problems with using std::cout and the output stream operator << is sending in double value between these two.

On 6/27/13 11:50 AM, Michael Betancourt wrote:

@bob-carpenter https://github.com/bob-carpenter, what do you mean by "unnormalized doubles"?

— Reply to this email directly or view it on GitHub https://github.com/stan-dev/stan/issues/138#issuecomment-20131726.

betanalpha commented 11 years ago

Wasn't this addressed when I switched from using a stringstream to Boost's lexical cast?

On Jun 27, 2013, at 5:18 PM, Bob Carpenter notifications@github.com wrote:

Oops -- I mean "denormalized", not "unnormalized". See:

http://www.cplusplus.com/reference/limits/numeric_limits/

There's a minimum normalized value (with mantissa/significand between 0.5 and 1 or something like that --- the idea is that you don't want to lose precision with a bunch of leading zeroes). Then there's a minimum denormalized value, which is smaller.

Where we had problems with using std::cout and the output stream operator << is sending in double value between these two.

  • Bob

On 6/27/13 11:50 AM, Michael Betancourt wrote:

@bob-carpenter https://github.com/bob-carpenter, what do you mean by "unnormalized doubles"?

— Reply to this email directly or view it on GitHub https://github.com/stan-dev/stan/issues/138#issuecomment-20131726.

— Reply to this email directly or view it on GitHub.

bob-carpenter commented 11 years ago

On 6/27/13 12:29 PM, Michael Betancourt wrote:

Wasn't this addressed when I switched from using a stringstream to Boost's lexical cast?

I thought so.

It may only be an issue with v1.3.

betanalpha commented 11 years ago

@bob-carpenter Checkout feature/print_tweaks - bin/print was tweaked and lots of doc was added. There is a formatting issue in pages 11 and 27 -- we need to figure out how to squeeze the entire bin/print output into the doc. I'm pretty sure \tiny ain't gonna cut it.

Oh, and sorry for the delay. I had most this worked out Thursday but then my power cut out and, well, let's just say that I still don't have power at my apartment.

bob-carpenter commented 11 years ago

Look good:

I can live with:

I'd still prefer

How about

andrewgelman commented 11 years ago
  1. How about MC_se (i.e., "Monte Carlo error") or is that just more confusion?
  2. I agree that 5 quantiles are too many. I feel uncomfortable with 2.5% and 97.5%. Maybe 10% and 90%? Or do we just go with 2.5%/97.5% for convention's sake?

changing "se_mean" to "se"? I know it's vague, but so is sd. And it takes up sooo much space.

getting rid of 25% and 50% quantiles? that'll help with printing. I'm happy changing 2.5% and 97.5% to something else, too, but having five quantiles seems like too much. I don't know how to make one page landscape-mode in LaTeX, but I think it's possible.

— Reply to this email directly or view it on GitHub.

betanalpha commented 11 years ago

Look good:

• changing timing significant digits

• changing n_eff/time to n_eff/s

Speaking of that, should we change Rhat to R_hat? Having n_eff underscored but Rhat as one word started looking weird to me today.

I can live with:

• not changing "thin" to "keep"

Just didn't hear any other strong opinions and, not having any of my own, didn't touch it.

I'd still prefer

• a consistent number of significant digits where leading zeroes don't count. For example, when I compare se to mean, I don't want to see this treedepth__ 6.3e-02 0.0 (mean first, se second) because the se isn't on the relevant scale of the variable. I'm not sure what everyone else thinks, but I think this can be whoever's call is writing the code if there aren't strong feelings. I can always just keep upping the precision until I see some significant digits.

I think it's doable, it just will take some nontrivial coding and I'm not sure how long it would take. I see if I can look into it.

How about

• changing "se_mean" to "se"? I know it's vague, but so is sd. And it takes up sooo much space.

MCSE? I've seen that enough that it shouldn't be too confusing.

• getting rid of 25% and 50% quantiles? that'll help with printing. I'm happy changing 2.5% and 97.5% to something else, too, but having five quantiles seems like too much.

5, 50, and 95 seem good to me but I also have no qualms with 2.5 and 97.5.

I don't know how to make one page landscape-mode in LaTeX, but I think it's possible.

\usepackage{rotating}

\pagebreak \begin{sidewaystable} \end{sidewaystable} \pagebreak

I didn't know if people would be happy with sideways printing of the output.

betanalpha commented 11 years ago

Okay, the big question here is definition.

Currently we use arithmetic precision, i.e. the number of digits to the right of the decimal place. This is consistent with the definition used in iostream.

Bob was expecting significant figures which, if we assume that the numbers themselves have infinite precision, is the number of leading non-zero values.

I'm happy to do either, but I'd prefer a quorum. Changing to the latter would be every-so-slightly nontrivial because of the conflict with the iostream definition, but it's feasible. Of course we'd have to change precision to sigfigs to be consistent, as well.

On Jul 1, 2013, at 11:49 PM, Bob Carpenter notifications@github.com wrote:

a consistent number of significant digits where leading zeroes don't count. For example, when I compare se to mean, I don't want to see this treedepth__ 6.3e-02 0.0 (mean first, se second) because the se isn't on the relevant scale of the variable. I'm not sure what everyone else thinks, but I think this can be whoever's call is writing the code if there aren't strong feelings. I can always just keep upping the precision until I see some significant digits.

bob-carpenter commented 11 years ago

I

On 7/1/13 7:10 PM, Michael Betancourt wrote:

Look good:

• changing timing significant digits

• changing n_eff/time to n_eff/s

Speaking of that, should we change Rhat to R_hat? Having n_eff underscored but Rhat as one word started looking weird to me today.

I prefer R_hat.

I can live with:

• not changing "thin" to "keep"

Just didn't hear any other strong opinions and, not having any of my own, didn't touch it.

Good strategy.

I'd still prefer

• a consistent number of significant digits where leading zeroes don't count. For example, when I compare se to mean, I don't want to see this treedepth__ 6.3e-02 0.0 (mean first, se second) because the se isn't on the relevant scale of the variable. I'm not sure what everyone else thinks, but I think this can be whoever's call is writing the code if there aren't strong feelings. I can always just keep upping the precision until I see some significant digits.

I think it's doable, it just will take some nontrivial coding and I'm not sure how long it would take. I see if I can look into it.

How about

• changing "se_mean" to "se"? I know it's vague, but so is sd. And it takes up sooo much space.

MCSE? I've seen that enough that it shouldn't be too confusing.

OK, mcse it is. Or is that mc_se? There's no good way to indicate nesting structure without brackets.

• getting rid of 25% and 50% quantiles? that'll help with printing. I'm happy changing 2.5% and 97.5% to something else, too, but having five quantiles seems like too much.

5, 50, and 95 seem good to me but I also have no qualms with 2.5 and 97.5.

5 and 95 should be more robust with the small number of samples we're recommending, so I'm OK with that.

I don't know how to make one page landscape-mode in LaTeX, but I think it's possible.

\usepackage{rotating}

\pagebreak \begin{sidewaystable} \end{sidewaystable} \pagebreak

I didn't know if people would be happy with sideways printing of the output.

It's either that or truncating/wrapping the output. So I'd say go for it.

bob-carpenter commented 11 years ago

On 7/2/13 11:21 AM, Michael Betancourt wrote:

Okay, the big question here is definition.

Currently we use arithmetic precision, i.e. the number of digits to the right of the decimal place. This is consistent with the definition used in iostream.

Aha! They even talk about the distinction here:

http://en.wikipedia.org/wiki/Significant_figures

But what I don't understand is why mean seems different than se and sd in:

                 mean   se   sd     2.5%

lp__ -7.3e+00 0.0 0.8 -9.3e+00 accept_stat 5.3e-01 0.1 0.4 1.1e-06 stepsize 2.1e+00 0.1 0.2 2.0e+00 treedepth__ 3.6e-02 0.0 0.2 0.0e+00 theta 2.5e-01 0.0 0.1 6.0e-02

Given the way se and sd print, I'd expect mean to look like this:

-7.3
 0.5
 2.1
 0.0
 0.3

Bob was expecting significant figures which, if we assume that the numbers themselves have infinite precision, is the number of leading non-zero values.

I do prefer significant figures --- it's what seems relevant because the rows differ in scale.

I'm happy to do either, but I'd prefer a quorum. Changing to the latter would be every-so-slightly nontrivial because of the conflict with the iostream definition, but it's feasible. Of course we'd have to change precision to sigfigs to be consistent, as well.

It's certainly not urgent, even if we decide go to with significance instead of precision.

betanalpha commented 11 years ago

Yeah, it looks like there's a subtle problem with columns where each value is less than 1 (essentially the leading zeros aren't taken into account unless there's a value greater than 1). If we decide to stick with precision I'll fix it.

On Jul 2, 2013, at 4:43 PM, Bob Carpenter notifications@github.com wrote:

But what I don't understand is why mean seems different than se and sd in:

mean se sd 2.5% lp__ -7.3e+00 0.0 0.8 -9.3e+00 accept_stat 5.3e-01 0.1 0.4 1.1e-06 stepsize 2.1e+00 0.1 0.2 2.0e+00 treedepth__ 3.6e-02 0.0 0.2 0.0e+00 theta 2.5e-01 0.0 0.1 6.0e-02

Given the way se and sd print, I'd expect mean to look like this:

-7.3 0.5 2.1 0.0 0.3

betanalpha commented 11 years ago

@bob-carpenter

Checkout the updated branch. A significant rewrite of bin/print, but I think you will be pleased.

Note that sometimes the sig figs don't make sense -- that's due to iostream rounding. So, for example, you might set sig_figs=2 but see 1.00. That's because the value is really 0.999999999999 and iostream rounds 0.99(9) to 1.00 when you limit the precision.

bob-carpenter commented 11 years ago

On 7/2/13 5:22 PM, Michael Betancourt wrote:

@bob-carpenter https://github.com/bob-carpenter

Checkout the updated branch. A significant rewrite of bin/print, but I think you will be pleased.

Note that sometimes the sig figs don't make sense -- that's due to iostream rounding. So, for example, you might set sig_figs=2 but see 1.00. That's because the value is really 0.999999999999 and iostream rounds 0.99(9) to 1.00 when you limit the precision.

I think I can live with that. I guess it should be "1.0" with sig_figs=2.

bob-carpenter commented 11 years ago

Awesome. I love it.


Inference for Stan model: bernoulli_model 2 chains: each with iter=(1000,1000); warmup=(0,0); thin=(1,1); 2000 iterations saved.

Warmup took (0.088, 0.097) seconds, 0.19 seconds total Sampling took (0.082, 0.097) seconds, 0.18 seconds total

              Mean     MCSE  StdDev        5%   50%   95%  N_Eff  N_Eff/s    R_hat

lp__ -7.2 3.0e-02 0.72 -8.7e+00 -7.0 -6.8 594 3321 1.0e+00 accept_stat 0.51 1.8e-01 0.40 1.1e-05 0.49 1.0 5.3 29 1.1e+00 stepsize 2.0 4.0e-01 0.40 1.6e+00 2.4 2.4 1.0 5.6 2.2e+14 treedepth__ 0.063 6.5e-03 0.24 0.0e+00 0.00 1.0 1389 7765 1.0e+00

theta 0.25 3.5e-03 0.11 8.2e-02 0.23 0.45 1059 5922 1.0e+00

My first thought was that the periods don't line up for mean, but then I realized it's better that way. I like having the same number of significant digits for each number. It means I can trust the 0.00 in the treepdeth 50% column, for instance.

The 3321 shows the issue with numbers greater than 1. You get 4 digits of precision here, which we really don't want. But if you write 3300 or 3300.0, then people are going to think you mean that exactly. Even though I'd prefer 3300 for my own consumption, knowing how it was calculated, I think it's too confusing to spring on the public and not worth making a fuss about.

Do we want decimals in all the numbers? The ones without make it look like they're exact integer values.

Anyway, I'm totally OK with the way it is!

On 7/2/13 5:22 PM, Michael Betancourt wrote:

@bob-carpenter https://github.com/bob-carpenter

Checkout the updated branch. A significant rewrite of bin/print, but I think you will be pleased.

Note that sometimes the sig figs don't make sense -- that's due to iostream rounding. So, for example, you might set sig_figs=2 but see 1.00. That's because the value is really 0.999999999999 and iostream rounds 0.99(9) to 1.00 when you limit the precision.

— Reply to this email directly or view it on GitHub https://github.com/stan-dev/stan/issues/138#issuecomment-20377307.

bob-carpenter commented 11 years ago

These features being addressed in branch feature/print_tweaks

betanalpha commented 11 years ago

Just pushed a fix that ensures failure if method (or any of the other base arguments) are called more than once, and fixed the seg fault when running optimize. Nesterov seemed to freeze on a simple gaussian, but somewhere within the NesterovGradient code.

On Jul 10, 2013, at 8:30 PM, Daniel Lee bearlee@alum.mit.edu wrote:

Bob, hopefully you figured it out, but if not, it's just ./model help

For now, I'm assuming the compiled model is located at: ./model

Here are some changes that I want:

1) No arguments stops execution, prints help. Current behavior of "./model": runs the model, assumes no data, samples using NUTS2

Proposed behavior of "./model" is equivalent to "./model help". Does not sample.

2) Get rid of "method=<>". Just make it "<>". The set of things that could fit there (currently) would be: {sample, optimize, test} Current command to sample: "./model method=sample" (uses defaults) Current command to optimize: "./model method=optimize" (uses defaults)

Proposed command to sample: "./model sample" Proposed command to optimize: "./model optimize"

3) Only allow one of {sample, optimize, test}. Setting two or more should result in no execution, error message. Current command: "./model method=optimize method=sample" -- runs the sampler

Proposed command: "./model optimize sample" -- stops with message like "Only one command {optimize, sample, test} can be called"

4) Restructure help message. Current command: "./model help" -- outputs: Usage: model ... ... ...

Valid arguments to Stan:

id=<> Chain id data=<> Input data file init=<> Initialization method test_gradient Check model gradient against finite differences random Random number configuration output File output options method=<> Analysis method

See 'model' [ help | help-all ] for argument specific information or 'model' help-all for all argument information Failed to parse arguments, terminating Stan

Proposed command: "./model help".

Usage: model ... ... ...

Begin inference by selecting one of the following: sample Bayesian inference using Markov Chain Monte Carlo optimize Point estimation test Check model help Prints help

Additional arguments: data=<> Input data file init=<> Initialization method / file output File output options id=<> Chain id random Random number configuration

See 'model' [ help | help-all ] for argument specific information or 'model' help-all for all argument information

  • Ideally we would replace 'model' with the actually command line call, unless you think that's too messy.

5) Break out "test_gradient" option to "test"/"test gradient" Current command to test gradients: "./model test_gradient" Proposed command to test gradients: "./model test" or "./model test gradient"

  • "gradient" would be the sub-command to test
  • I'm thinking about future uses where we may build out other types of tests.

6) Fix help printouts. (I killed a lot of the verbose output, but I didn't do it right.) I think the verbosity should be contextually. From the top level, it should just show the simple help, but when called after some options, it should be verbose.

Current command: "./model method=sample test-all" output: "sample" (this is my fault -- it used to print out information about Sampling Algorithm and get into HMC and NUTS. There wasn't much, but there was a description).

Proposed command: "./model help" prints as above

  "./model sample help" prints the options that can be passed to sample directly.
  "./model sample help-all" prints help novels for each of the algorithms.

7) For the "./model method=sample algorithm=hmc engine=nuts" replace that with "./model sample nuts" I guess for sampling, I imagine there being a handful of options: {hmc, nuts, metropolis} and we'll add a few more like rmhmc, rmnuts. I think the settings should be flat after that. I'm not sure it helps to have multiple levels of options configurable in this tool. We would always have complete flexibility in C++. The same goes for optimize. I think the "sample" and "optimize" commands are just defaults. If someone doesn't play with the parameters, it should always work with just "sample."

Thoughts?

PS. I just made it crash: make models/basic_distributions/normal ./models/basic_distributions/normal method=optimize

On Tue, Jul 2, 2013 at 12:26 PM, Bob Carpenter carp@alias-i.com wrote: I checked out the feature/arg_refactor branch and built a model, but --help doesn't work.

How do I find the list of options available?

I know you e-mailed something at some point, but I can't find it among the pile of e-mails I have.

  • Bob