Long ell

[segasai]

This is the code that when inflating the ellipse prevents principal axes from being larger than the half of the cube diagonal. (It still tries to increase the volume to logvol, but by increasing other axes more). That solves the problem in #282 partially, but then moves the problem to the slice sampler. With this patch now the slice sampler hangs. I don't have time now to debug it further, but I'm guessing this is the issue with the volume in sample_batch()

[coveralls]

Pull Request Test Coverage Report for Build 925114901

• 29 of 29 (100.0%) changed or added relevant lines in 3 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.1%) to 77.212%

---

Files with Coverage Reduction	New Missed Lines	%
py/dynesty/sampling.py	1	90.19%
<!--	Total:	1		-->

Totals
Change from base Build 919713254:	0.1%
Covered Lines:	3612
Relevant Lines:	4678

---

💛 - Coveralls

[segasai]

I've set the volumes to zero in multiple places and updated how .expand is treated for bounds so it wouldn't be affected by the scale_to_logvol(). I also had to rewrite the bootstrap check in test_gau to avoid a false-positive fail of the test.

The test in a loop seem to work $ for a in seq 0 36 ; do { echo $a ; env DYNESTY_TEST_RANDOMSEED=$a PYTHONPATH=py:tests:$PYTHONPATH pytest -s tests/test_pathology.py ; } ; done

But I don't feel 100% confident that the setting of volumes to zero is completely harmless.

[joshspeagle]

Great. As for an alternative to setting the volumes to 0, maybe just picking a small value just above the numerical instability threshold could work?

[segasai]

The problem is that I don't have a good understanding where and how the volume used and what it affects, so I dont' really know TBH. specifically I don't know if point volume is somethign that can enter in the calculations of logz, posterior etc

[joshspeagle]

Ah, happy to confirm it doesn't! It's entirely used in constructing the bounds.

[segasai]

Then I would say we should commit this as it is. And then remove all the pointvol arguments from https://github.com/joshspeagle/dynesty/blob/master/py/dynesty/bounding.py and make pointvol as internal constant of correspoding to 1e-308 or something. And in fact I'm not sure we even need that as building an ellipsoid around one point is not something we should be doing at all. From what I remember, one case when I was hitting that is when the batch sampling decides to return the top interval with just one live point in the interval. I think this may need to be dealt in a different way, like returning an interval broad enough to have at least some number of points

[segasai]

Okay I've made the test PR #284 (based on this PR) that gets rid of point volumes completely. All the tests pass, but the test suite slows down and as far as I understand this is due to the removal of this https://github.com/joshspeagle/dynesty/blob/cad81f485c7c3167260c03f7e8b2e165c21e7f6d/py/dynesty/bounding.py#L1566

Basically before the full hierarchical split was considered only if pointvol=0 or the requirement wrt pointvol was satisfied. I am not sure it made much sense, given that pointvol values were somewhat arbitrary, but that clearly prevented that codepath being taken often.

[segasai]

thanks for merging I think if #284 is not merged The half of thsi e100d0727180ad04122237075287cbd7e29eb1b2 and this fe5d5ac9e861959e2799868a3f6221f88c46de2d commits need to be applied here as fixes

[joshspeagle]

I'm in the process of merging that now, so that should be fine in a few minutes.