Writer refactor

46 views
Skip to first unread message

Bob Carpenter

unread,
Mar 31, 2015, 4:38:05 AM3/31/15
to stan...@googlegroups.com
I'm creating a new thread, so hopefully you can reply to
this.

Why not have the Writer include a method to get an ostream
to write to, as in this:

std::ostream* get_ostream();

Then you could call this method and pass it into the relevant
methods.

- Bob

Michael Betancourt

unread,
Mar 31, 2015, 4:50:46 AM3/31/15
to stan...@googlegroups.com
Because there’s no guarantee that the interfaces
will implement the Writer with streams. It also
seems counter to the general callback pattern.

One option is to have the base_mcmc class
keep a stringstream internally, use that for all
of the model method calls, and then dump the
contents into the Writer callback at the end of
each transition.
> --
> 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

unread,
Mar 31, 2015, 5:01:32 AM3/31/15
to stan...@googlegroups.com
Actually, yeah, having internal stringstream buffers
that get flushed to the callbacks after each transition
should solve everything. As a bonus it will let me
remove the callback template on all the HMC code.

Bob Carpenter

unread,
Mar 31, 2015, 5:06:05 AM3/31/15
to stan...@googlegroups.com

> On Mar 31, 2015, at 7:50 PM, Michael Betancourt <betan...@gmail.com> wrote:
>
> Because there’s no guarantee that the interfaces
> will implement the Writer with streams.

They could always pass in a stringstream themselves
if they don't want to pass it directly to some terminal
output or pass NULL if they don't want output.

> It also
> seems counter to the general callback pattern.

It's just a callback within a callback :-)

Allen B. Riddell

unread,
Mar 31, 2015, 8:48:44 AM3/31/15
to stan...@googlegroups.com
Cython supports a lot of C++ but it doesn't support C++ streams
directly. (Rcpp is very good in this area.)

I'll look at the code again---maybe this isn't relevant.

Michael: are you going to rebase the branch at some point so it's easier
to review?

Thanks,

Allen

Michael Betancourt

unread,
Mar 31, 2015, 8:58:14 AM3/31/15
to stan...@googlegroups.com
It’ll be merged up to develop — a rebase may be tricky due to some conflicts.

Joshua N Pritikin

unread,
Mar 31, 2015, 9:04:59 AM3/31/15
to stan...@googlegroups.com
On Tue, Mar 31, 2015 at 01:58:11PM +0100, Michael Betancourt wrote:
> It’ll be merged up to develop — a rebase may be tricky due to some
> conflicts.

Rebase or merge should have the same set of conflicts, as far as I know.

--
Joshua N. Pritikin
Department of Psychology
University of Virginia
485 McCormick Rd, Gilmer Hall Room 102
Charlottesville, VA 22904
http://people.virginia.edu/~jnp3bc

Allen B. Riddell

unread,
Mar 31, 2015, 9:26:05 AM3/31/15
to stan...@googlegroups.com
Sorry, I mean rebase within the branch, e.g., merge related commits etc
so each commit stands on its own and is easy to review by itself.

Here's a post describing the idea: https://www.reviewboard.org/docs/codebase/dev/git/clean-commits/

Actually, the commit history isn't that bad. There are only a few that
aren't pellucid, e.g. "Almost compilable..." dbd4c7818664cc5c2b5d91e867650751419edb64

Best,

Allen

Michael Betancourt

unread,
Apr 1, 2015, 6:06:38 AM4/1/15
to stan...@googlegroups.com
Allen,

Okay, everything is merged and tests passing and ready for you
to give it a shot at feature/issue-1361-writer_callback.

You should be able to reduce a bunch of code by using the
new method services, as in the now much shorter
src/stan/services/command.hpp.

I believe you’ve already implemented an interrupt callback, which
just means you’ll have to implement writer callbacks for info, errors,
algorithm output, and algorithm diagnostics. info and errors can
just point to cout/cerr (see src/stan/interface_callbacks/writer/(cout|cerr).hpp)
but if you want to guarantee absolutely silent output you can also add
switches and other fancy accouterments. I think you can have some fun
with the output and diagnostic writers — the key/value callbacks should
allow you to write useful items directly into Python memory (like the
adapted step size and metric, plus all the timings) while the std::vector
callback should allow you to stream the samples somewhere useful
as they come in.

Anyways, let me know what you think.

-Mike

Allen B. Riddell

unread,
Apr 1, 2015, 8:24:43 AM4/1/15
to stan...@googlegroups.com
Great. I'll give it a shot.

Do you want to give me a commit reference rather than the branch so you
can keep adding stuff or fixing things?

Best,

Allen

Michael Betancourt

unread,
Apr 1, 2015, 8:53:44 AM4/1/15
to stan...@googlegroups.com
> Do you want to give me a commit reference rather than the branch so you
> can keep adding stuff or fixing things?

I wasn’t planning on changing anything before your feedback, but
to be safe let’s just go with the current head, which for posterity is
9f47ab9481709cd25d0fd2ef5f59d0ff9f22abf4.

Bob Carpenter

unread,
Apr 1, 2015, 9:22:07 PM4/1/15
to stan...@googlegroups.com
I'm confused --- if Cython can't handle std::ostream, how are you
dealing with the streams in the current interface?

- Bob

Allen B. Riddell

unread,
Apr 2, 2015, 10:37:13 AM4/2/15
to stan...@googlegroups.com
Poorly. Either stuff gets thrown into stdout or it gets converted in
pure C++ (in stan_fit.hpp) into something Cython can deal with. I'm told
I can overload the "<<" operator in Cython -- but it would be easier not
to have to do that. My sense is that C++ streams are not used for
communication between different C++ libraries that often -- or am I
wrong about that?

Best,

Allen
Reply all
Reply to author
Forward
0 new messages