Using arguments (src/stan/gm/arguments/) as configuration; refactoring src/stan/common/command.hpp

[Daniel Lee]

I want to refactor the function:

    template <class Model>
    int command(int argc, const char* argv[])

 

It is currently 769 lines and RStan pretty much reimplements the whole thing to run most of the functions. (I haven't looked at Allen's code, but my guess is PyStan also reimplements all of it.)

One way to deal with this is to change the interface to something like:

    template <class Model, class SampleRecorder, class DiagnosticRecorder,
                     class MessageRecorder, class InterruptHandler, ...>

    int command(std::vector<argument*> arguments)

Each interface would be responsible for implementing the additional template parameters including the recorders and the "..." (tbd).

This function would then run based on what's passed in as arguments. It would be on the interfaces to translate to this argument structure.

This would allow interface designers to focus on implementing the different recorders, handlers, etc. and translate from the interface to an argument structure. This should stay backwards compatible always, so interface implementers can be behind Stan without penalty and being forced to change interface code with every new version.

Michael, was the original intent of the argument structure to be used as configuration?

As it currently stands

- there are no tests

- not sure how easy it is to validate configurations

- how would we handle changes to configuration over time?

- what helper methods are available for constructing a valid configuration?

For everyone else, anyone deal with configuration structures and have a pattern that they know about, is easy to maintain and implement, and will serve the purpose of capturing and communicating the configuration? Or... any thoughts on how to shape the arguments into something really easy to use cross-interface?

I think the stuff in src/stan/gm/arguments/ is almost there, but as it stands now, it needs a little bit more work to make it really usable. Maybe a combination of doc, tests, and factories might be enough? I'd love an extra set of eyes on the code.

Daniel

[Michael Betancourt]

I want to refactor the function:

    template <class Model>
    int command(int argc, const char* argv[])

 

It is currently 769 lines and RStan pretty much reimplements the whole thing to run most of the functions. (I haven't looked at Allen's code, but my guess is PyStan also reimplements all of it.)

Yup.

One way to deal with this is to change the interface to something like:

    template <class Model, class SampleRecorder, class DiagnosticRecorder,
                     class MessageRecorder, class InterruptHandler, ...>

    int command(std::vector<argument*> arguments)

Each interface would be responsible for implementing the additional template parameters including the recorders and the "..." (tbd).

This function would then run based on what's passed in as arguments. It would be on the interfaces to translate to this argument structure.

This would allow interface designers to focus on implementing the different recorders, handlers, etc. and translate from the interface to an argument structure. This should stay backwards compatible always, so interface implementers can be behind Stan without penalty and being forced to change interface code with every new version.

This has also been my thinking.  The alternative is to have each interface define some intermediate object, say a configuration string matching what we input to argv in CmdStan and let the parser handle the unpacking.

Michael, was the original intent of the argument structure to be used as configuration?

The original intent was to represent the configuration internally but it should serve equally as well as an external object.

As it currently stands

- there are no tests

- not sure how easy it is to validate configurations

- how would we handle changes to configuration over time?

- what helper methods are available for constructing a valid configuration?

I think you missed https://github.com/stan-dev/stan/tree/develop/src/test/integration/gm/arguments.

In particular, argument_parser_test builds up all possible combinations of valid/invalid arguments

dynamically and tests that each yields a valid/invalid configuration.  Valid/invalid values are

defined by each argument and the configurations are built up from the recursive dependencies

of each argument so that nothing has to be hand-coded.

For everyone else, anyone deal with configuration structures and have a pattern that they know about, is easy to maintain and implement, and will serve the purpose of capturing and communicating the configuration? Or... any thoughts on how to shape the arguments into something really easy to use cross-interface?

I think the stuff in src/stan/gm/arguments/ is almost there, but as it stands now, it needs a little bit more work to make it really usable. Maybe a combination of doc, tests, and factories might be enough? I'd love an extra set of eyes on the code.

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/d/optout.

[Bob Carpenter]

Turns out I do have a lot of thoughts on this. More below.

On Jun 24, 2014, at 6:05 AM, Daniel Lee <bea...@alum.mit.edu> wrote:

> I want to refactor the function:

> template <class Model>
> int command(int argc, const char* argv[])
>
> It is currently 769 lines and RStan pretty much reimplements the whole thing to run most of the functions. (I haven't looked at Allen's code, but my guess is PyStan also reimplements all of it.)

That would be great. I thought that was where you
were heading when you dove into R in the first place.

This is also why I think the design for the current R
interface proposal from Andrew and Ben is so important.
We're thinking a 3.0 version that breaks backward compatibility
but is much more consistent along all these fronts.

> One way to deal with this is to change the interface to something like:
> template <class Model, class SampleRecorder, class DiagnosticRecorder,
> class MessageRecorder, class InterruptHandler, ...>
> int command(std::vector<argument*> arguments)

How about something like this:

template <class Model,
class SampleWriter,
class DiagnosticWriter,
class MessageWriter,
class InterruptHandler, ...>

int command(const CmdConfig& config,
const Model& model,
SampleWriter& sampleRecorder,
...);

The classes won't be enough unless they can all be constructued
based on the config. And this way the caller won't need to
specify the template args.

I very much like the idea of there being a config object
that the interfaces just populate. If the mapping can be
general, then that's even better.

The only reasonable pattern for something general and
nested for config is the following

Simple: properties objects map string keys to values,
where a value is either a string or another property
object

Sequence Values: Add sequence values

Numeric Values: Add numeric values.

With sequence and numeric values, we basically get something that
could be coded as strings in JSON and would have a direct analogue
as an R list or Python dictionary. Then we can share the doc, with
only the method of constructing the nested object changing. It's
not super idiomatic for R, but I like it --- we're breaking the regular
command-line idioms for the commnad-line config. I don't know about Python's
idioms.

Defaults and Inheritance: Java's Properties class implementation maintains
a pointer to a parent properties class (which may be null). Keys are
first looked up in the local implementation, but if the key doesn't exist,
the call is delegated to the parent class. The whole thing can be flattened
for transport.

I'm not sure the best way to nest this in C++. A general
approach would be to use the variant structures from Boost that
are used in the abstract syntax tree for the parser, which has
the same problem in that right-hand sides can be one of many
things and you want to act differently for each.

Consistency: if we can manage purely compositional consistency,
that would be awesome. doc, testing and maintenance problems arise
as soon as values or existence of one key can depend on another (e.g.,
model string or model file in current setup).

- Bob

>
> Each interface would be responsible for implementing the additional template parameters including the recorders and the "..." (tbd).
> This function would then run based on what's passed in as arguments. It would be on the interfaces to translate to this argument structure.
> This would allow interface designers to focus on implementing the different recorders, handlers, etc. and translate from the interface to an argument structure. This should stay backwards compatible always, so interface implementers can be behind Stan without penalty and being forced to change interface code with every new version.
>
>
> Michael, was the original intent of the argument structure to be used as configuration?
> As it currently stands
> - there are no tests
> - not sure how easy it is to validate configurations
> - how would we handle changes to configuration over time?
> - what helper methods are available for constructing a valid configuration?
>
> For everyone else, anyone deal with configuration structures and have a pattern that they know about, is easy to maintain and implement, and will serve the purpose of capturing and communicating the configuration? Or... any thoughts on how to shape the arguments into something really easy to use cross-interface?
>
> I think the stuff in src/stan/gm/arguments/ is almost there, but as it stands now, it needs a little bit more work to make it really usable. Maybe a combination of doc, tests, and factories might be enough? I'd love an extra set of eyes on the code.
>
>
>
> Daniel
>
>
>

[Allen Riddell]

Hi Daniel and Bob,

This looks fine from the Python side of things; Rcpp and Cython do pretty much
the same thing.

That said, I'd urge erring on side of simplicity and C idioms for maximum
compatibility with (future) wrapping libraries. For example, using a struct or
an enum rather than a class when it's possible seems like a good idea.

Best,

-a

[Daniel Lee]

Thanks, Bob. I'll respond below. First, I wanted to get back to Michael's email.

On Tue, Jun 24, 2014 at 3:47 AM, Michael Betancourt <betan...@gmail.com> wrote:

As it currently stands

- there are no tests

- not sure how easy it is to validate configurations

- how would we handle changes to configuration over time?

- what helper methods are available for constructing a valid configuration?

I think you missed https://github.com/stan-dev/stan/tree/develop/src/test/integration/gm/arguments.

