Possible mistake in dual average implementation

[aseyboldt]

There's a difference in the dual averaging algorithm in stan/nutpie and walnuts, and it looks to me like a typo, not an intended change?

In the walnuts implementation log_est_avg is updated based only on log_est_ and last_log_est:
https://github.com/flatironinstitute/walnuts/blob/main/include/walnuts/dual_average.hpp#L103

In stan it is updated based on the last log_est_avg value and the current log_est value (called x_bar_ and x in that codebase):
https://github.com/stan-dev/stan/blob/develop/src/stan/mcmc/stepsize_adaptation.hpp#L68

This might explain why the step size adaptation works better with different parameters in the walnuts codebase?

I tried the new walnuts parameters from #25 in nutpie (which matches the stan impl at least in that regard at least), and I don't get good estimates (one color per chain):

compared to the old parameters:

(I accidentally opened an issue in the old walnuts repo first: bob-carpenter/walnuts#48)

[WardBrian]

I believe you are correct that this is a mistake. Here's the relevant equation from Hoffman & Gelman 2014:

In particular, it looks like the current code here incorrectly uses $\log{\epsilon_{m-1}}$ for the final term, rather than $\log{\bar\epsilon_{m-1}}$. Pretty easy to miss that little -!

Line 102 should read log_est_avg_ = prop2 * log_est_ + (1 - prop2) * log_est_avg_; to match the paper and Stan/nutpie's code, and line 99 can just be deleted

[bob-carpenter]

Yikes---will fix ASAP. I guess my convergence unit tests weren't sufficient :-(.

[aseyboldt]

In fairness to those tests, I think it still should converge, just slower...