flatironinstitute / bayes-kit

Bayesian inference and posterior analysis for Python
MIT License
41 stars 3 forks source link

normalize acor[0] to be 1? #14

Closed magland closed 1 year ago

magland commented 1 year ago

https://github.com/flatironinstitute/bayes-kit/blob/8f8c494cf65ce39f0982566b54ed1a9a277f8c82/bayes_kit/ess.py#L120-L135

I might be missing something here, but I think you need to normalize acor so that acor[0] is 1.

bob-carpenter commented 1 year ago

acor[0] will be 1 as calculated by my FFT function, but for some reason, the autocorr_np is no longer giving me anything like the same answers. I just removed it.

I just verified against running in R that the FFT implementation is correct. I'm adding unit tests now. And indeed, I need to trim down the result.

bob-carpenter commented 1 year ago

Also, if I don't include the size argument in the FFT, I don't get the right answer.

magland commented 1 year ago

Hmmmm, well if you do

autocorr_fft([1, 0, 0, 0])

you get

[ 1.00000000e+00 -8.33333333e-02 -1.66666667e-01 -2.50000000e-01
  7.40148683e-17 -2.50000000e-01 -1.66666667e-01 -8.33333333e-02]

whereas I would expect to get [1, 0, 0, 0, ...]

bob-carpenter commented 1 year ago

That's the right answer according to R:

> z <- c(1, 0, 0, 0)
> acf(z, pl=FALSE)

Autocorrelations of series ‘z’, by lag

     0      1      2      3 
 1.000 -0.083 -0.167 -0.250 
> 

I also fixed so it only returns a vector of the same size as the input, so the output exactly matches R's results now. And I forgot to mention that the pl=FALSE is just so that it doesn't automatically pop up a plot (when I said I didn't like most R interfaces, this is a good example).