Stan IO

[Krzysztof Sakrejda]

I'll have some time available on an ongoing basis over the next six months or so to do some Stan-related projects. One of the things I'm interested in is the input/output (reader/writer?) refactor but it's hard to figure out where the different pieces of it are at and how to contribute. I've already looked over the code under (https://github.com/stan-dev/stan/pull/1546) so I get what the overall goal is. Any suggestions about what the needs are for that branch at the moment?

[Bob Carpenter]

You want to talk to Michael about the writer refactor.
It's a mess right now --- that's the point of refactoring.

- Bob

> On Sep 10, 2015, at 1:13 PM, Krzysztof Sakrejda <krzysztof...@gmail.com> wrote:
>
> I'll have some time available on an ongoing basis over the next six months or so to do some Stan-related projects. One of the things I'm interested in is the input/output (reader/writer?) refactor but it's hard to figure out where the different pieces of it are at and how to contribute. I've already looked over the code under (https://github.com/stan-dev/stan/pull/1546) so I get what the overall goal is. Any suggestions about what the needs are for that branch at the moment?
>

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

[Michael Betancourt]

Well, it hasn’t been the most elegant process. Basically my proposing something
to little response then my implementing it to everyone complaining about one thing
or another with no constructive alternatives. Hence everything moving glacially.

At the moment the writer is stuck in pull request limbo, and given that no one has
picked up I’m going to have to put off the ODE stuff and do it this week. Because
that’s all written and needs only be split up into a million pull requests, the only to
do is to update the interfaces (other than CmdStan which is already ready in a
branch).

Then there is the refactoring of the var_context. This will require lots of work
but I need to get it into a working state first, which will require the writer be done
to avoid some nasty merge conflicts. Once we start on this again there will be
plenty to help with.

Once that’s done we have to figure out how to implement the new flat arguments
(let’s all complain that we don’t like the current set up but not write a new one!),
especially validation methods for the interfaces.

Finally at that point we can implement a slew of new features blocked by the
lack of a consistent API and then define an output file specification consistent
with the API I/O routes. Given that spec we can write a robust csv_reader,
and all kinds of fancy bits like streaming data formats and streaming estimators.

So the immediate to dos are sparse
1) Propagate the new writer callback through the VB code and get it to compile.
2) Wait for all of this to be merged into Stan and then help update the interfaces.

[Krzysztof Sakrejda]

On Monday, September 14, 2015 at 5:55:16 AM UTC-4, Michael Betancourt wrote:
> Well, it hasn’t been the most elegant process. Basically my proposing something
> to little response then my implementing it to everyone complaining about one thing
> or another with no constructive alternatives. Hence everything moving glacially.

Thanks for taking the time to sketch out where things are at, I appreciate it's frustrating to move something this big. I don't have much of an immediate response to this but it does help with reading the code. I do have two questions below.

> At the moment the writer is stuck in pull request limbo, and given that no one has
> picked up I’m going to have to put off the ODE stuff and do it this week. Because
> that’s all written and needs only be split up into a million pull requests, the only to
> do is to update the interfaces (other than CmdStan which is already ready in a
> branch).

Is the right CmdStan branch the update_to_write_callback or issue-151-remove-writer? It would probably help me understand what's going on if I have an example to go by on the interface side.

> Then there is the refactoring of the var_context. This will require lots of work
> but I need to get it into a working state first, which will require the writer be done
> to avoid some nasty merge conflicts. Once we start on this again there will be
> plenty to help with.

When you say "need to get [var_context] into a working state", does that mean that it's not in use? I thought I saw it in CmdStan and rstan interface code. Could you elaborate a little more about that bit?

> Once that’s done we have to figure out how to implement the new flat arguments
> (let’s all complain that we don’t like the current set up but not write a new one!),
> especially validation methods for the interfaces.
>
> Finally at that point we can implement a slew of new features blocked by the
> lack of a consistent API and then define an output file specification consistent
> with the API I/O routes. Given that spec we can write a robust csv_reader,
> and all kinds of fancy bits like streaming data formats and streaming estimators.

+1 for streaming I/O!

Anyway, thanks again for clarifying what's going on, it really helps to understand the C++ API and where it's going.

Krzysztof

[Daniel Lee]

Just a couple comments:

On Mon, Sep 14, 2015 at 7:56 AM, Krzysztof Sakrejda <krzysztof...@gmail.com> wrote:

On Monday, September 14, 2015 at 5:55:16 AM UTC-4, Michael Betancourt wrote:
> Well, it hasn’t been the most elegant process.  Basically my proposing something
> to little response then my implementing it to everyone complaining about one thing
> or another with no constructive alternatives.  Hence everything moving glacially.

