big all-knowing objects

[Bob Carpenter]

See:

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

- Bob

[Ben Goodrich]

I understand what you are saying, but to some extent, it is necessary to preserve the existing stan() behavior and the make-like behavior Andrew wants. You can have the stanfit object that comes out of stan() consist of a stanmodel object, a config object, a data object, a coda-like object for the samples, vectors of names, etc.  But stan() can't do all the make-like behavior and just output the coda-like object because the all the other objects would be local to stan().

I don't think the omniscience of a God object is a big problem, but omnipotence could potentially be. So, you need to lock down the parts that should be changed, validate whatever might be mutable, flush output that was specific to the previous run, etc.

Ben

[Bob Carpenter]

On Aug 14, 2014, at 5:50 PM, Ben Goodrich <goodri...@gmail.com> wrote:

> On Thursday, August 14, 2014 4:12:10 PM UTC-4, Bob Carpenter wrote:
> See:
>
> http://en.wikipedia.org/wiki/God_object
>
> - Bob
>
> I understand what you are saying, but to some extent, it is necessary to preserve the existing stan() behavior and the make-like behavior Andrew wants.

> You can have the stanfit object that comes out of stan() consist of a stanmodel object, a config object, a data object, a coda-like object for the samples, vectors of names, etc. But stan() can't do all the make-like behavior and just output the coda-like object because the all the other objects would be local to stan().

I thought the only "make-like" behavior on the table was not
recompiling models if there's already a shared object/dll lying
around. So I'm not seeing where a single big object through which
every operation is carried out is needed in the design.

> I don't think the omniscience of a God object is a big problem, but omnipotence could potentially be. So, you need to lock down the parts that should be changed, validate whatever might be mutable, flush output that was specific to the previous run, etc.

The omniscience is a problem. If the big object returned by stan()
needs to know about the insides of the components it holds onto
and provide methods itself to configure them and reuse them, I think
it's going to be a mess to document and maintain.

- Bob

[Ben Goodrich]

On Friday, August 15, 2014 12:04:59 AM UTC-4, Bob Carpenter wrote:

On Aug 14, 2014, at 5:50 PM, Ben Goodrich <goodri...@gmail.com> wrote:
> On Thursday, August 14, 2014 4:12:10 PM UTC-4, Bob Carpenter wrote:
> See:
>
> http://en.wikipedia.org/wiki/God_object
>
> - Bob
>
> I understand what you are saying, but to some extent, it is necessary to preserve the existing stan() behavior and the make-like behavior Andrew wants.

> You can have the stanfit object that comes out of stan() consist of a stanmodel object, a config object, a data object, a coda-like object for the samples, vectors of names, etc.  But stan() can't do all the make-like behavior and just output the coda-like object because the all the other objects would be local to stan().

I thought the only "make-like" behavior on the table was not
recompiling models if there's already a shared object/dll lying
around.    So I'm not seeing where a single big object through which
every operation is carried out is needed in the design.  

Just having a stan() function that inputs a path, config, and data means those little objects would have to be instantiated and if they are not returned along with the coda-like object as part of a bigger object, then they get garbage collected.
 

> I don't think the omniscience of a God object is a big problem, but omnipotence could potentially be. So, you need to lock down the parts that should be changed, validate whatever might be mutable, flush output that was specific to the previous run, etc.

The omniscience is a problem.  If the big object returned by stan()
needs to know about the insides of the components it holds onto
and provide methods itself to configure them and reuse them, I think
it's going to be a mess to document and maintain.

It isn't going to be much worse that it is now. As Allan pointed out, log_prob and gradient should really be methods for a stanmodel not a stanfit. But if we define log_prob and gradient methods for a stanfit that don't do anything but call the log_prob and gradient methods for a stanmodel, then we just have to change things in one place. For documentation, I think the format of R documentation for S4 classes and methods is basically worthless, so being able to document reference class methods Python-style is a lot easier for the developer and a lot more useful for the user.

Ben

[Bob Carpenter]

On Aug 15, 2014, at 10:12 AM, Ben Goodrich <goodri...@gmail.com> wrote:

> On Friday, August 15, 2014 12:04:59 AM UTC-4, Bob Carpenter wrote:
> On Aug 14, 2014, at 5:50 PM, Ben Goodrich <goodri...@gmail.com> wrote:
> > On Thursday, August 14, 2014 4:12:10 PM UTC-4, Bob Carpenter wrote:
> > See:
> >
> > http://en.wikipedia.org/wiki/God_object
> >
> > - Bob
> >
> > I understand what you are saying, but to some extent, it is necessary to preserve the existing stan() behavior and the make-like behavior Andrew wants.
>
> > You can have the stanfit object that comes out of stan() consist of a stanmodel object, a config object, a data object, a coda-like object for the samples, vectors of names, etc. But stan() can't do all the make-like behavior and just output the coda-like object because the all the other objects would be local to stan().
>
> I thought the only "make-like" behavior on the table was not
> recompiling models if there's already a shared object/dll lying
> around. So I'm not seeing where a single big object through which
> every operation is carried out is needed in the design.
>
> Just having a stan() function that inputs a path, config, and data means those little objects would have to be instantiated and if they are not returned along with the coda-like object as part of a bigger object, then they get garbage collected.

