Writers in the HMC Code

[Michael Betancourt]

I just spend the last there hours or so trying to incorporate all of the
changes needed to make the RHMC branch work with the added
error writers, and not the majority of the HMC code is passing around
these f’ing writers, most of which do nothing. When more writers
are added to implement logging it’s going to get even worse.

I really want to fix this soon to put the HMC code back in a clean
state. I imagine this would consist of

a) A logger object which wraps all of the necessary writers and
exposes methods like “write_info”, “writer_fatal”, etc.

b) Passing the logger to the samplers on construction.

c) Storing a reference to the input logger in the internal ps_point
instantiation so that when this->z_ is passed around so to is
the logger.

In this way the logging implementation can be tweaked without
affecting the HMC code.

Also, in the future please don’t merge updates that touch nearly
all of the HMC code without my review.

[Bob Carpenter]

> On Apr 25, 2016, at 11:21 AM, Michael Betancourt <betan...@gmail.com> wrote:
>
> I just spend the last there hours or so trying to incorporate all of the
> changes needed to make the RHMC branch work with the added
> error writers, and not the majority of the HMC code is passing around
> these f’ing writers, most of which do nothing. When more writers
> are added to implement logging it’s going to get even worse.
>
> I really want to fix this soon to put the HMC code back in a clean
> state. I imagine this would consist of
>
> a) A logger object which wraps all of the necessary writers and
> exposes methods like “write_info”, “writer_fatal”, etc

+1. I've been saying the same thing.

> b) Passing the logger to the samplers on construction.

I take it they're all stateful already?

> c) Storing a reference to the input logger in the internal ps_point
> instantiation so that when this->z_ is passed around so to is
> the logger.

I have no opinion on this.

> In this way the logging implementation can be tweaked without
> affecting the HMC code.

:-) I already suggested the same thing to Daniel.

> Also, in the future please don’t merge updates that touch nearly
> all of the HMC code without my review.

OK by me.

- Bob

[Michael Betancourt]

The HMC samplers have an appropriate derived ps_point implementation

as well as the appropriate Hamiltonian, integrator, etc.  But all nontrivial

state is in that ps_point.

[Daniel Lee]

On Mon, Apr 25, 2016 at 11:21 AM, Michael Betancourt <betan...@gmail.com> wrote:

Also, in the future please don’t merge updates that touch nearly
all of the HMC code without my review.

That was my bad. I'll avoid doing that. We needed it for a change in behavior upstream (in RStan).

Once we got to generic logging, it'll have to change, but I'll make sure it's run by you before it happens.

Daniel

[Michael Betancourt]

In this case a second writer wasn’t needed — the existing code

could have been used with an error_writer passed into each

method by the services, instead of the info_writer.

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

[Daniel Lee]

I traced through it all and unfortunately, we needed a second writer. I tried to replace it with the writers and leave the implementation alone (at multiple levels), but it didn't work for all the edge cases.

[Michael Betancourt]

What edge cases?  The only output is error messages from bad gradients

and the verbose output from verbose_evolve which can go anywhere as

it’s not exposed.

[Daniel Lee]

I'm pretty sure it had to do with print() and reject(). Don't remember the exact cases now.

[Bob Carpenter]

print() is sent to streams passed into the model class
methods. reject() creates text for an exception.

- Bob