Re: design doc for arg refactor? or --help option on model?

[Daniel Lee]

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 <arg1> <subarg1_1> ... <subarg1_m> ... <arg_n> <subarg_n_1> ... <subarg_n_m>

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' <arg1> [ 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 <arg1> <subarg1_1> ... <subarg1_m> ... <arg_n> <subarg_n_1> ... <subarg_n_m>

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' <arg1> [ 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 <ca...@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

[Michael Betancourt]

Most of the changes you want can be implemented by adding a buffer layer in the hierarchy, although I'm not sure I agree with such a strategy. Comments inline.

> 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.

This is new behavior and a break from 1.3. Personally, I'd prefer to have a default method.

> 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"

I can see some of the motivation to removing method, but it does create a singular argument that has to implemented differently from all of the rest.

> 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"

Not the desired behavior. I'm looking into it.

> 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.

My opinion is to go with a categorical argument, test=test_name

We can make gradient the only test here and leave the rest to be added later.

> 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.

Well, it used to do that. I'm not sure what changes you've made.

> 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 strongly disagree. Not only do I dislike flattening the sampling options (Riemannian is a property of the metric, not the particular HMC algorithm) but then you have dangling parameters (RMHMC requires fixed point iteration parameters, and if we flatten everything then we have allow other algorithms to have those parameters set or do really awkward parameter validation after the fact.

> 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."

That is how it currently works, provided we elevate "method" to a special argument that doesn't need the prefix.

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

There's a bug somewhere in the translation of the optimization code. I thought I had fixed it.

[Bob Carpenter]

Cool --- I can be a tiebreaker until others
weigh in.

Jiqiang --- what are you thinking about for
RStan here? I don't know what the conventions are
there.

More inline.

On 7/10/13 5:12 PM, Michael Betancourt wrote:
> Most of the changes you want can be implemented by adding a buffer layer in the hierarchy, although I'm not sure I agree with such a strategy. Comments inline.
>
>> 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.
>
> This is new behavior and a break from 1.3.

I think we can set our standards a bit higher than 1.3.

> Personally, I'd prefer to have a default method.

How about having the default method be "help" :-)

Either way's OK with me. I have a slight preference for Daniel's suggested
"help" behavior. It's what git does, and I think git's a good
model to follow.

>> 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"
>
> I can see some of the motivation to removing method, but it does create a singular argument that has to implemented differently from all of the rest.

If it's not too hard to implement, I'd very much like to do it
this way, again following the git model.

>> 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"
>
> Not the desired behavior. I'm looking into it.

If you think of this evaluating left to right, then how about having it
call the optimize command, which shouldn't take "sample" as an argument.

We should probably check for duplicate defs everywhere. 1.3
doesn't do that.

>> 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.
>
> My opinion is to go with a categorical argument, test=test_name
>
> We can make gradient the only test here and leave the rest to be added later.

I like Daniel's suggestion of "./model test". I don't know what to do for
trying to run more than one test at once.

>> 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.
>
> Well, it used to do that. I'm not sure what changes you've made.

Agreed. that it should be contextual.

>> 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 strongly disagree. Not only do I dislike flattening the sampling options (Riemannian is a property of the metric, not the particular HMC algorithm) but then you have dangling parameters (RMHMC requires fixed point iteration parameters, and if we flatten everything then we have allow other algorithms to have those parameters set or do really awkward parameter validation after the fact.

I agree with Michael's disagreement :-)

I only like flattening the top one, ./model sample.
All of the samplers have a similar flavor in terms of their
input and output. The metric and algorithm (?) make sense
as configurable options. That's what's so cool about the
way Michael set up mcmc!

>> 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."
>
> That is how it currently works, provided we elevate "method" to a special argument that doesn't need the prefix.

Agreed.

>> PS. I just made it crash:
>> make models/basic_distributions/normal
>> ./models/basic_distributions/normal method=optimize
>
> There's a bug somewhere in the translation of the optimization code. I thought I had fixed it.

Is the BFGS code not in? I saw the note on Nesterov.

- Bob

[Jiqiang Guo]

I do not know what convention to follow in R and I am fine with it.

In RStan, currently if we use function stan_model to create a stanmodel object, there are two methods defined: sampling and optimizing.  For example,

sm <- stan_model(file = 'schools.stan')

fit <- sampling(sm, data = , iter = ,....)

op <- optimizing(sm, data = , method = ...)

So it is like Stan 2.0 though we have a shortcut function stan() that wraps the above stan_model and sampling.  But function stan() cannot do optimization and currently there is no shortcut function.  And now, test gradient is done through sampling function (so function stan can do test gradient) by using argument test_gradient = TRUE.  I can define another function for stanmodel object so we can do 

tt <- test(sm, data = ...)

--

Jiqiang 

[Michael Betancourt]

Just pushed some changes.

1) Default now calls help.
2) The actual model name is printed in the help output (had to make model::model_name() a static method)
3) method= is now optional (if the argument matches a valid method then "method=" is automatically pretended)
4) A method must be explicitly satisfied or the code dies.
5) Reenabled the verbose help/help-all so Bob can take a look
6) A few bug fixes, mostly to do with specifying the random seed

