PTMCMCSampler
PTMCMCSampler copied to clipboard
Possible broadcasting bug in version 2.1.1
I noticed that I'm getting a new broadcasting error message which I think coincides with the recent version update to PTMCMCSampler. I wrote a very simple toy model problem with only three parameters, and I do not get the following error when using version 2.0.0. However, when using 2.1.1, the following bit of code:
injection = [2, 5, 0.5] burnin = 100 it = 10000 sampler.sample(np.asarray(injection), it, burn=burnin, thin=1, SCAMweight=1, AMweight=1, DEweight=1)
Now returns the following error message:
ValueError Traceback (most recent call last)
in 2 it = 10000 3 ----> 4 sampler.sample(np.asarray(injection), it, burn=burnin, thin=1, SCAMweight=1, AMweight=1, DEweight=1) #covUpdate=500, 2 frames /usr/local/lib/python3.7/dist-packages/PTMCMCSampler/PTMCMCSampler.py in _updateDEbuffer(self, iter, burn) 756 757 self._DEbuffer = shift_array(self._DEbuffer, -len(self._AMbuffer)) # shift DEbuffer to the left --> 758 self._DEbuffer[-len(self._AMbuffer) :] = self._AMbuffer # add new samples to the new empty spaces 759 760 # SCAM jump
ValueError: could not broadcast input array from shape (1000,3) into shape (100,3)
I am not up to speed on recent changes. @AaronDJohnson, @paulthebaker any help here?
It looks like my DEbuffer
change is not behaving properly when modifying the burn kwarg. I'll investigate.
The issue is that normally burn
> covUpdate
, and I had assumed this would be the case. This is because burn only sets the size of _DEbuffer
and when the neff
calculations start happening. 100 might be a large enough buffer size for such a small sampling problem (so that DEbuffer is still much larger than the autocorrelation length which will surely be quite small here).
The question is what should be done in this scenario. There are two options that I think are viable: thin the extra samples from AMbuffer so that they fit in the smaller DEbuffer, or require burn
to be larger than covUpdate
.
@paulthebaker what do you think?
Hi @AaronDJohnson , I see, thanks for checking this! Honestly as I said, I was testing just a super simple toy model problem just to verify I had set everything up correctly and to see if the sampler was working. So I had very low values for the burn and number of iterations because I just wanted it to finish quickly. In my real-world problem that I'm building up to these will very likely be much larger and will probably naturally meet the burn
> covUpdate
criterion.
That said, I'll leave it to you guys to decide how best to adjust for this small bug, but honestly one option is to simply add to the docstring a notation that burn
> covUpdate
, or add a check for this condition and output an error message if it is not met (along the lines of what your second solution idea was).
I'd say just add a check and raise a ValueError
if burn < covUpdate
in the __init__
. @CaseyMcGrath you can make a PR with this change if you want