Re: head's up: merging bbvb into develop

[Bob Carpenter]

[moved to stan-dev]

I'm not exactly sure what "outputting stuff" means. Is
it a Stan or CmdStan issue? Anyway, if there's an issue,
we obviously can't release it until it's fixed, which means
waiting for 2.8.

In any case, there needs to be a unit test that catches the
problem or it's going to get released in a broken state. We do
not do any by-hand testing on releases, nor do we want to. If this
kind of thing can happen, then it signals we don't yet have an
appropriate set of tests for advi.

- Bob

P.S. I'm bcc-ing Kui and Dustin, but if they want to communicate to
Stan developers about release issues, they should also be on stan-dev.

P.P.S. As I've said before, I think the name "advi" is misleading
because the key point isn't autodiff --- any method of computing
model gradients would work. But I can live with it that's the
majority feeling or if it's too much of a pain to change.

What's the name of the CmdStan top-level equivalent of "sample" and
"optimize" and "diagnose" for advi?


> On Jun 23, 2015, at 2:45 PM, Alp Kucukelbir <akucu...@gmail.com> wrote:
>
> hi daniel, bob + dustin, kui,
>
> dustin told me that you guys are considering
> merging this branch in for 2.7. that's GREAT
> news. so happy.
>
> kui separately told me that he did this on his
> end and that bbvb stopped outputting stuff.
>
> i just wanted to alert everyone to this. i'm sure
> it's an easy fix, but everyone on this email
> could likely benefit from being informed.
>
> cheers
> alp

[Dustin Tran]

I’m not sure what that means either. I can add a test to see if variational inference prints anything to standard output (which may be what Kui means?).

The name of the CmdStan top-level equivalent is currently `experimental variational`, e.g., `./bernoulli experimental variational data file=bernoulli.data.R`. It is under `experimental` at the moment because I believe EP was supposed to be (or is) as well.

I tend to agree on the comments about the ADVI name. It’s nice that it emphasizes one of the primary features that distinguishes our variational inference from other implementations. However, there are other primary features in place too, e.g., parameter transformations, and there will be many other excellent features in the future such as my long term goal of putting in copulas to generally learn the dependency structure among the latent variables. I personally like “automatic variational inference”, as in the title of the preprint and which follows the motivation of probabilistic programming more closely. Regardless, Alp has been spearheading the project from the get-go, and I’m happy to stick by whatever he decides to name it.

Dustin

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

+1 to advi being awkward. If black box VB (bbvb) is no good
then I’d support auto_vb.

[Dustin Tran]

bbvb is a no go because the algorithm isn’t technically running bbvb from the original paper (ours has lower variance)

[Michael Betancourt]

You’ve hit one of my sore spots — the terrible pollution of names (really, branding)
endemic to machine learning and increasingly statistics. Honestly, I don’t think we
should let that terrible behavior affect out naming choices as most of our users are
not in the machine learning community (and if we were really serious about it
we’d have to start following startups and use misspellings).

Following the other algorithms we should just call the base folder “vb" and then
have this specific implementation in that folder with a given name. Perhaps
“unconstrained_gauss_vb” or the like.

[Bob Carpenter]

Thanks for reposting on stan-dev.

The first order of business is to create an issue. Ideally
this will come with a test that fails. Feel free to actually
write it as a test in a branch and then we can work on the fix
in that branch.

We do not want any fixes that do not come with unit tests
that (a) fail in the old version, and (b) pass in the new.

I don't even know if this is a CmdStan or Stan issue right
now given the fluid nature of the command code recently.

Nothing should be testing cin or cout, but should instead
use passed-in stringstreams to collect answers.

Daniel will hopefully know how to fix the problem.

And by the way, this is why we like those completely broken
tests that just make sure output's getting through. (They're
broken because they're typically hardwired to exact output rather
than more functional.)

- Bob

