Request for Volunteers to Test Adaptation Tweak

[bbbales2]

So let me go ahead and summarize this.

1. As a reviewer, I asked for a test (Request for Volunteers to Test Adaptation Tweak - #64 by bbbales2)

@bbbales2

I think Bob’s question is the last thing that needs to be checked and then this is good. So the question is does upping adapt_delta still get rid of divergences in a similar way as it did previously.

2. As a developer, you suggested I could do this test myself (Request for Volunteers to Test Adaptation Tweak - #66 by betanalpha)

@betanalpha

If you want to verify this empirically then please go ahead, but given the upcoming 2.21 release I will start pushing hard if this can’t be accomplished by next week.

3. I asked you to do this test yourself. As a reviewer I felt I’d already done too much testing here (Request for Volunteers to Test Adaptation Tweak - #48 by bbbales2)
4. As a tech lead you sent me an e-mail suggesting that the tests I was asking from developer-you were inappropriate
5. I contested that and provided evidence why developer-you is fallible (Request for Volunteers to Test Adaptation Tweak - #91 by bbbales2) and said that if tech-lead-you wanted to contest this we could talk to the SGB.
6. Tech-lead you reiterated that again this wasn’t about developer-you

@betanalpha

this isn’t about whether or not the check you requested is easy or not, it’s about whether the test is relevant to the Stan algorithm API

7. Regarding the SGB, SGB-you stepped in to say this also didn’t involve the SGB, and in fact the conflict between tech-lead-you and myself (not a tech lead) should be resolved by the local tech lead (you, in this case).

@betanalpha

Speaking as a nominal tech lead and someone on the SGB, this matter is not within the scope of the SGB. The tech local tech lead, myself, sets local policy and conflicts between tech leads go up to the technical working group director.

8. Tech-lead-you verbally approved the pull request from developer-you and requested that it be merged by the technical working director @seantalts (Feature/2789 improved stepsize adapt target by betanalpha · Pull Request #2836 · stan-dev/stan · GitHub). There were no external code reviews at this point. It was merged 30 minutes later.

In paragraph form, I asked, as the reviewer, you to verify the behavior of adapt_delta. This was based on input from @Bob_Carpenter in this thread, and everyone who came to the Stan meeting (which is publicly announced Request for Volunteers to Test Adaptation Tweak - #58 by bbbales2). In that meeting, everyone basically thought the changes sounded good (@jonah and @bgoodri in particular), and @jonah agreed we should at least verify adapt_delta behavior. @ariddell and @ahartikainen never responded. I think that addresses this complaint from tech-lead-you:

@betanalpha

It also sets precedent that reviewers can impose tests of undone behavior, implicitly forcing behavior expectations without discussion or consensus.

So I asked developer-you to verify the adapt_delta behavior. And I understand that divergences are an indicator that things have gone awry, but considering these changes to the timestep adaptation are most different for problems with divergent trajectories, I judged that it would be prudent to test the behavior of the new adaptation on problems where divergent transitions arise, regardless of the statistical validity of the output. Regarding the central limit theorem, I don’t believe warmup as implemented in Stan is reversible, and so I don’t know how much these arguments about CLT and whatnot apply (given that timestep adaptation is happening in warmup).

@betanalpha

Showing that increasing adapt_delta reduces divergences on one model isn’t lenient – because that isn’t even guaranteed behavior its success would be irrelevant to other models and if anything give a false sense of security.

That’s what lenient means.

If the relation between adapt_delta and timestep is monotonic, then why didn’t we simply lower the default adapt_delta target with the previous implementation? If we’re only testing on models without divergences, what’s the problem? We can achieve the same difference right?

@betanalpha

Yeah. In terms of divergences the tweak just rescales adapt_delta so it’s equivalent to running the old algorithm with a slightly lower adapt_delta or equivalently a higher step size. When a model is diverging both should lead to more divergent trajectories (modulo statistical variation).

Oh okay, that’s cool (Request for Volunteers to Test Adaptation Tweak - #49 by betanalpha).

MODERATOR EDIT: The following section contained personal attacks which have been removed (by @martinmodrak)

So then all this difficult work you’ve done [here] is to avoid writing tests for a pull request that could be replaced by lowering the adapt_delta defaults?

I just did the tests myself in 10 minutes. They seem fine to me. I reject this pull request.