factoring RStan, PyStan, etc.

[Bob Carpenter]

The context of this is that we were discussing adding structured
mass matrices and I pointed out that for every change to
command-line Stan, there needs to be a corresponding reimplementation
in RStan and PyStan.

On 11/30/13, 12:00 PM, Michael Betancourt wrote:
> Shouldn't the interfaces be factored from these details?

Yes. As things stand now, anything in stan/gm/command.hpp
needs to be reimplemented in RStan and PyStan. So any
factorization of the code that will minimize the amount of
code that's directly implemented in the command-line code
would be a big help.

Also, anything with std::cout, std::cin, or std::cerr
hardcoded needs to be rewritten for RStan and PyStan.

Another option I've been considering is that we just have
RStan and PyStan call the command function in Stan directly.
There are two ways this could work:

Option 1. RStan and PyStan would:

* write data and inits to a file,
* convert config for the call into the appropriate format,
* call a new process to run models that writes output into files, and
* read the output back in.

This is what I did originally in the prototype, and then for
reasons that are now hazy to me, we switched gears to do a deeper
in-memory integration. In retrospect, I think this was a mistake.

Option 2: RStan and PyStan would call the command directly,
configuring it with the appropriate input, output and error
streams. If we do this, we'd need to be much more careful about
clean resource release (files and memory for now), because
R and Python are persistent processes, so it's very bad form
for a function they call to leak resources.

Option 2 is what we were aiming for with RStan, but work on
the command-line version of Stan has never complied. One of the
issues I remember is that holding onto C++ structures and making
them persistent, which isn't easy in R through Rcpp. That's
why the equivalent of the command-line bin/print is also completely
independently coded in RStan (or at least I think it is).

- Bob

[Bob Carpenter]

[on stan-dev now]

On 11/30/13, 12:21 PM, Michael Betancourt wrote:
> This would require a giant change to the HMC code, unless
> we move to a general sparse matrix sampler and even that would
> require a lot of work.

If by that you mean we'd need to refactor the matrix interfaces
to the samplers to something more general, then I agree. It's
going to be a lot of work however we do it.

Having structured matrices in the Stan modeling language is
a different, but related issue in terms of the linear algebra.
And it's absolutely critical if we want to build scalable
Gaussian processes along the line of Aki's GPStuff.

- Bob

[Allen Riddell]

I'm very much in favor of factoring out common code from RStan, PyStan, and
CmdStan. I feel like a few judicious #ifdef's might lighten the load on
everyone.

I'll provide one example:

rstan/rstan/inst/include/rstan/mcmc_output.hpp

and

pystan/mcmc_output.hpp

differ only on 10 lines (out of ~200).

Best,

Allen

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

[Bob Carpenter]

Rather than ifdefs, I'd prefer to have reusable base classes or
richer arguments in order to use the language itself rather than
the preprocessor. It's more modular that way and hence makes
testing from within the language a lot easier, for a start.

For example, we have some things hardcoded for files or for std::cout
that should really be abstracted to deal directly with streams.

Michael's introduction of the mcmc_writer abstraction was intended
as a step in this direction, but it looks like we have further to go.

What I really think we need to do is abstract out the reusable bits
of command.hpp so they can be reused in R/Python/etc. --- that is, if
we don't go the route of just wrapping the command-line version of Stan
in the way that R2WinBUGS and R2jags do.

As far as I/O classes go, there's rarely a reason to make decisions statically
using template parameters or ifdefs as opposed to having more runtime
arguments.

- Bob

[Ross Boylan]

On Sat, 2013-11-30 at 12:26 -0500, Bob Carpenter wrote:
> Also, anything with std::cout, std::cin, or std::cerr
> hardcoded needs to be rewritten for RStan and PyStan.

In python having subprocesses that do both input and output can be a bit
challenging, at least if the sequence isn't known. I think more recent
versions (still in the python 2 series) handled this somewhat
better--maybe with the introduction of the subprocess module. ("Recent"
is at least a few years old).