[Bob Carpenter]

Great. I'll check it out. The model name
should've been static all along.

- Bob

[Michael Betancourt]

Just pushed a refactor of the tests: now test is a method value so you can call

./model diagnostic

which defaults to

./model diagnostic test=gradient

I believe this just leaves

-- Help output formatting

Bob, did you take a look at the help/help-all output? Any comments?
Dan, you added "short_description". Is that something you wanted to follow up on?

-- Tests

Dan, where are we on tests? I'm happy to write a bunch of tests if we decide on exactly what we want tested.

[Bob Carpenter]

I'm having to make fairly substantial changes to all
of our infrastructure classes in order to add a template
parameter to allow the Jacobian to be suppressed.

I really hope this isn't going to be a merge trainwreck, but
I'm very nervous.

The biggest issue is that there's no way to have a template
parameter in a virtual function.

So I'm removing all the virtual functions and also have
removed prob_grad_ad.

One issue that's tripping me up is that there are models
checked into version control rather than being generated.
I'll do the same, but it'd be nice if they could be
generated on the fly so that they were guaranteed to match
what was currently being done by the compiler.

I hope the new command structure and what I'm doing with the

- Bob

[Daniel Lee]

On Jul 13, 2013, at 10:23 PM, Bob Carpenter wrote:
> I'm having to make fairly substantial changes to all
> of our infrastructure classes in order to add a template
> parameter to allow the Jacobian to be suppressed.

Sounds good.

> I really hope this isn't going to be a merge trainwreck, but
> I'm very nervous.

Don't worry about it. If it's difficult, I'll help with the merge.

> The biggest issue is that there's no way to have a template
> parameter in a virtual function.
>
> So I'm removing all the virtual functions and also have
> removed prob_grad_ad.
>
> One issue that's tripping me up is that there are models
> checked into version control rather than being generated.
> I'll do the same, but it'd be nice if they could be
> generated on the fly so that they were guaranteed to match
> what was currently being done by the compiler.

Hmm. Which models? We could generate them instead if that makes sense.

> I hope the new command structure and what I'm doing with the

...?

> - Bob
>


More below.

> On 7/13/13 6:43 PM, Michael Betancourt wrote:
>> Just pushed a refactor of the tests: now test is a method value so you can call
>>
>> ./model diagnostic
>>
>> which defaults to
>>
>> ./model diagnostic test=gradient

That works.

>>
>> I believe this just leaves
>>
>> -- Help output formatting

Yeah, I think we need to get the help / formatting to a point where users can understand what's going on without being overwhelmed.

>>
>> Bob, did you take a look at the help/help-all output? Any comments?
>> Dan, you added "short_description". Is that something you wanted to follow up on?

Sorry -- the "short description" was something that wasn't well thought through. We can drop that. Comments:
- the model displayed in the help should be displayed for the "Usage: model..." line also.
- the model name is incorrect. I think we should be grabbing the command line call, not the model name. I compiled 'models/basic_distributions/normal' and I'm seeing the help line as 'normal_model'
- I'm still in favor of dropping the "method=" for the analysis methods. I see the point about the implementation having to treat it differently, but if we could, I'm in favor of dropping it.

To get back to an old comment:

>> 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 strongly disagree. Not only do I dislike flattening the sampling options (Riemannian is a property of the metric, not the particular HMC algorithm) but then you have dangling parameters (RMHMC requires fixed point iteration parameters, and if we flatten everything then we have allow other algorithms to have those parameters set or do really awkward parameter validation after the fact.

- I'm seeing this as front end code and making it simple for a user. If a user wanted to do RMHMC, I wouldn't expect comprehension of the structure of our command line arguments for the parent options in order to get how to use RMHMC. With regards to design for the algorithm, of course this is a good thing. I'm just thinking about using Stan. Also, it makes it easier writing coherent doc if every analysis method is treated as its own. I know, this means for duplicated doc, but there shouldn't be too many options, right? We're not talking hundreds.

>>
>> -- Tests
>>
>> Dan, where are we on tests? I'm happy to write a bunch of tests if we decide on exactly what we want tested.
>>

I've added some unit tests so far to test just the basic argument classes. Want to add some more? One of them is already failing from different behavior that was introduced in your patch. We also need to adjust all our calls to our models. And our command test should flex a lot of these command line level tests. Such as calling multiple options at once, calling all options, calling valid options with invalid options, etc. Do you want to take a look? I've already started to rebuild the test from scratch.


Daniel


>
> --
> 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]