> On Jun 23, 2015, at 5:11 PM, Kui Tang <kui...@gmail.com> wrote:
>
> I believe it's an interaction between the recent cout/cerr fix.
>
> I merged feature/bbvb into develop and fixed one trivial conflict involving some #includes in a test case. Everything compiles.
>
> When running CmdStan on the bernoulli example

>
> ./bernoulli experimental variational data file=bernoulli.data.R
>

> the output.csv file contains only comments indicating the options, and no iteration trace.
>
> I believe there is something in the variational module that does not play nicely with the new cout/cerr machinery, and hopefully it will be a simple fix. I have not yet had the time to investigate the code myself though.
>
>
> On Tue, Jun 23, 2015 at 2:11 PM, Kui Tang <kui...@gmail.com> wrote:
> I believe it's an interaction between the recent cout/cerr fix.
>
> I merged feature/bbvb into develop and fixed one trivial conflict involving some #includes in a test case. Everything compiles.
>
> When running CmdStan on the bernoulli example

>
> ./bernoulli experimental variational data file=bernoulli.data.R
>

> the output.csv file contains only comments indicating the options, and no iteration trace.
>
> I believe there is something in the variational module that does not play nicely with the new cout/cerr machinery, and hopefully it will be a simple fix. I have not yet had the time to investigate the code myself though.

> --
> Sincerely,
> Kui Tang
>
>
>
> --
> Sincerely,
> Kui Tang

[Alp Kucukelbir]

internally, we can call it whatever we'd like. the current cmdStan interface
uses the words "experimental variational".

but the algorithm is called ADVI in the NIPS submission. it will also be called that
in the journal extension. for better or worse, that's it's name (like NUTS), so i think
sticking to the name would be helpful, for clarity's sake.

[Bob Carpenter]

I'd rather not include "experimental" in the command name because
that builds in a future break in backward compatiblity.

How about just "vb" or "vi"? If we introduce variations later,
those can either get different commands or these commands can be
modified (yes, that'll break backward compatiblity, but that's
how our commands are currently set up).

I'd also be OK with advi --- I would overall prefer finer-grained
commands.

I think we have some latitude because we're already planning
a backward-compatiblity break in Stan 3, with the sincere hope we
don't dig ourselves into the Python 2 vs. Python 3 hole, with a lot
of users left behind.

- Bob

[Alp Kucukelbir]

adding experimental was something you guys asked me to do!

i vote for "vi" or just "variational". 

i just think the documentation etc. should refer to the algorithm

as ADVI. just like how cmdStan uses "sample" but refers to 

"NUTS" in the manual.

You received this message because you are subscribed to a topic in the Google Groups "stan development mailing list" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/stan-dev/OHN_viSpvtQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to stan-dev+u...@googlegroups.com.

[Bob Carpenter]

> On Jun 24, 2015, at 11:27 AM, Alp Kucukelbir <akucu...@gmail.com> wrote:
>
> adding experimental was something you guys asked me to do!

Sorry about the miscommunication --- if someone asked you
to make it part of the command name, I missed it, or I would've
objected then.

We just wanted to label the whole algorithm "experimental" in
the manual and perhaps spit out a warning at the top of running
it to reinforce the idea. Then we could easily remove those things
later without breaking backward compatiblity. That's how we rolled
out the optimizer.

> i vote for "vi" or just "variational".
>
> i just think the documentation etc. should refer to the algorithm
> as ADVI. just like how cmdStan uses "sample" but refers to
> "NUTS" in the manual.

CmdStan uses "engine=nuts" (if I recall the syntax correctly)
by default for "sample". So So VI could use "engine=advi" or
whatever by default to leave room for future implementations
in the same command.

Michael may have some input here having designed that hierarchical
command structure in the first place. It'll get flattened
out considerably into its own command in RStan and PyStan.

- Bob

[Alp Kucukelbir]

ok. dustin fixed the lack of output business. it seems
like the definition of "write_iteration" changed.