Yes, I completely forgot about those tests. But, they don't fix configuration.

I realized that these tests don't fix behavior and prevent others from changing behavior. I came to that conclusion from Marcus's lbfgs branch where he went through and added new argument items as well as changed existing configuration items. The addition of new items doesn't bother me, it's the change to the existing items that weren't caught that scares me a bit. If this is what interface writers are going to use as the interface into Stan, we need to make sure it doesn't change without forcing the developer changing it to be aware that it has effects beyond the class.

In particular, argument_parser_test builds up all possible combinations of valid/invalid arguments

dynamically and tests that each yields a valid/invalid configuration.  Valid/invalid values are

defined by each argument and the configurations are built up from the recursive dependencies

of each argument so that nothing has to be hand-coded.

As I was saying before, this sort of testing is more circular than I want it. Maybe it shouldn't be hand-coded, but perhaps we can serialize the current valid configurations to disk now and really force someone changing the spec to be conscious and think through the changes made to this spec.

The alternative is to have each interface define some intermediate object, say a configuration string matching what we input to argv in CmdStan and let the parser handle the unpacking.

I would not like to use a string as the configuration. I think that's really hard to maintain.

On Tue, Jun 24, 2014 at 6:06 AM, Bob Carpenter <ca...@alias-i.com> wrote:

Turns out I do have a lot of thoughts on this.  More below.

On Jun 24, 2014, at 6:05 AM, Daniel Lee <bea...@alum.mit.edu> wrote:

> I want to refactor the function:

>     template <class Model>
>     int command(int argc, const char* argv[])
>
> It is currently 769 lines and RStan pretty much reimplements the whole thing to run most of the functions. (I haven't looked at Allen's code, but my guess is PyStan also reimplements all of it.)

That would be great.  I thought that was where you
were heading when you dove into R in the first place.

Yes, that's where I was heading, but was only able to knock out about 200 lines safely in the first pass.

 

This is also why I think the design for the current R
interface proposal from Andrew and Ben is so important.
We're thinking a 3.0 version that breaks backward compatibility
but is much more consistent along all these fronts.

> One way to deal with this is to change the interface to something like:
>     template <class Model, class SampleRecorder, class DiagnosticRecorder,
>                      class MessageRecorder, class InterruptHandler, ...>
>     int command(std::vector<argument*> arguments)

How about something like this:

template <class Model,
          class SampleWriter,
          class DiagnosticWriter,
          class MessageWriter,
          class InterruptHandler, ...>

    int command(const CmdConfig& config,
                const Model& model,
                SampleWriter& sampleRecorder,
                ...);

The classes won't be enough unless they can all be constructued
based on the config.  And this way the caller won't need to
specify the template args.

Absolutely. That was an oversight on my part.

 

I very much like the idea of there being a config object
that the interfaces just populate.  If the mapping can be
general, then that's even better.

What do you mean by "mapping" and "general"?

 

 

The only reasonable pattern for something general and
nested for config is the following

Simple:  properties objects map string keys to values,
where a value is either a string or another property
object

Sequence Values:  Add sequence values

Numeric Values:  Add numeric values.

With sequence and numeric values, we basically get something that
could be coded as strings in JSON and would have a direct analogue
as an R list or Python dictionary.  Then we can share the doc, with
only the method of constructing the nested object changing.  It's
not super idiomatic for R, but I like it --- we're breaking the regular
command-line idioms for the commnad-line config.  I don't know about Python's
idioms.

I think that's what Micheal's argument config is actually accomplishing, but with validation and nesting and really allowing a subset to be instantiated. I think it's hard to instantiate and manipulate, hence the thought of adding additional factory functions to help that process along. If it were easy to use and had doc, I'd be perfectly happy with it and wouldn't be bringing this point up.

Could we create something general like Bob's suggested with simple, sequence, and numeric values? Then add validation rules on top of it? And have an easy way to generate valid configurations?

 

Defaults and Inheritance:  Java's Properties class implementation maintains
a pointer to a parent properties class (which may be null).  Keys are
first looked up in the local implementation, but if the key doesn't exist,
the call is delegated to the parent class.  The whole thing can be flattened
for transport.