- the model displayed in the help should be displayed for the "Usage: model..." line also.
- the model name is incorrect. I think we should be grabbing the command line call, not the model name. I compiled 'models/basic_distributions/normal' and I'm seeing the help line as 'normal_model'

The suffix _model is easily removed, but I don't know how to get the path into the C++ side.

 

- I'm still in favor of dropping the "method=" for the analysis methods. I see the point about the implementation having to treat it differently, but if we could, I'm in favor of dropping it.

That has been fixed -- I think you missed an intermediate email.

 

- I'm seeing this as front end code and making it simple for a user. If a user wanted to do RMHMC, I wouldn't expect comprehension of the structure of our command line arguments for the parent options in order to get how to use RMHMC. With regards to design for the algorithm, of course this is a good thing. I'm just thinking about using Stan. Also, it makes it easier writing coherent doc if every analysis method is treated as its own. I know, this means for duplicated doc, but there shouldn't be too many options, right? We're not talking hundreds.

The permutations get up there. I'd rather we have the full, hierarchical options and then a bunch of short-cuts for commonly used algorithms.  Otherwise it's going to be a pain in the ass to customize the samplers.

I've added some unit tests so far to test just the basic argument classes. Want to add some more? One of them is already failing from different behavior that was introduced in your patch. We also need to adjust all our calls to our models. And our command test should flex a lot of these command line level tests. Such as calling multiple options at once, calling all options, calling valid options with invalid options, etc. Do you want to take a look? I've already started to rebuild the test from scratch.

We should be able to auto-generate these, right?  Given a list of valid arguments and invalid arguments we can randomly (or full combinatorically) create valid or invalid argument strings and check for the expected result.

[Daniel Lee]

On Jul 15, 2013, at 10:22 AM, Michael Betancourt wrote:

> - the model displayed in the help should be displayed for the "Usage: model..." line also.
> - the model name is incorrect. I think we should be grabbing the command line call, not the model name. I compiled 'models/basic_distributions/normal' and I'm seeing the help line as 'normal_model'
>
> The suffix _model is easily removed, but I don't know how to get the path into the C++ side.

I was thinking about using argv[0]. Is that guaranteed to be the executable call?

>
> - I'm still in favor of dropping the "method=" for the analysis methods. I see the point about the implementation having to treat it differently, but if we could, I'm in favor of dropping it.
>
> That has been fixed -- I think you missed an intermediate email.

Yeah, I must have. I've been sick and on antibiotics. My head hasn't felt right in a bit. Although, I'm up to speed and when I run help, it still shows as "method=<>"

>
> - I'm seeing this as front end code and making it simple for a user. If a user wanted to do RMHMC, I wouldn't expect comprehension of the structure of our command line arguments for the parent options in order to get how to use RMHMC. With regards to design for the algorithm, of course this is a good thing. I'm just thinking about using Stan. Also, it makes it easier writing coherent doc if every analysis method is treated as its own. I know, this means for duplicated doc, but there shouldn't be too many options, right? We're not talking hundreds.
>
> The permutations get up there. I'd rather we have the full, hierarchical options and then a bunch of short-cuts for commonly used algorithms. Otherwise it's going to be a pain in the ass to customize the samplers.

I think I see where you're going with it now. If we can get the output to look appropriate, I'm happy with it.

> I've added some unit tests so far to test just the basic argument classes. Want to add some more? One of them is already failing from different behavior that was introduced in your patch. We also need to adjust all our calls to our models. And our command test should flex a lot of these command line level tests. Such as calling multiple options at once, calling all options, calling valid options with invalid options, etc. Do you want to take a look? I've already started to rebuild the test from scratch.
>
> We should be able to auto-generate these, right? Given a list of valid arguments and invalid arguments we can randomly (or full combinatorically) create valid or invalid argument strings and check for the expected result.

The basic argument classes (arg_*.hpp) should be unit tested on their own. One of those is failing now. (categorical_argument)

With regards to auto-generating... yes, but I haven't quite figured out what to test exactly. If you want to help define what to test, how to specify them, etc., I think I could help make that happen. I'm not the sharpest right now and not sure I can get it up and operational in short order.

[Michael Betancourt]

I was thinking about using argv[0]. Is that guaranteed to be the executable call?

Yeah, you're absolutely right.  I can change that.

 

Yeah, I must have. I've been sick and on antibiotics. My head hasn't felt right in a bit. Although, I'm up to speed and when I run help, it still shows as "method=<>"

Right, help will have to be tweaked a bit but I think it's hack-able.

 

