njzifjoiez
commented
9 months ago I have pushed two commits (2111ea4 and eb7581) this morning with some modifications regarding your comments, mainly changing the Theta notations, improving the docstring and moving out some functions of the main sampler.
I messed up a bit, and the change is split between both commits. There is one comment I didn't get about the docstring, but I will continue on it in the specific thread. I haven't advanced on the unit tests for the different functions, I will look at that next. Let me know if there are more things to do about some notations/docstring that would need to be change.
Note: I added by mistake a python file in the first commit to parse some test results; I removed it; it's not related to the change here.
JohannesBuchner
I did some updates before creating the merge request. I changed the names of the function for PopulationSimpleSliceSampler as a working name, and I made some modifications regarding your second question so that the default setup is, I think, theoretically valid.
To answer your questions:
-> I selected different thresholds, Lmin or a likelihood above based on the points. Setting everything to Lmin also works. It was just an ad-hoc modification because ending the run was a bit faster. I remove it, I was already hesitating to do it.
-> yes indeed there is no stepping out procedure, the idea is more to have an interval that is larger than the area above Lmin so we don't have to steppout. If there is no scale adaptation, the idea is to have a slice that will reach the bound of the unit cube, so it is the type 1) interval of section 4.1 of Neal's paper. In the case of adaptation, the idea is to try to reduce it to fit the current regions so it would keep decreasing as the sampler focuses on a smaller area of the parameter space. So the current code would need some adaptations to adjust this behaviour when ultranest is re-sampling areas of lower likelihood, maybe a reset in the scale or something like that. The jitter is here to balance an adaptation that would be too strong and impose a too small slice width. Though I agree with you, I don't think that it preserves the detailed balance with the jitter, but I am also not sure that it is preserved with the scale adaptation. I kept them because the runs tend to end faster when activated, but I am not sure of how robust it is. So, I changed the default setup to the constant slice width.
To give some insight into the performance, I ran some tests this morning on my laptop with a 20d rosenbrock problem. I ran the default setup for my sampler, the normal slice sampler and the current population one, and here are the last printing of the runs:
My sampler:
time -p python test_PopSliceSampler.py --x_dim 20 --log_dir test --rosenbrock --seed 4 --SimSlice --popsize 50 --nstep 100 --direction 1 --num_live_points 600Current population slice sampler
time -p python test_PopSliceSampler.py --x_dim 20 --log_dir test --rosenbrock --seed 4 --PopSlice --popsize 50 --nstep 100 --direction 1 --num_live_points 600Normal slice sampler:
time -p python test_PopSliceSampler.py --x_dim 20 --log_dir test --rosenbrock --seed 4 --Slice --popsize 50 --nstep 100 --direction 1 --num_live_points 600I made the same run with the adaptation of the slice size (adapt factor of 0.9) and another with the same adaptation+jitter. So, there may not be a correct sampling strategy as their logZ is higher than the three other runs, but the runs were faster.
Adaptation only:
Here with the jitter+adaptation