I'm not sure the best way to nest this in C++.  A general
approach would be to use the variant structures from Boost that
are used in the abstract syntax tree for the parser, which has
the same problem in that right-hand sides can be one of many
things and you want to act differently for each.

Micheal's implemented it using pointers.

 

Consistency:  if we can manage purely compositional consistency,
that would be awesome.  doc, testing and maintenance problems arise
as soon as values or existence of one key can depend on another (e.g.,
model string or model file in current setup).

From the command line, I think the only place I recall that has this sort of inconsistency is the init argument which could take a string or 0.

Just cause it might not be clear, I'm not in favor redo-ing work. If we can get the current argument infrastructure easy enough to get right from other interfaces other than the command line, I'd be happy.

[Daniel Lee]

Hi Allen,

Thanks for the reminder! If we can get a C interface out, I think we could dance around Rcpp and Julia integration would be easy.

Daniel

[Bob Carpenter]

On Jun 24, 2014, at 6:02 PM, Allen Riddell <a...@ariddell.org> wrote:

> Hi Daniel and Bob,
>
> This looks fine from the Python side of things; Rcpp and Cython do pretty much
> the same thing.
>
> That said, I'd urge erring on side of simplicity and C idioms for maximum
> compatibility with (future) wrapping libraries. For example, using a struct or
> an enum rather than a class when it's possible seems like a good idea.

A C++ struct is just a class where everything's public.
Is there a narrower notion of struct we can invoke somehow?
You mean something without methods? Or without virtual methods?

Enums are OK for finite sets of values, but most of our
config is nested in that you specify an enum-like decision
(e.g., HMC or NUTS, or sample vs. optimize), and then that determines
a further set of params.

So I'm not exactly sure what you mean by simple when we need
nested config, some of which takes values other than enums.

Is there something specific we need to avoid on the C++
side?

- Bob

[Bob Carpenter]

On Jun 24, 2014, at 6:29 PM, Daniel Lee <bea...@alum.mit.edu> wrote:

> Hi Allen,
>
> Thanks for the reminder! If we can get a C interface out, I think we could dance around Rcpp and Julia integration would be easy.

I think it'd be great to get rid of the Rcpp dependency,
as it seems very brittle.

What would a C interface look like from the R side
and how would we have to restrict our output?

Hadley has a page up about it:

http://adv-r.had.co.nz/C-interface.html#c-data-structures

It looks really ugly. I see some other wrappers, but those
look like add-on packages like Rcpp, so we'd have to make sure
we're not just jumping out of the frying pan and into the fire.

- Bob

[Bob Carpenter]

On Jun 24, 2014, at 6:28 PM, Daniel Lee <bea...@alum.mit.edu> wrote:
>
> As I was saying before, this sort of testing is more circular than I want it. Maybe it shouldn't be hand-coded, but perhaps we can serialize the current valid configurations to disk now and really force someone changing the spec to be conscious and think through the changes made to this spec.

It seems to me that one of two things will happen.

1. It's easy to update the tests, so they won't test
that things stay fixed.

2. It's hard to update the tests, so the interfaces will
never get changed.

They will make people take a conscious step to change the interfaces,
but presumably they're doing that when they change the interfaces
anyway.

So we'd need to have something that tests current behavior, but
isn't just auto-generated.

To the extent we can make dependencies modular, we can test
those.

> The alternative is to have each interface define some intermediate object, say a configuration string matching what we input to argv in CmdStan and let the parser handle the unpacking.
>
> I would not like to use a string as the configuration. I think that's really hard to maintain.

Why? Seems like the easiest way to do things to me.
For instance, if we have something like JSON, that seems
more portable than passing objects around through Rcpp
or other interfaces. Not that I think it's the right thing
to do, just that it seems easiest.

- Bob

[Daniel Lee]

On Tue, Jun 24, 2014 at 1:14 PM, Bob Carpenter <ca...@alias-i.com> wrote:

On Jun 24, 2014, at 6:29 PM, Daniel Lee <bea...@alum.mit.edu> wrote:

> Hi Allen,
>
> Thanks for the reminder! If we can get a C interface out, I think we could dance around Rcpp and Julia integration would be easy.

I think it'd be great to get rid of the Rcpp dependency,
as it seems very brittle.

What would a C interface look like from the R side
and how would we have to restrict our output?