That isn't why it's moving glacially. The reason most things get into the code base slowly is because pull requests are usually too big on construction, have problems, and are really not ready to go into the code as-is. There are plenty of pull requests that move quickly because they're easy to review, easy to verify, and they're ready to go (or close) when submitted.

Most proposals are fine, but proposals and implementation are different. When we start reviewing implementation, we actually look through and make sure it's not just correct but also something that can be maintained going forward.

Thanks for taking the time to sketch out where things are at, I appreciate it's frustrating to move something this big.  I don't have much of an immediate response to this but it does help with reading the code.  I do have two questions below.

> At the moment the writer is stuck in pull request limbo, and given that no one has
> picked up I’m going to have to put off the ODE stuff and do it this week.  Because
> that’s all written and needs only be split up into a million pull requests, the only to
> do is to update the interfaces (other than CmdStan which is already ready in a
> branch).

Is the right CmdStan branch the update_to_write_callback or issue-151-remove-writer?  It would probably help me understand what's going on if I have an  example to go by on the interface side.

The update_to_write_callback is where we want to get. issue-151-remove-writer is a much narrower scoped pull request. Check stan-dev/math#151

 

> Then there is the refactoring of the var_context.  This will require lots of work
> but I need to get it into a working state first, which will require the writer be done
> to avoid some nasty merge conflicts.  Once we start on this again there will be
> plenty to help with.

When you say "need to get [var_context] into a working state", does that mean that it's not in use?  I thought I saw it in CmdStan and rstan interface code. Could you elaborate a little more about that bit?

Michael's got some updates to the var_context that should make things much easier. It'll conflict with the writer interface change to the api.

 

> Once that’s done we have to figure out how to implement the new flat arguments
> (let’s all complain that we don’t like the current set up but not write a new one!),
> especially validation methods for the interfaces.
>
> Finally at that point we can implement a slew of new features blocked by the
> lack of a consistent API and then define an output file specification consistent
> with the API I/O routes.  Given that spec we can write a robust csv_reader,
> and all kinds of fancy bits like streaming data formats and streaming estimators.

+1 for streaming I/O!

Ok, but there are deeper issues here. Right now, the var_context is immutable, so it's easy to think about the instantiated model class as really exposing a function:

p(x, theta)

where x is truly fixed and is data.

We then take that instantiated model and do things assuming that this is true and that:

p(theta | x) \propto p(x, theta)

But... what happens when data is streaming and mutable? Then every call to p(x, theta) isn't really that what we thought it was.

I have no problem with it, but I do think we need to make it clear when data is fixed and when data is not. We've always assumed in our code that data is fixed, but once we change that, we'll need to rethink how we handle var_contexts.

 

Anyway, thanks again for clarifying what's going on, it really helps to understand the C++ API and where it's going.

Krzysztof

>
> So the immediate to dos are sparse
> 1) Propagate the new writer callback through the VB code and get it to compile.
> 2) Wait for all of this to be merged into Stan and then help update the interfaces.

>
>
> > I'll have some time available on an ongoing basis over the next six months or so to do some Stan-related projects.  One of the things I'm interested in is the input/output (reader/writer?) refactor but it's hard to figure out where the different pieces of it are at and how to contribute.  I've already looked over the code under (https://github.com/stan-dev/stan/pull/1546) so I get what the overall goal is.  Any suggestions about what the needs are for that branch at the moment?

[Michael Betancourt]

> Is the right CmdStan branch the update_to_write_callback or issue-151-remove-writer? It would probably help me understand what's going on if I have an example to go by on the interface side.

update_to_writer_callback will the eventual state of CmdStan.

> When you say "need to get [var_context] into a working state", does that mean that it's not in use? I thought I saw it in CmdStan and rstan interface code. Could you elaborate a little more about that bit?

See https://github.com/stan-dev/stan/tree/feature/refactor_io_reader_writer

The idea is for the interfaces to create all var_contexts externally and then
pass them into the API. In order to get randomized inits this requires changing
the way the var_contexts are accessed, in particular factoring out all of the
transformations hidden in io_reader and io_writer.

This requires a ton of changes, and this branch never compiled because
I got to the point where making it work would conflict with the changes
in the writer.

[Michael Betancourt]

That isn't why it's moving glacially. The reason most things get into the code base slowly is because pull requests are usually too big on construction, have problems, and are really not ready to go into the code as-is. There are plenty of pull requests that move quickly because they're easy to review, easy to verify, and they're ready to go (or close) when submitted.

The API is an f’ing mess right now.  Nothing is going to be easy to review, and if

I’m submitting bad pull requests then how come literally no one else is working

on the API to make it better?

Most proposals are fine, but proposals and implementation are different. When we start reviewing implementation, we actually look through and make sure it's not just correct but also something that can be maintained going forward. 

Then you’re not reviewing the proposals well enough.  If every change requires

“write and then we’ll talk” then we’re going to waste a tremendous amount of

manpower.  

+1 for streaming I/O!

Ok, but there are deeper issues here. Right now, the var_context is immutable, so it's easy to think about the instantiated model class as really exposing a function:

p(x, theta)

where x is truly fixed and is data.

We then take that instantiated model and do things assuming that this is true and that:

p(theta | x) \propto p(x, theta)

But... what happens when data is streaming and mutable? Then every call to p(x, theta) isn't really that what we thought it was.

I have no problem with it, but I do think we need to make it clear when data is fixed and when data is not. We've always assumed in our code that data is fixed, but once we change that, we'll need to rethink how we handle var_contexts.

I very much did _not_ mean streaming var_contexts.  I meant streaming outputs

to file, as is currently done in CmdStan but perhaps in a compressed format,

and streaming input to the stan_csv_reader.  For example, having a stansummary

executable that doesn’t need to load everything into memory but rather can

compute all posterior expectations using streaming estimators (like the Welford

estimators).

[Daniel Lee]

On Mon, Sep 14, 2015 at 12:42 PM, Michael Betancourt <betan...@gmail.com> wrote:

That isn't why it's moving glacially. The reason most things get into the code base slowly is because pull requests are usually too big on construction, have problems, and are really not ready to go into the code as-is. There are plenty of pull requests that move quickly because they're easy to review, easy to verify, and they're ready to go (or close) when submitted.

The API is an f’ing mess right now.  Nothing is going to be easy to review, and if

I’m submitting bad pull requests then how come literally no one else is working

on the API to make it better?

=). Great question. We definitely could use help.

