We depend on a custom numpy implementation of slice sampling

[alvorithm]

Problem

For performance reasons, we are using a home-grown slice sampler programmed by Conor in numpy. We don't know if this is strictly necessary and why.

Solution

Benchmark slice-np against the pyro samplers and determine whether and under which precise circumstances there is a performance gap between slice-pyro and slice-np. If possible, reduce our dependency to Pyro sampling methods, which are more capable and will be externally maintained. @jan-matthis

Follow-up work

• If slice-np needs to stay, then make mcmc_method string be explicit about pyro vs. numpy (e.g. slice_pyro, not slice. Use underscores as they facilitate identification in parameterized pytests).
• Refactor the sampling code, potentially the Posterior object would receive a sampler and not just a potential_function. @meteore

[jan-matthis]

I wrote a basic slice sampler in Pyro that we can consider moving upstream in the future. For the time being, our custom implementation has more features though.

Some notes regarding the Pyro implementation:

• The MCMC kernel of Pyro supports running chains in parallel using multiprocessing (mp)
• With multiprocessing, a context needs to be provided, see https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods
• On macOS, using "fork" as context seems a stable choice, while on Linux, especially in combination with cuda, "spawn" is more stable
• Multiprocessing is disabled in the tests for now, because of difficulties with multiprocessing: While running test_logistic_regression with mp on macOS was fine, running on Linux caused threading errors with fork and pickling errors with spawn
• I've added a custom version of Pyro's MCMC kernel with an extra keyword argument available_cpu: setting available_cpu=1 disables all multiprocessing if num_chains>1

[janfb]

I think this is mostly resolved now @jan-matthis @michaeldeistler : we have a separate sampler interface, and we have extra pyro MCMC methods. The benchmark between them is missing, but I don't see this as an urgent problem.

[michaeldeistler]

I agree, we can close this. The use-case for numpy is quite clearly the vectorization. We might reconsider when pyro adds this feature.