Not sure yet, but I think we'd have to write a clear C interface.

From the RStan side, in C code, we'd have adaptors that wrap the "clear C interface" and use all these SEXP, VECSXP, etc.

 

Hadley has a page up about it:

  http://adv-r.had.co.nz/C-interface.html#c-data-structures

It looks really ugly.  I see some other wrappers, but those
look like add-on packages like Rcpp, so we'd have to make sure
we're not just jumping out of the frying pan and into the fire.

Yeah, good point. Although, if we get a C compatible interface, in addition to a C++ interface, I think it would make it easier to integrate with something like Julia and probably SAS. =P.

[Bob Carpenter]

On Jun 24, 2014, at 6:28 PM, Daniel Lee <bea...@alum.mit.edu> wrote:

> On Tue, Jun 24, 2014 at 6:06 AM, Bob Carpenter <ca...@alias-i.com> wrote:
> Turns out I do have a lot of thoughts on this. More below.
>
> On Jun 24, 2014, at 6:05 AM, Daniel Lee <bea...@alum.mit.edu> wrote:
>

> I very much like the idea of there being a config object
> that the interfaces just populate. If the mapping can be
> general, then that's even better.

I mean if there's a programmatic way to do it, as in the way
Michael currently put things together.

>
> What do you mean by "mapping" and "general"?

By "mapping", I just meant you move among

Stan Config <--> Python Dictionary
<--> R List
<--> JSON serialization

By "general", I mean that once the config object's defined,
then so is everything else. So we can write the <---> functions
exactly once.

> The only reasonable pattern for something general and
> nested for config is the following
>
> Simple: properties objects map string keys to values,
> where a value is either a string or another property
> object
>
> Sequence Values: Add sequence values
>
> Numeric Values: Add numeric values.
>
> With sequence and numeric values, we basically get something that
> could be coded as strings in JSON and would have a direct analogue
> as an R list or Python dictionary.

...

> I'm not sure the best way to nest this in C++. A general
> approach would be to use the variant structures from Boost that
> are used in the abstract syntax tree for the parser, which has
> the same problem in that right-hand sides can be one of many
> things and you want to act differently for each.
>
> Micheal's implemented it using pointers.

That's highly sub-optimal from a type safety point of view.
But it's essentially what Java does. The Boost variant
structures are more types safe (you can't just drop in any old
pointer), but they're a pain to use.

- Bob

[Daniel Lee]

On Tue, Jun 24, 2014 at 1:23 PM, Bob Carpenter <ca...@alias-i.com> wrote:

On Jun 24, 2014, at 6:28 PM, Daniel Lee <bea...@alum.mit.edu> wrote:
>
> As I was saying before, this sort of testing is more circular than I want it. Maybe it shouldn't be hand-coded, but perhaps we can serialize the current valid configurations to disk now and really force someone changing the spec to be conscious and think through the changes made to this spec.

It seems to me that one of two things will happen.

1.  It's easy to update the tests, so they won't test
that things stay fixed.

2.  It's hard to update the tests, so the interfaces will
never get changed.

They will make people take a conscious step to change the interfaces,
but presumably they're doing that when they change the interfaces
anyway.

So we'd need to have something that tests current behavior, but
isn't just auto-generated.

Yeah, I'm out of ideas. Not sure where we should be between 1 & 2.

I don't presume changes to the interface are conscious now.

 

To the extent we can make dependencies modular, we can test
those.

> The alternative is to have each interface define some intermediate object, say a configuration string matching what we input to argv in CmdStan and let the parser handle the unpacking.
>
> I would not like to use a string as the configuration. I think that's really hard to maintain.

Why?  Seems like the easiest way to do things to me.
For instance, if we have something like JSON, that seems
more portable than passing objects around through Rcpp
or other interfaces.  Not that I think it's the right thing
to do, just that it seems easiest.

Sorry, I really said the wrong thing. I would not like to use CmdStan's command line invocation as the string configuration. It feels a little too non-standard for me to be touting that as our public interface. But I'm happy to be out-voted here.

 

[Bob Carpenter]

On Jun 24, 2014, at 7:32 PM, Daniel Lee <bea...@alum.mit.edu> wrote:

> On Tue, Jun 24, 2014 at 1:23 PM, Bob Carpenter <ca...@alias-i.com> wrote:
>
> On Jun 24, 2014, at 6:28 PM, Daniel Lee <bea...@alum.mit.edu> wrote:
> >
> > As I was saying before, this sort of testing is more circular than I want it. Maybe it shouldn't be hand-coded, but perhaps we can serialize the current valid configurations to disk now and really force someone changing the spec to be conscious and think through the changes made to this spec.
>
> It seems to me that one of two things will happen.
>
> 1. It's easy to update the tests, so they won't test
> that things stay fixed.
>
> 2. It's hard to update the tests, so the interfaces will
> never get changed.
>
> They will make people take a conscious step to change the interfaces,
> but presumably they're doing that when they change the interfaces
> anyway.
>
> So we'd need to have something that tests current behavior, but
> isn't just auto-generated.
>
> Yeah, I'm out of ideas. Not sure where we should be between 1 & 2.
>
> I don't presume changes to the interface are conscious now.

Zombie programmers? I presume at least Marcus intended
to change the interface to include L-BFGS options. If other
things changed, they need to be documented and tested, as well,
of course.

>
> To the extent we can make dependencies modular, we can test
> those.
>
> > The alternative is to have each interface define some intermediate object, say a configuration string matching what we input to argv in CmdStan and let the parser handle the unpacking.
> >
> > I would not like to use a string as the configuration. I think that's really hard to maintain.
>
> Why? Seems like the easiest way to do things to me.
> For instance, if we have something like JSON, that seems
> more portable than passing objects around through Rcpp
> or other interfaces. Not that I think it's the right thing
> to do, just that it seems easiest.
>
> Sorry, I really said the wrong thing. I would not like to use CmdStan's command line invocation as the string configuration. It feels a little too non-standard for me to be touting that as our public interface. But I'm happy to be out-voted here.

Agreed. I think I'd rather go with something like JSON
that has a more conventional structure. I was the one
who was strongly opposed to the new interface from the get
go, and I'm still on the fence about it. It works better
than I expected in practice, but it's still highly non-standard
and hence a pain to doc. The problem is that a more standard
interface has all the flat dependency issues which Michael neatly
solved with his approach, which is why I'm on the fence rather
than way over it.

- Bob

[Daniel Lee]

On Tue, Jun 24, 2014 at 1:36 PM, Bob Carpenter <ca...@alias-i.com> wrote:

On Jun 24, 2014, at 7:32 PM, Daniel Lee <bea...@alum.mit.edu> wrote:

> On Tue, Jun 24, 2014 at 1:23 PM, Bob Carpenter <ca...@alias-i.com> wrote:
>
> On Jun 24, 2014, at 6:28 PM, Daniel Lee <bea...@alum.mit.edu> wrote:
> >
> > As I was saying before, this sort of testing is more circular than I want it. Maybe it shouldn't be hand-coded, but perhaps we can serialize the current valid configurations to disk now and really force someone changing the spec to be conscious and think through the changes made to this spec.
>
> It seems to me that one of two things will happen.
>
> 1.  It's easy to update the tests, so they won't test
> that things stay fixed.
>
> 2.  It's hard to update the tests, so the interfaces will
> never get changed.
>
> They will make people take a conscious step to change the interfaces,
> but presumably they're doing that when they change the interfaces
> anyway.
>
> So we'd need to have something that tests current behavior, but
> isn't just auto-generated.
>
> Yeah, I'm out of ideas. Not sure where we should be between 1 & 2.
>
> I don't presume changes to the interface are conscious now.

Zombie programmers?  I presume at least Marcus intended
to change the interface to include L-BFGS options.   If other
things changed, they need to be documented and tested, as well,
of course.

Yes, zombie programmers. I turn into a zombie sometimes, especially when I'm refactoring. Sometimes I don't realize that my change actually has an effect until I run the tests and see the failure. So, I'm using my experience here as a guide.

I believe Marcus intended to change the interface to include the L-BFGS options. But I can't tell if the change from "> 0" to ">= 0" in arg_tolerance or changes in arg_bfgs taking tol_obj from 1e-8 to 1e-12 was intended. Just not sure what to make of it just based on the changed code.

 