So there might also be python version issues, if you don't want to
exclude "old" pythons. Old probably means < 2.4, since that is when
subprocess (http://docs.python.org/2/library/subprocess.html) was
introduced. As a point of reference, my system running a Debian that is
2 generations out of date still has python 2.5 (though it's possible I
got a backport).

From the motiving PEP (http://www.python.org/dev/peps/pep-0324/):
The subprocess module provides the following enhancements over
previous functions:
......
- A communicate() method, which makes it easy to send stdin data
and read stdout and stderr data, without risking deadlocks.
Most people are aware of the flow control issues involved with
child process communication, but not all have the patience or
skills to write a fully correct and deadlock-free select loop.
This means that many Python applications contain race
conditions. A communicate() method in the standard library
solves this problem.

It is definitely possible to hold onto C++ structures in R; I assume the
present code does because you can reuse models without recompiling them.
If stan ran in a separate process would that be possible?

I'm not sure if the proposal contemplates exposing the command line
invocation of stan to the R or python users; FWIW such a system seems a
bit klunky to me, although it's clearly more robust against interface
changes.

Ross Boylan

[Bob Carpenter]

On 12/2/13, 7:45 PM, Ross Boylan wrote:
> On Sat, 2013-11-30 at 12:26 -0500, Bob Carpenter wrote:
>> Also, anything with std::cout, std::cin, or std::cerr
>> hardcoded needs to be rewritten for RStan and PyStan.
>
> In python having subprocesses that do both input and output can be a bit
> challenging, at least if the sequence isn't known.

Rather than a subprocess (though I'm not sure what that is),
I was imagining something where R or Python writes the data
to file from R or Python, a separate process is executed,
which reads from that file and writes to disk, then R or Python
read it back in.

But anyway, it sounds like nobody wants to do it this way.
So we'll see how long it remains maintainable in the present
configuration of calling the Stan C++ code directly.

> I think more recent
> versions (still in the python 2 series) handled this somewhat
> better--maybe with the introduction of the subprocess module. ("Recent"
> is at least a few years old).
>
> So there might also be python version issues, if you don't want to
> exclude "old" pythons. Old probably means < 2.4, since that is when
> subprocess (http://docs.python.org/2/library/subprocess.html) was
> introduced. As a point of reference, my system running a Debian that is
> 2 generations out of date still has python 2.5 (though it's possible I
> got a backport).
>
> From the motiving PEP (http://www.python.org/dev/peps/pep-0324/):
> The subprocess module provides the following enhancements over
> previous functions:
> ......
> - A communicate() method, which makes it easy to send stdin data
> and read stdout and stderr data, without risking deadlocks.
> Most people are aware of the flow control issues involved with
> child process communication, but not all have the patience or
> skills to write a fully correct and deadlock-free select loop.
> This means that many Python applications contain race
> conditions. A communicate() method in the standard library
> solves this problem.
>
> It is definitely possible to hold onto C++ structures in R; I assume the
> present code does because you can reuse models without recompiling them.

Just models. The samples themselves are translated back into R
structures because there's not an easy way to make C++ structures
persistent. I don't know how important that is, but I think that
was the main design consideration.

> If stan ran in a separate process would that be possible?
>
> I'm not sure if the proposal contemplates exposing the command line
> invocation of stan to the R or python users; FWIW such a system seems a
> bit klunky to me, although it's clearly more robust against interface
> changes.

You could still wrap the command-line calls so the interface
from the R side wouldn't change, though that would mean that RStan
would need to change to keep up with CmdStan, which is our "reference"
implementation insofar as it's what most of our developers use.

- Bob

[Ben Goodrich]

On Tuesday, December 3, 2013 12:45:03 AM UTC-5, Bob Carpenter wrote:

On 12/2/13, 7:45 PM, Ross Boylan wrote:
> On Sat, 2013-11-30 at 12:26 -0500, Bob Carpenter wrote:
>> Also, anything with std::cout, std::cin, or std::cerr
>> hardcoded needs to be rewritten for RStan and PyStan.
>
> In python having subprocesses that do both input and output can be a bit
> challenging, at least if the sequence isn't known.

Rather than a subprocess (though I'm not sure what that is),
I was imagining something where R or Python writes the data
to file from R or Python, a separate process is executed,
which reads from that file and writes to disk, then R or Python
read it back in.

But anyway, it sounds like nobody wants to do it this way.
So we'll see how long it remains maintainable in the present
configuration of calling the Stan C++ code directly.

I'm not that opposed to changing that aspect of it. The only potential benefit of the way we were doing it would be memory, and if we are making copies anyway, then we don't save any memory relative to writing everything to the disk. It would be easier to update RStan.

Ben

[Allen Riddell]

Bob,

I would be in favor of the kind of separation you're describing. Or at least
something that moves in that direction. The less C++ code in the Python package
the easier it will be to maintain.

Best,

-a

[Ross Boylan]

On Tue, 2013-12-03 at 00:45 -0500, Bob Carpenter wrote:
> On 12/2/13, 7:45 PM, Ross Boylan wrote:
> > On Sat, 2013-11-30 at 12:26 -0500, Bob Carpenter wrote:
> >> Also, anything with std::cout, std::cin, or std::cerr
> >> hardcoded needs to be rewritten for RStan and PyStan.
> >
> > In python having subprocesses that do both input and output can be a
> bit
> > challenging, at least if the sequence isn't known.
>
> Rather than a subprocess (though I'm not sure what that is),
> I was imagining something where R or Python writes the data
> to file from R or Python, a separate process is executed,
> which reads from that file and writes to disk, then R or Python
> read it back in.

I'm just using subprocess to mean a (child) process invoked from another
(parent) process. So I think the scenario you are describing is a
subprocess.
Ross

[Allen Riddell]

Might there also be some middle way? Could we use sockets or something to pass
data between Python/R and C++? The maintainability problem isn't with calling
C++ code from Python or R per se, it's all the additional C++ plumbing (shared
by pystan and rstan) to construct the sampler arguments and hold the results.

re: memory. PyStan is creating copies of the data anyway in order to do parallel
sampling (which I think should be the default).

Best,

-a

[Bob Carpenter]

On 12/3/13, 3:37 PM, Allen Riddell wrote:
> Might there also be some middle way? Could we use sockets or something to pass
> data between Python/R and C++? The maintainability problem isn't with calling
> C++ code from Python or R per se, it's all the additional C++ plumbing (shared
> by pystan and rstan) to construct the sampler arguments and hold the results.
>
> re: memory. PyStan is creating copies of the data anyway in order to do parallel
> sampling (which I think should be the default).

I agree about that. I think the reason it's not in
R is that there's not a good way to do it on Windows.

- Bob

[Bob Carpenter]

Allen: You're driving the PyStan bus, so it's
really up to you at this point!

You could pass a socket address for reading to Stan rather
than a file name. I'm not sure what the support is in C++
or Boost for this --- if it's as easy as in Java, it shouldn't
be a problem to generalize our file arguments to sockets.
And ideally, we could even do HTTP to read web-served files;
that's also trivial in Java, but I have no experience doing it in C++.

- Bob

[Allen Riddell]

Perhaps it would be a good idea to revisit this when there's another C++ API
change like arg-refactor. Since all the (overlapping) C++ wrapping code has been
written and is working well, I'm not sure it makes sense to start afresh.

While I'm happy to help in any refactoring effort, my C++ skills are modest.
I only understand the Stan codebase to the extent it touches the C++ wrapping
code that Jiqiang wrote (mainly stan_fit.hpp and mcmc_output.hpp).