The basic argument classes (arg_*.hpp) should be unit tested on their own. One of those is failing now. (categorical_argument)

With regards to auto-generating... yes, but I haven't quite figured out what to test exactly. If you want to help define what to test, how to specify them, etc., I think I could help make that happen. I'm not the sharpest right now and not sure I can get it up and operational in short order.

But righting tests while on meds is the best way to right tests that make sense, right?  :-)

I'll try to think about an autogenerated approach, although I won't have much time to dedicate for the next few days.

[Bob Carpenter]

On 7/15/13 10:22 AM, Michael Betancourt wrote:
...

> - I'm still in favor of dropping the "method=" for the analysis methods. I see the point about the implementation
> having to treat it differently, but if we could, I'm in favor of dropping it.
>
>
> That has been fixed -- I think you missed an intermediate email.

Great!

> - I'm seeing this as front end code and making it simple for a user. If a user wanted to do RMHMC, I wouldn't expect
> comprehension of the structure of our command line arguments for the parent options in order to get how to use
> RMHMC. With regards to design for the algorithm, of course this is a good thing. I'm just thinking about using Stan.

Well, they may not comprehend it, but if it's there, it
makes it easier on the development side. How it's documented
and understood by users is a different issue.

For git, what you have is:

git <git-options> command <command-options>

Things like logins are in git-options and anything command-specific
in the command options.

Git's own help, which you run as:

> git help <command>

does not document the git-options, just the help-specific ones.
Basically each <command> gets a man page.

Other than naming the command options coherently across commands,
I don't see that there's much we can do. We can doc families of
commands that have similar options together.

> Also, it makes it easier writing coherent doc if every analysis method is treated as its own. I know, this means for
> duplicated doc, but there shouldn't be too many options, right? We're not talking hundreds.

We just had a similar discussion on the list about propagating
the ~ syntax to each and every distribution's doc. So far, no
feedback from anyone on that, so I'll probably do it.

Having said that, the big issue with cut-and-paste is that it
makes any modifications going forward much harder.

All things being equal, simplicity for the user is a good
tiebreaker. So definitely the help options for a subcommand
should dump everything out

- Bob

[Michael Betancourt]

I just pushed a bunch of updates - aside from testing and doc this should be considered a first "draft".  Let me know if you prefer any tweaks to the output.

Regarding testing, the only question is just how much we want to test.  It's straightforward to take a configuration from samples.csv, turn it into an argument string, execute a test model, and then compare the consistency of the original configuration and the Stan output.  But how many configurations should we test?  Would it be enough to just test one configuration for each method, i.e. sample, optimize, and test?  If we wanted more, should we test every combination or just enough combinations so that every argument is used at least once?

--
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+unsubscribe@googlegroups.com.

[Bob Carpenter]

On 7/18/13 9:11 AM, Michael Betancourt wrote:
> I just pushed a bunch of updates - aside from testing and doc this should be considered a first "draft". Let me know if
> you prefer any tweaks to the output.

Thanks. I'll take a look ASAP.

> Regarding testing, the only question is just how much we want to test. It's straightforward to take a configuration
> from samples.csv, turn it into an argument string, execute a test model, and then compare the consistency of the
> original configuration and the Stan output. But how many configurations should we test? Would it be enough to just
> test one configuration for each method, i.e. sample, optimize, and test? If we wanted more, should we test every
> combination or just enough combinations so that every argument is used at least once?

Daniel tried to be exhaustive in testing config in the original
command. I'm not sure what was being tested for results.

It might actually be simpler with subcommands, because fewer
options should be viable at any given time.

- Bob

[Michael Betancourt]

The problem was that before the options were mostly flat (and not every combination was tested). With a hierarchy testing every combination becomes combinatorial which is just about the worst scaling you can have.

[Daniel Lee]

On Jul 18, 2013, at 1:55 PM, Michael Betancourt wrote:
> The problem was that before the options were mostly flat (and not every combination was tested). With a hierarchy testing every combination becomes combinatorial which is just about the worst scaling you can have.

Hence, my desire to have each method be independent.

I would want to test:
1) for each method, all possible combinations of options
2) each combination of methods, but not testing the interaction of the method options and the methods.

Does that make sense?

>
> On Jul 18, 2013, at 6:02 PM, Bob Carpenter <ca...@alias-i.com> wrote:
>
>> Daniel tried to be exhaustive in testing config in the original
>> command. I'm not sure what was being tested for results.
>>
>> It might actually be simpler with subcommands, because fewer
>> options should be viable at any given time.
>

> --
> 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.

[Michael Betancourt]

Making each method independent would just push the combinatorics to the method specifications. My main concern is the pure number of tests, but if that's not a concern let me play around a bit. I'm pretty sure I can automate all of this, if not directly in the test then at least in some scripts.