I don't see any reason to save those things in memory.

There is the issue of which compiled models we save and which
we toss --- we don't want to clutter up someone's file system
every time they do a Stan run.

> > I don't think the omniscience of a God object is a big problem, but omnipotence could potentially be. So, you need to lock down the parts that should be changed, validate whatever might be mutable, flush output that was specific to the previous run, etc.
>
> The omniscience is a problem. If the big object returned by stan()
> needs to know about the insides of the components it holds onto
> and provide methods itself to configure them and reuse them, I think
> it's going to be a mess to document and maintain.
>
> It isn't going to be much worse that it is now. As Allan pointed out, log_prob and gradient should really be methods for a stanmodel not a stanfit.

I completely agree.

> But if we define log_prob and gradient methods for a stanfit that don't do anything but call the log_prob and gradient methods for a stanmodel, then we just have to change things in one place.

We don't need or want that higher-level method. If the stanfit just has a handle on the
stanmodel, then we don't have to write two functions, two sets of doc, two sets of tests,
or change things anywhere in the container when we change the stanmodel object.

> For documentation, I think the format of R documentation for S4 classes and methods is basically worthless,

You won't get any argument there.

> so being able to document reference class methods Python-style is a lot easier for the developer and a lot more useful for the user.

I think reference classes are the way to go --- I'd assumed that's
what S4 did.

For doc, I think it makes sense to have objects with methods (e.g., stanmodel
with log prob, gradient and Hessian methods), then document those methods on
the object. I'm not familiar with what you mean by Python style, but that's
what Javadoc (built in) does for Java and doxygen does for C++.

- Bob

[Ben Goodrich]

On Friday, August 15, 2014 11:08:54 AM UTC-4, Bob Carpenter wrote:

> Just having a stan() function that inputs a path, config, and data means those little objects would have to be instantiated and if they are not returned along with the coda-like object as part of a bigger object, then they get garbage collected.

I don't see any reason to save those things in memory.

Suppose the user does

posterior <- stan(...)

and now the user wants to evaluate the log-probability at a point. At the moment, they can do

log_prob(posterior, theta_unconstrained)

but that "works" because posterior object has slots for the data and the stanmodel. Similarly, if you want to load posterior off the disk and query what config was used to generate it, then the config needs to be part of it.
 

> But if we define log_prob and gradient methods for a stanfit that don't do anything but call the log_prob and gradient methods for a stanmodel, then we just have to change things in one place.

We don't need or want that higher-level method.  If the stanfit just has a handle on the
stanmodel, then we don't have to write two functions, two sets of doc, two sets of tests,
or change things anywhere in the container when we change the stanmodel object.

We don't really want it but we do need it. I could see having

log_prob(posterior, theta_unconstrained)

throw an error message saying that you need to instead call
 

log_prob(posterior$get_stanmodel(), theta_unconstrained)

but that is not how R users expect to interact with objects.

It is 1.001 functions where the .001 function is the first calling the second. It is also 1.001 instances of Python-style documentation where posterior$help("log_prob") calls posterior$get_stanmodel()$help("log_prob"). It is two sets of tests but doesn't require two sets of changes to the code when the internals of stanmodel change.  

For doc, I think it makes sense to have objects with methods (e.g., stanmodel
with log prob, gradient and Hessian methods), then document those methods on
the object.  I'm not familiar with what you mean by Python style, but that's
what Javadoc (built in) does for Java and doxygen does for C++.

From http://stat.ethz.ch/R-manual/R-devel/library/methods/html/refClass.html

Documentation for the methods can be obtained by the $help method for the generator object. Methods for classes are not documented in the Rd format used for R functions. Instead, the $help method prints the calling sequence of the method, followed by self-documentation from the method definition, in the style of Python. If the first element of the body of the method is a literal character string (possibly multi-line), that string is interpreted as documentation. See the method definitions in the example.

Ben

[Bob Carpenter]

On Aug 15, 2014, at 11:35 AM, Ben Goodrich <goodri...@gmail.com> wrote:

> On Friday, August 15, 2014 11:08:54 AM UTC-4, Bob Carpenter wrote:
> > Just having a stan() function that inputs a path, config, and data means those little objects would have to be instantiated and if they are not returned along with the coda-like object as part of a bigger object, then they get garbage collected.
>
> I don't see any reason to save those things in memory.
>
> Suppose the user does
>
> posterior <- stan(...)
>
> and now the user wants to evaluate the log-probability at a point. At the moment, they can do
>
> log_prob(posterior, theta_unconstrained)
>
> but that "works" because posterior object has slots for the data and the stanmodel. Similarly, if you want to load posterior off the disk and query what config was used to generate it, then the config needs to be part of it.

I'm not saying don't have the fit object that consolidates things,
just don't tie functions into it. So I'd prefer what you use later.

> > But if we define log_prob and gradient methods for a stanfit that don't do anything but call the log_prob and gradient methods for a stanmodel, then we just have to change things in one place.
>
> We don't need or want that higher-level method. If the stanfit just has a handle on the
> stanmodel, then we don't have to write two functions, two sets of doc, two sets of tests,
> or change things anywhere in the container when we change the stanmodel object.
>
> We don't really want it but we do need it. I could see having
>
> log_prob(posterior, theta_unconstrained)
>
> throw an error message saying that you need to instead call

No! That also exposes the functionality higher up.

>
> log_prob(posterior$get_stanmodel(), theta_unconstrained)
>
> but that is not how R users expect to interact with objects.

It should be! Is there no way to use a syntax like:

posterior$model()$log_prob(theta_unconstrained)

This is the whole point of having OO constructs in the first place.
It encapsulates knowledge on a need-to-know basis to decouple
classes from each other to make them easy to doc and test in
a modular fashion.

> It is 1.001 functions where the .001 function is the first calling the second.

I'm not familiar with this notation.

> It is also 1.001 instances of Python-style documentation where posterior$help("log_prob") calls posterior$get_stanmodel()$help("log_prob"). It is two sets of tests but doesn't require two sets of changes to the code when the internals of stanmodel change.

I think it does. Every change in stanmodel needs to be reflected
in the higher-order object's doc and tests.

> For doc, I think it makes sense to have objects with methods (e.g., stanmodel
> with log prob, gradient and Hessian methods), then document those methods on
> the object. I'm not familiar with what you mean by Python style, but that's
> what Javadoc (built in) does for Java and doxygen does for C++.
>
> From http://stat.ethz.ch/R-manual/R-devel/library/methods/html/refClass.html
>
> Documentation for the methods can be obtained by the $help method for the generator object. Methods for classes are not documented in the Rd format used for R functions. Instead, the $help method prints the calling sequence of the method, followed by self-documentation from the method definition, in the style of Python. If the first element of the body of the method is a literal character string (possibly multi-line), that string is interpreted as documentation. See the method definitions in the example.

OK. Thanks for the clarification. I'm totally OK with this kind of doc.
It's what Javadoc and doxygen do, but they're, perhaps not surprisingly,
a little more structured in how they approach it (there's a markup language
for function doc).

- Bob

[Ben Goodrich]

On Friday, August 15, 2014 11:45:25 AM UTC-4, Bob Carpenter wrote:

> log_prob(posterior$get_stanmodel(), theta_unconstrained)
>
> but that is not how R users expect to interact with objects.

It should be!   Is there no way to use a syntax like:

  posterior$model()$log_prob(theta_unconstrained)

This is the whole point of having OO constructs in the first place.
It encapsulates knowledge on a need-to-know basis to decouple
classes from each other to make them easy to doc and test in
a modular fashion.

Something like that syntax would be possible, but Andrew hates it and it is non-traditional for a user-facing function. You could define a high-level (non-method) function

log_prob(stanfit, theta_unconstrained)

that just calls

stanfit$model()$log_prob(theta_unconstrained)

but then you have to document the (non-method) log_prob function in the Rd format. If you believe, as I do, that Rd format has negative value for S4 classes and methods, then that is a problem.

I think OO in the R case is mostly useful for inheritance, ease of calling from C++, and to avoid Rd documentation.

> It is 1.001 functions where the .001 function is the first calling the second.

I'm not familiar with this notation.

I mean it is 2 functions that only takes slightly more effort than 1 function. If you define

stanfit$log_prob(theta_unconstrained)

as

stanfit$model()$log_prob(theta_unconstrained)

and define

stanfit$help("log_prob")

as

stanfit$model()$help("log_prob")

then the cost is basically just having to write and execute more unit tests for the stanfit class that are basically just a "copy, paste, find, and replace" of your unit tests for the stanmodel class.

 

> It is also 1.001 instances of Python-style documentation where posterior$help("log_prob") calls posterior$get_stanmodel()$help("log_prob"). It is two sets of tests but doesn't require two sets of changes to the code when the internals of stanmodel change.

I think it does.  Every change in stanmodel needs to be reflected
in the higher-order object's doc and tests.  

Only if you change the argument(s) that stanmodel$log_prob() takes. Most methods take zero or one arguments, so that is not a major concern.

Ben