>
> To the extent we can make dependencies modular, we can test
> those.
>
> > The alternative is to have each interface define some intermediate object, say a configuration string matching what we input to argv in CmdStan and let the parser handle the unpacking.
> >
> > I would not like to use a string as the configuration. I think that's really hard to maintain.
>
> Why?  Seems like the easiest way to do things to me.
> For instance, if we have something like JSON, that seems
> more portable than passing objects around through Rcpp
> or other interfaces.  Not that I think it's the right thing
> to do, just that it seems easiest.
>
> Sorry, I really said the wrong thing. I would not like to use CmdStan's command line invocation as the string configuration. It feels a little too non-standard for me to be touting that as our public interface. But I'm happy to be out-voted here.

Agreed.  I think I'd rather go with something like JSON
that has a more conventional structure.  I was the one
who was strongly opposed to the new interface from the get
go, and I'm still on the fence about it.  It works better
than I expected in practice, but it's still highly non-standard
and hence a pain to doc.  The problem is that a more standard
interface has all the flat dependency issues which Michael neatly
solved with his approach, which is why I'm on the fence rather
than way over it.

- Bob

[Michael Betancourt]

This really sounds like a breakdown of our development process.  Reviewers should be asking these

questions directly to the submitters and agreeing before checking off for the merge.  I think having

tools to help reviewers identify changes can definitely be useful, but I would be hesitant to try to 

bootstrap them into the tests,.

[Daniel Lee]

On Tue, Jun 24, 2014 at 2:03 PM, Michael Betancourt <betan...@gmail.com> wrote:

This really sounds like a breakdown of our development process.  Reviewers should be asking these

questions directly to the submitters and agreeing before checking off for the merge.  

Well... it did happen.... well, not quite. It was dwarfed by more substantial changes.

And while we could just push back on the submitter, we actually want this in at some point, so we're trying to work with Marcus instead of just leaving it to him to fix.

 

I think having tools to help reviewers identify changes can definitely be useful, but I would be hesitant to try to 

bootstrap them into the tests,.

I just see tests as a way of informing someone wanting to change it some unspecified amount of time in the future. The actual pull request, although not lost in theory, will be lost in practice. The tests would exist.

But yeah, if we could change our process to something more usable, I'd be happy with that.

 

On Jun 24, 2014, at 6:44 PM, Daniel Lee <bea...@alum.mit.edu> wrote:

I believe Marcus intended to change the interface to include the L-BFGS options. But I can't tell if the change from "> 0" to ">= 0" in arg_tolerance or changes in arg_bfgs taking tol_obj from 1e-8 to 1e-12 was intended. Just not sure what to make of it just based on the changed code.

 

[Allen Riddell]

Right -- if it's not possible, it's not possible.

> Is there something specific we need to avoid on the C++
> side?

Nope.

[Michael Betancourt]

On Jun 24, 2014, at 7:10 PM, Daniel Lee <bea...@alum.mit.edu> wrote:

On Tue, Jun 24, 2014 at 2:03 PM, Michael Betancourt <betan...@gmail.com> wrote:

This really sounds like a breakdown of our development process.  Reviewers should be asking these

questions directly to the submitters and agreeing before checking off for the merge.  

Well... it did happen.... well, not quite. It was dwarfed by more substantial changes.

And while we could just push back on the submitter, we actually want this in at some point, so we're trying to work with Marcus instead of just leaving it to him to fix.

But that’s exactly how you—don’t— want to do it!  A pull request is a discussion, and if there are questions the submitter can answer them.  Having a test in place won’t answer anything down the road either.

I just see tests as a way of informing someone wanting to change it some unspecified amount of time in the future. The actual pull request, although not lost in theory, will be lost in practice. The tests would exist.

But what would they say?  That something changed — we know that already from the diff.  And the test

doesn’t make it clear why it should not be changed.  Tests can validate spec but that are a bad replacement

for doc.

[Daniel Lee]

Agreed. But from the pragmatic side, I'd take a working test over working doc.

[Daniel Lee]

Anyway, I think the point is, we need to enforce things manually before they go in, which is happening.

It just sucks that it's taking so long for things to get in. A minor part of that is testing, but the root cause is that everyone writes code that isn't clear to some level.

[Allen Riddell]

A C interface would be wonderful. I'd be happy to help out with this.

Best, -a

On 06/24, Daniel Lee wrote: