joshspeagle / dynesty

Dynamic Nested Sampling package for computing Bayesian posteriors and evidences
https://dynesty.readthedocs.io/
MIT License
357 stars 77 forks source link

fix the volume determination #224

Closed segasai closed 3 years ago

segasai commented 3 years ago

Hi,

I'm seeing these errors

  /home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/bounding.py:611: RuntimeWarning: invalid value encountered in double_scalars
  self.expand_tot = self.vol_tot / vol_tot_orig
/home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/bounding.py:183: RuntimeWarning: invalid value encountered in double_scalars
  f = np.exp((np.log(vol) - np.log(self.vol)) / self.n)  # linear factor
/home/skoposov/pyenv38/lib/python3.8/site-packages/dynesty/bounding.py:401: RuntimeWarning: invalid value encountered in double_scalars
  self.expand_tot *= vol_tot / self.vol_tot

Withe following variable values from bounding.py: self.vol_tot, vol_tot_orig, expands, self.vols inf inf [1.] [inf]

Digging deeper into this, it could be traced to https://github.com/joshspeagle/dynesty/blob/95d43aa483ff0b058ebba4df04961c198f58d0ba/dynesty/bounding.py#L157 where the volume is computed from the slogdet, while the checks are done based on eigen values

I think the volume needs to be computed based on the eigen values themselves. That is more precise anyway. I fix that in the patch.

In the future I think, it's better to store logs of volumes, but that's beyond this patch.

Cheers, S

joshspeagle commented 3 years ago

I think the volume needs to be computed based on the eigenvalues themselves. That is more precise anyway. I fix that in the patch.

I agree -- looking back, this was a patchwork approach that worked fine in general but was bound to suffer from numerical instability.

In the future I think, it's better to store logs of volumes, but that's beyond this patch.

Agreed. The more things that can be done in logarithms the better.