One of the problems is that there aren't enough tests to make it easy to shift things around safely. So... if anyone wanted to move something, the thing that would help the most is to add tests to existing things.

 

Most proposals are fine, but proposals and implementation are different. When we start reviewing implementation, we actually look through and make sure it's not just correct but also something that can be maintained going forward. 

Then you’re not reviewing the proposals well enough.  If every change requires

“write and then we’ll talk” then we’re going to waste a tremendous amount of

manpower.  

I disagree that this takes a tremendous amount of manpower. But I think what you're suggesting is that the "proposal" should be much more than what they are (which I would be happy to agree to, but I think it still takes the same manpower). Currently, proposals are just a suggestion of where to go next; I think you're suggesting it should actually be a design doc and an implementation sketch. I'm ok with that, but for those that haven't done these things in the past, it's not straightforward to do this well. So far we've let implementations speak for themselves. Of course, you're right. We should have more up-front checking, but we just haven't done that to date.

[Krzysztof Sakrejda]

On Monday, September 14, 2015 at 11:48:19 AM UTC-4, Daniel Lee wrote:
[snip]

Thanks for the info about branches, it helps sketch things out.

> > Finally at that point we can implement a slew of new features blocked by the
> > lack of a consistent API and then define an output file specification consistent
>
> > with the API I/O routes.  Given that spec we can write a robust csv_reader,
> > and all kinds of fancy bits like streaming data formats and streaming estimators.
>
> +1 for streaming I/O!
>
> Ok, but there are deeper issues here. Right now, the var_context is immutable, so it's easy to think about the instantiated model class as really exposing a function:
>
> p(x, theta)
>
> where x is truly fixed and is data.
>
>
> We then take that instantiated model and do things assuming that this is true and that:
>
> p(theta | x) \propto p(x, theta)
>
>
> But... what happens when data is streaming and mutable? Then every call to p(x, theta) isn't really that what we thought it was.
>
>
> I have no problem with it, but I do think we need to make it clear when data is fixed and when data is not. We've always assumed in our code that data is fixed, but once we change that, we'll need to rethink how we handle var_contexts.

I'm mostly excited for streaming output as Michael suggests b/c handling large .csv files has been a nuisance for my workflow. I've seen some use cases tossed around for mutable data and it looks interesting but not really all that related to what I'm doing now. Streaming input would be neat if it made writing interfaces simpler and more stable (I'm thinking of the R-related segfault problems I've seen in rstan).

[Bob Carpenter]

> On Sep 14, 2015, at 3:05 PM, Krzysztof Sakrejda <krzysztof...@gmail.com> wrote:
>
> ...

> I'm mostly excited for streaming output as Michael suggests b/c handling large .csv files has been a nuisance for my workflow.

I thought streaming output would be directed to .csv files.

Or do you mean streaming where you accumulate means and covariances?
It's not possible to compute streaming medians without storing at
least half of what would be in that .csv file in memory or somewhere.
95% intervals could be done storing 5% of the data in memory and so on.

- Bob

[Krzysztof Sakrejda]

On Monday, September 14, 2015 at 3:44:11 PM UTC-4, Bob Carpenter wrote:

> > On Sep 14, 2015, at 3:05 PM, Krzysztof Sakrejda <krzysztof./..@gmail.com> wrote:
> >
> > ...
>
> > I'm mostly excited for streaming output as Michael suggests b/c handling large .csv files has been a nuisance for my workflow.
>
> I thought streaming output would be directed to .csv files.

I would prefer to stream output to something else. My first thought for an alternative was a database keyed by sample/parameter. I get that write times could be an issue but for model complex models with model checking I end up with thousands of parameters so I really want something keyed that does not rely on R's memory management.

Krzysztof

[Bob Carpenter]

Let me back up and stop guessing. What's the nuisance? I
don't understand "something keyed that doesn't rely on R".

There's no way we're going to want to require a database to be
set up to run Stan --- there are too many dependencies already.
It could be optional in some way, which I guess R is OK with.

- Bob

[Krzysztof Sakrejda]

On Monday, September 14, 2015 at 5:05:49 PM UTC-4, Bob Carpenter wrote:
> Let me back up and stop guessing. What's the nuisance?

Very wide .csv files take a long time to load into memory, they are unwieldy to maninpulate in R, they are difficult to read with csvkit (complicated header, adaptation data after column names), they are difficult to extract metadata from... if I load/subset, I have to reload each time I want a new subset of columns (also slow)... I think that's the main list.

> I
> don't understand "something keyed that doesn't rely on R".
>
> There's no way we're going to want to require a database to be
> set up to run Stan --- there are too many dependencies already.
> It could be optional in some way, which I guess R is OK with.

All Stan needs to do is hand over control over output by relying on callbacks. I could see two ways of doing it:

1) Create a Stan interface that relies on streaming everything (model, data, etc...) through protocol buffers (?) and provide the necessary callbacks as part of that interface. The result is another interface package, as independent of Stan/Math libraries as any other. A more reasonable goal might be to fork CmdStan just to experiment with how well streaming output to different databases works.

2) Make rstan's API (_NOT_ the regular interface) flexible enough to allow the programmer to pass in arbitrary callbacks. The result is another R package that's dependent on rstan.

I'm not saying either option is straightforward but with the current refactor I don't think it's outlandish either.

Krzysztof

>
> - Bob

[Krzysztof Sakrejda]

Thanks for this, it'll take me a while to figure out what's going on but this helps a lot.

Krzysztof

[Michael Betancourt]

One of the problems is that there aren't enough tests to make it easy to shift things around safely. So... if anyone wanted to move something, the thing that would help the most is to add tests to existing things.

WE ARE BREAKING BEHAVIOR TO FORM A CONSISTENT API.

By construction existing tests would all fail — this is pointless.

[Michael Betancourt]

Or do you mean streaming where you accumulate means and covariances?
It's not possible to compute streaming medians without storing at
least half of what would be in that .csv file in memory or somewhere.
95% intervals could be done storing 5% of the data in memory and so on

Median’s require just two samples — you throw away everything other

than the central two values, which can be done streaming.  Then interpolate

the two central values however you like to give the median estimator.

[Michael Betancourt]

I think the more appropriate statement is “we are not going to maintain any databases”.

So if you want to fork Stan or an interface and add writer callbacks that stream to databases

then go for it (a consistent API should make that straightforward) but don’t expect it to be

officially supported by Stan.

[Krzysztof Sakrejda]

On Tuesday, September 15, 2015 at 4:53:04 AM UTC-4, Michael Betancourt wrote:
> I think the more appropriate statement is “we are not going to maintain any databases”.
> So if you want to fork Stan or an interface and add writer callbacks that stream to databases
> then go for it (a consistent API should make that straightforward)

Yes, that's what I'd like to ultimately go for, but for now I'm happy to do some of the work to improve the API.

> but don’t expect it to be officially supported by Stan.

Yes, I have no such expectation.

Krzsyztof

[Bob Carpenter]

Agreed. So we need to update the tests along with the behavior.

- Bob

[Bob Carpenter]

Allen's been suggesting along similar lines for protocol buffers.
As Michael says, I believe this could be layered on top of the
underlying Stan API.

- Bob

[Daniel Lee]

Just wanted to state:

Testing existing code isn't pointless. It helps with a lot of things, especially with refactoring. Fixing existing tests is much easier than trying to figure out how to instantiate code that's never been tested, that isn't designed to be tested, etc.

Daniel

[Krzysztof Sakrejda]

On Tuesday, September 15, 2015 at 11:46:00 AM UTC-4, Bob Carpenter wrote:
> Allen's been suggesting along similar lines for protocol buffers.
> As Michael says, I believe this could be layered on top of the
> underlying Stan API.
>
> - Bob

Agreed, and I did notice a few other people express interest. Once this part is done I want to contribute to (and maybe drive if nobody else does) having that happen. Krzysztof