i'll get rid of the "experimental" keyword and update
all the documentation etc.

i don't mind us changing the interface down the road.

[Bob Carpenter]

> On Jun 24, 2015, at 3:00 PM, Alp Kucukelbir <akucu...@gmail.com> wrote:
>
> ok. dustin fixed the lack of output business. it seems
> like the definition of "write_iteration" changed.

Glad to hear it. Once things are merged, then it's on
whoever changes things to make sure unit tests all pass,
which is another reason to have good tests. And a good
reason to merge early and often, which wasn't really possible
with this major-new-feature project.

> i'll get rid of the "experimental" keyword and update
> all the documentation etc.
>
> i don't mind us changing the interface down the road.

I do mind changing interfaces. Breaking backward compatibility
is a huge deal in non-research code, because it breaks things
users have developed and forces them to learn new idioms. And
they're not invested in the guts of our package as we are, so for
them, it's just a huge hassle when they update the package
and everything breaks.

- Bob

[Alp Kucukelbir]

roger that.

ok so i've made the changes as discussed above. however
the following travis CI test seems to have failed.

https://travis-ci.org/stan-dev/stan/jobs/68211740

i'm not quite sure what went on here. these tests are
about mcmc i believe?

cheers
alp

[Bob Carpenter]

I'm guessing this is related to Daniel's refactoring
of std::cout. I understand Dustin coded around that in
your stuff, but maybe it's broken for MCMC?

- Bob

[Dustin Tran]

Oops, forgot to reply to this. The only change I added due to Daniel's refactoring was adding a new argument to all calls to write_iteration() in advi.hpp. I don't think MCMC calls this, and it currently compiles fine in develop as Daniel's refactoring was merged there.

Dustin

[Alp Kucukelbir]

hi daniel,

so dustin and i "re-launched" the travis test, and it worked this time.
there must've been some problem with travis in the previous push.

we've made all the cout/cerr adjustments. as of right now feature/bbvb
is up to date with stan/develop, and it works great with cmdStan/develop.

cheers
alp

[Daniel Lee]

Presumably there's a pull request open for CmdStan? Are there any tests that use advi from CmdStan?

[Alp Kucukelbir]

there is a pull request open for cmdStan.

dustin and i just looked at the tests in cmdStan. we're not

quite sure how to adapt them for ADVI, especially since

it seems like we haven't fully settled on the command 

structure for ADVI. (dustin and i are considering a multitude

of extensions that would slightly change the command structure.)

presumably we could just copy the test for optimization and

look for things in the header of output.csv? what do you think?

--
You received this message because you are subscribed to a topic in the Google Groups "stan development mailing list" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/stan-dev/OHN_viSpvtQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to stan-dev+u...@googlegroups.com.

[Daniel Lee]

I think that's good enough for now. We really need to do a lot more end-to-end testing at the CmdStan / RStan / PyStan level, but we just haven't built it out. I want to make sure that even though changes to Stan's variational stuff look ok, that it actually runs from CmdStan as you intended it.

Should we wait for the arguments to settle before merging? Those are currently in Stan.

[Alp Kucukelbir]

sounds good. everything seems to run perfectly fine from cmdStan.

i think we're set to go. the arguments are as stable as they are

going to be. any modifications will be in the months-to-a-year

timeframe, as they involve exploring new research ideas.

[Dustin Tran]

As an FYI, the pull request for feature/advi-command is still outstanding: https://github.com/stan-dev/cmdstan/pull/117. It's required to run variational inference in cmdstan.

Now that feature/bbvb is merged, hopefully someone is also interested in adding variational inference to the other interfaces.

Dustin

[Bob Carpenter]

I just wanted to call this out with a new subject.

And yes, we definitely need to get the CmdStan
merge done. Michael is probably the best person to look
at that, but he's joining the gang at ICML. So Daniel and
I could take a look next week.

- Bob