[Daniel Lee]

On Thu, Jul 18, 2013 at 5:29 PM, Michael Betancourt <betan...@gmail.com> wrote:

Making each method independent would just push the combinatorics to the method specifications. 

Yeah. But each method has to be tested combinatorially, right? Even if it is nested.

My main concern is the pure number of tests, but if that's not a concern let me play around a bit.  I'm pretty sure I can automate all of this, if not directly in the test then at least in some scripts.

High numbers of tests are fine. The google tests don't take a long time to run. The simple model we use to test the interface also doesn't take much time.

We'll need to test enough of the options one way or another. Embedded options makes sense to a certain degree, but I think the tests should be the same no matter how we implement it under the hood.

[Bob Carpenter]

On 7/18/13 5:35 PM, Daniel Lee wrote:

> On Thu, Jul 18, 2013 at 5:29 PM, Michael Betancourt <betan...@gmail.com <mailto:betan...@gmail.com>> wrote:
>
> Making each method independent would just push the combinatorics to the method specifications.
>
>
> Yeah. But each method has to be tested combinatorially, right? Even if it is nested.

Yes, but if you have 10 methods with 5 binary parameters, that's 10 * 2^5
rather than 1 method with 10 parameters, which is 2^10.

But what if the parameters are continuous? We have to use some judgment to
test reasonable values, like negative ones if there's a positivity constraint.

> My main concern is the pure number of tests, but if that's not a concern let me play around a bit. I'm pretty sure
> I can automate all of this, if not directly in the test then at least in some scripts.
>
>
> High numbers of tests are fine. The google tests don't take a long time to run. The simple model we use to test the
> interface also doesn't take much time.

Yup -- those aren't the bottleneck. It's all the compilation.

> We'll need to test enough of the options one way or another. Embedded options makes sense to a certain degree, but I
> think the tests should be the same no matter how we implement it under the hood.

Yes, but we can also make the tests more modular if the code's
more modular. We can't just test everything. For the code
where there's different behavior at different scales (like log1m(x),
which goes to second-order Taylor series then first-order), we test
where we know scales change.

- Bob

[Michael Betancourt]

Right now I'm only considering the testing of argument specification, i.e. are all valid inputs actually valid? Testing valid values is another test altogether and one that is not heavily tested in the current version of Stan. Again, I think this can be readily automated by leveraging the argument tree in argument parser, but it will take some time to build the framework.

[Michael Betancourt]

Okay, I just checked in my idea.  Give it a try by running pulling, running "mkfast test/gm/arguments/argument_configuration", and then checking the output in test/gm/arguments/*.config.

The basic idea is that the argument_probe class runs through the argument tree.  For each valued argument (list and singleton) it varies possible inputs (all valid values plus a "fail" value for list arguments, and good and bad values for singleton arguments that must be defined in each implementation) and then prints out the resulting tree from the original argument (all other values are set to defaults).  Tests will be broken up by the top list of arguments (method, id, data, output, random, etc).

The tests would then take these files, convert them to an argument string (basically by removing newlines and whitespace), and then try to run them through an actual model to validate success.  I just wanted to see if you guys agreed with the general strategy or not before proceeding.  Thoughts?

Dan, do you know of a way to run an equivalent of `ls` from within the tests?

[Michael Betancourt]

Pushed a partial implementation of the argument configuration tests.  Every good and bad combination of arguments is run through a test model and tested to have either the correct configuration or fail appropriately.  I still need to split up the tests into each base argument and then test the interactions between the method argument and all others, but that shouldn't be hard to finish tomorrow.  Incidentally, I found a few bugs (some serious, some minor) building the tests.

Also, the nesterov optimization is failing for some reason.  I'll investigate tomorrow, but if anyone has any ideas I am all ears.

[Bob Carpenter]

On 7/22/13 12:32 PM, Michael Betancourt wrote:
> Pushed a partial implementation of the argument configuration tests. Every good and bad combination of arguments is run
> through a test model and tested to have either the correct configuration or fail appropriately. I still need to split
> up the tests into each base argument and then test the interactions between the method argument and all others, but that
> shouldn't be hard to finish tomorrow. Incidentally, I found a few bugs (some serious, some minor) building the tests.

Great!

> Also, the nesterov optimization is failing for some reason. I'll investigate tomorrow, but if anyone has any ideas I am
> all ears.

Failing to function or failing to compile?

I changed it a fair bit on the dev branch to make
it compile, but there weren't any tests.

Anybody want to volunteer to add tests?

We should also test all the generated model code configs.

I was sort of expecting more pushback on the jumbo
pull request, but we can add this all retroactively.

- Bob

>
> On Fri, Jul 19, 2013 at 5:44 PM, Michael Betancourt <betan...@gmail.com <mailto:betan...@gmail.com>> wrote:
>
> Okay, I just checked in my idea. Give it a try by running pulling, running "mkfast
> test/gm/arguments/argument_configuration", and then checking the output in test/gm/arguments/*.config.
>
> The basic idea is that the argument_probe class runs through the argument tree. For each valued argument (list and
> singleton) it varies possible inputs (all valid values plus a "fail" value for list arguments, and good and bad
> values for singleton arguments that must be defined in each implementation) and then prints out the resulting tree
> from the original argument (all other values are set to defaults). Tests will be broken up by the top list of
> arguments (method, id, data, output, random, etc).
>
> The tests would then take these files, convert them to an argument string (basically by removing newlines and
> whitespace), and then try to run them through an actual model to validate success. I just wanted to see if you guys
> agreed with the general strategy or not before proceeding. Thoughts?
>
> Dan, do you know of a way to run an equivalent of `ls` from within the tests?
>
>
> On Fri, Jul 19, 2013 at 9:41 AM, Michael Betancourt <betan...@gmail.com <mailto:betan...@gmail.com>> wrote:
>
> Right now I'm only considering the testing of argument specification, i.e. are all valid inputs actually valid?
> Testing valid values is another test altogether and one that is not heavily tested in the current version of
> Stan. Again, I think this can be readily automated by leveraging the argument tree in argument parser, but it
> will take some time to build the framework.
>

> On Jul 19, 2013, at 1:23 AM, Bob Carpenter <ca...@alias-i.com <mailto:ca...@alias-i.com>> wrote:
>
> > But what if the parameters are continuous? We have to use some judgment to
> > test reasonable values, like negative ones if there's a positivity constraint.
>
>
>

[Michael Betancourt]

It's compiles but freezes during execution.

[Michael Betancourt]

Yikes, nesterov_gradient is all kinds of crazy. It looks like the step size initialization routine actually performs a gradient descent and hits the optimum before a Nesterov step is even made. Then the step function implements another step size initialization (entirely duplicated from the initialization algorithm) that infinite loops when starting at the mode.

Anyone who actually knows how this is supposed to work want to fix it? If I don't hear from anyone by tomorrow morning I'll go ahead and clean it up while hoping I don't mess up the algorithm.

[Marcus Brubaker]

I have no idea how it's supposed to behave but even before the refactoring I was never able to make it really work on any of my problems which was part of why I implemented BFGS.  I think Matt would be the only one who really knows what's going on with it.

[Bob Carpenter]

We can just get rid of it if nobody wants to fix it.
It wasn't going to be the default optimizer.

- Bob

[Michael Betancourt]

Whatever Matt implemented, it's nothing like the descriptions I found.  I'm guessing he was doing something tricky (my kingdom for some algorithmic commenting!).  In any case I just hacked something from the references I found that works for a simple model, as well as finishing up the argument configuration tests.  There's way too much code duplication, but it works.  Once everyone signs off (after perhaps a few round of tweaks) we can merge dev in again and then update the doc.

--
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+unsubscribe@googlegroups.com.

[Daniel Lee]

Thanks for the updates. It looks pretty good. I ran the test, but it's failing (I think it's the nesterov optimization thing you mentioned).

I'm running a simple model, the one in src/models/command.stan. (to build: make src/models/command)

Some things I've found:

- init with a negative number hangs:  ./src/models/command init=-1 sample data=src/models/command.data.R

It doesn't sample, doesn't do anything.

- percentages are off for calculating iterations:  ./src/models/command sample num_warmup=10 num_samples=10 data=src/models/command.data.R
...
Iteration:  1 / 20 [  5%]  (Warmup)
Iteration: 11 / 20 [ 55%]  (Sampling)
...

Should be 0%, 50%?

Was the intention to have 20/20 be [100%]?

- can we rename the 'diagnostic' method to a verb? The other options are verbs: 'sample' and 'optimize'.

[Michael Betancourt]

> - percentages are off for calculating iterations: ./src/models/command sample num_warmup=10 num_samples=10 data=src/models/command.data.R
> ...
> Iteration: 1 / 20 [ 5%] (Warmup)
> Iteration: 11 / 20 [ 55%] (Sampling)
> ...
> Should be 0%, 50%?

Yeah, the code was assuming that the current iteration had finished. Fixed.

> Was the intention to have 20/20 be [100%]?

The problem is that the number of warmup iterations and then samples are both smaller than the refresh rate. Should we always print out the last value?

> - can we rename the 'diagnostic' method to a verb? The other options are verbs: 'sample' and 'optimize'.

Any preferences? I went with "diagnostic", which would be come "diagnose" because I wanted to reserve "test" for specifying the actual test. Go with "diagnose" or do something like "method=test test_name=…"?

[Michael Betancourt]

Do you have the latest version? I did a fresh pull (which grabbed the code I pushed the other day) and src/models/command ran fine.

[Bob Carpenter]

On 7/25/13 2:45 PM, Daniel Lee wrote:
> Thanks for the updates. It looks pretty good. I ran the test, but it's failing (I think it's the nesterov optimization
> thing you mentioned).

We can just get rid of the Nesterov optimization method
unless you think it's a good alternative to BFGS in some cases.

> I'm running a simple model, the one in src/models/command.stan. (to build: make src/models/command)
>
> Some things I've found:
>
> - init with a negative number hangs: ./src/models/command init=-1 sample data=src/models/command.data.R
> It doesn't sample, doesn't do anything.
>
> - percentages are off for calculating iterations: ./src/models/command sample num_warmup=10 num_samples=10
> data=src/models/command.data.R
> ...
> Iteration: 1 / 20 [ 5%] (Warmup)
> Iteration: 11 / 20 [ 55%] (Sampling)
> ...
> Should be 0%, 50%?

Eh? Isn't 11/20 55%?

> Was the intention to have 20/20 be [100%]?

The two options that would make sense would be 11/20 and 55%
or 1/10 and 10%. I prefer the continuous numbering because
it gives you a better overall sense of progress. Restarting
progress monitors is a UI no-no. We could print the actual
number of iterations and the number of samples collected; the
latter could roll in thinning.

> - can we rename the 'diagnostic' method to a verb? The other options are verbs: 'sample' and 'optimize'.

That'd be "diagnose". I like it.

- Bob

[Michael Betancourt]

We can just get rid of the Nesterov optimization method

unless you think it's a good alternative to BFGS in some cases.

That's also easy.

Eh?  Isn't 11/20 55%?

My understanding of the issue is that we're processing iteration 11, but it hasn't been completed yet.  So we're really only at (i - 1) / n, not i / n.  At least, that's the motivation behind the fix I pushed last night.

Also there's still the question of whether or not to force output on the last iteration as well as the first.

 

That'd be "diagnose".  I like it.

No problem.  I'll push a fix in just one minute. 

[Michael Betancourt]

a) I went back to the original progress output, so the percent complete assumes that the current iteration has already finished (it looked awkward otherwise).

b) But I added an automatic output of the final sample so you'll always see progress warnings for the first and last sample.

c) Because Nesterov seems to be working for me I didn't trash it, but I did change the default optimization algorithm to BFGS.

I think that addresses all of the outstanding issues -- did I miss anything?

[Michael Betancourt]

Sorry I missed the meeting yesterday -- any word on how to proceed with the arg refactor? Are there more tests that should be implemented or output tweaks to be made, or should I start updating the manual?

[Bob Carpenter]

The arg refactor's a go (thank Daniel for all the
testing). Thanks for being so patient --- it takes forever
to get a lot of feedback and test, but I think it's
worth it in the end.

That's the last big ticket item for 2.0 --- there are
a few minor items that I can knock off in a day or two.
I'm waiting on a bunch of doc updates until after your
revisions.

The big to-do item for AFTER 2.0 is getting fvar<fvar<var> >
working on Windows --- the heavy templating is blowing out the
32-bit compiler that comes with RTools. A 64-bit compile might help,
but it remains to figure out how to make it easy (or even possible)
to install.

- Bob

[Michael Betancourt]

Sorry it took so long -- I spent A LOT of time trying to make LaTeX do things before giving up and writing a Bash script to prepare all of the formatting for me.  Incidentally, is there a place we should be putting handy shell scripts for things like formatting output for the manual or generating files for tests?

In any case, execution works, bin/print works, and all of the unit-tests pass.  I've also updated the manual with the new changes (and some formatting tweaks).  I'll leave it to you guys if you want to try a quick open-beta with users or just go ahead and push the changes.

Also, I am very excited about getting around the Windows limitations and getting some higher-order autodiff.  It'll give me something to do besides writing papers.

--
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+unsubscribe@googlegroups.com.

[Bob Carpenter]

On 8/5/13 6:37 AM, Michael Betancourt wrote:
> Sorry it took so long -- I spent A LOT of time trying to make LaTeX do things before giving up

You should ask here before getting too bogged down. I've been
LaTeXing since 1985, so I've built up some experience over time.

I'm also a nut about typesetting and want to inspect any big
changes to doc structure for consistency with general typesetting
conventions. For instance, I had to rejigger the function
description environment after a third-party patch to allow line breaks
because it added too much space between the signature and definition --- the
rule is that text needs to be closest to its own header so that
what goes with what is identifiable when squinting.

Despite producing beautiful docs, the language itself and its
package structure is even more of a nightmare than R and I've never
gotten deep into LaTeX package design or used too many raw TeX escapes.

I've also gotten great help from our user list after struggling with
index linking.

> and writing a Bash script
> to prepare all of the formatting for me.

Formatting of what?

> Incidentally, is there a place we should be putting handy shell scripts for
> things like formatting output for the manual or generating files for tests?

I'd put ones that go with the manual in the manual subdirectory.

I would rather not have the manual build target depend on running
scripts unless absolutely necessary.

> In any case, execution works, bin/print works, and all of the unit-tests pass. I've also updated the manual with the
> new changes (and some formatting tweaks). I'll leave it to you guys if you want to try a quick open-beta with users or
> just go ahead and push the changes.

Awesome.

I can give it a whirl. It's all on your arg refactor branch?

I'm waiting until after the big merge before doing any more work on
Stan code, so I'd like to get it in ASAP. (I have to do things like
make some major additions and revisions to the manual, change the parser
which is going to cause conflicts if the last parser fix isn't merged
first, etc.)

> Also, I am very excited about getting around the Windows limitations and getting some higher-order autodiff. It'll give
> me something to do besides writing papers.

We can give you a remote login to the Jenkins box if you want to sort it out :-)

- Bob

[Michael Betancourt]

> Formatting of what?

Formatting of the argument hierarchy into something marginally pretty in LaTeX. Not only converting math symbols but also proper indentation (I hit the recursive limit of the description environment reaaaaaaally quickly and had to hack something instead) and macros for each kind of argument.

> I can give it a whirl. It's all on your arg refactor branch?

Yessir.

> We can give you a remote login to the Jenkins box if you want to sort it out :-)

Okay, I don't want it __that__ much. :-)

[Bob Carpenter]

On 8/5/13 12:48 PM, Michael Betancourt wrote:
>> Formatting of what?
>
> Formatting of the argument hierarchy into something marginally pretty in LaTeX. Not only converting math symbols but also proper indentation (I hit the recursive limit of the description environment reaaaaaaally quickly and had to hack something instead) and macros for each kind of argument.

OK. I'll take a look.

>> I can give it a whirl. It's all on your arg refactor branch?
>
> Yessir.
>
>> We can give you a remote login to the Jenkins box if you want to sort it out :-)
>
> Okay, I don't want it __that__ much. :-)

The real bottleneck is the probability functions and the
OperandsAndPartials abstraction, which needs to be generalized
to var.

At that point, I don't think we should let not building on
Windows stop us in our tracks on RMHMC.

- Bob

[Daniel Lee]

On Mon, Aug 5, 2013 at 1:08 PM, Bob Carpenter <ca...@alias-i.com> wrote:

The real bottleneck is the probability functions and the
OperandsAndPartials abstraction, which needs to be generalized
to var.

At that point, I don't think we should let not building on
Windows stop us in our tracks on RMHMC.

Yes. We should definitely not stop working on RMHMC. I'm trying to break things apart. We'll see how that goes. If someone wants to figure out how to install a different compiler on Windows, I'm more than happy installing it on that machine.
 

[Ben Goodrich]

It may work as is if you give Windows enough Virtual Memory. It took me like 15GB of total memory to do that determinant test, which compiled but then ran into linker errors on my laptop. Obviously, 15GB is unacceptable, so we should try to separate things more.

Have you tried these clang++ / VC binaries

http://www.ishani.org/web/articles/code/clang-win32/

But even if that works, I don't think it helps much because we still are pretty much stuck telling users to just install Rtools.

Ben

[Bob Carpenter]

On 8/5/13 2:29 PM, Ben Goodrich wrote:
> On Monday, August 5, 2013 2:01:25 PM UTC-4, Daniel Lee wrote:
>

> On Mon, Aug 5, 2013 at 1:08 PM, Bob Carpenter <ca...@alias-i.com <javascript:>> wrote:

> It may work as is if you give Windows enough Virtual Memory. It took me like 15GB of total memory to do that determinant
> test, which compiled but then ran into linker errors on my laptop. Obviously, 15GB is unacceptable, so we should try to
> separate things more.

Indeed. I also think we can make a sequence
of instantiation files to precompile into more
translation units the way we did for the grammars.

> Have you tried these clang++ / VC binaries
>
> http://www.ishani.org/web/articles/code/clang-win32/
>
> But even if that works, I don't think it helps much because we still are pretty much stuck telling users to just install
> Rtools.

If we find something that works, we could write our own
Windows installers along the lines of RTools.

I don't know how to do it any more than I know how to write
Linux installers or Mac .pkg installers, though.

- Bob