guillochon / MOSFiT

Modular Open Source Fitter for Transients
http://mosfit.readthedocs.io/
MIT License
41 stars 53 forks source link

add ultranest as a sampler #211

Closed JohannesBuchner closed 1 year ago

JohannesBuchner commented 2 years ago

This is connects MOSFiT with ultranest.

UltraNest is a open-source nested sampling package which can fit a wide variety of models. It is documented at https://johannesbuchner.github.io/UltraNest/

This PR enables basic functionality, including

The dynamic nested sampling features described in the "Baselining and batching" are also implemented in ultranest, with the reactive nested sampling generalisation of dynamic nested sampling. The desired accuracy on logz can be set with the dlogz parameter, the desired number of effective samples can be set with the min_ess parameters, when calling sampler.run().

For difficult and very high-dimensional problems (20d+), a step sampler can lead to faster convergence. This can be done with:

self._sampler.stepsampler = ultranest.stepsampler.SliceSampler(
    nsteps=nsteps,
    generate_direction=ultranest.stepsampler.generate_mixture_random_direction,
)

and connecting the number of steps parameter to a command line argument. I did not include this in this PR, because I wanted to get your opinion on the contribution first.

The documentation states that there is interest to drop MCMC in favor of nested sampling. If this is the case, the MOSFiT code could be simplified substantially.

guillochon commented 2 years ago

I think this is fine to merge when the github workaround part is removed from the PR. Could you do that? Thanks!

JohannesBuchner commented 2 years ago

should be okay now

mnicholl commented 2 years ago

A couple of things that could be improved before merging:

mnicholl commented 1 year ago

Hi Johannes, I think this is nearly ready to merge. I see some places in main.py that refer to method == 'nester' or method in ('nester', 'ultranest'). I think this would fail with dynesty -- the sampler is still called nester but the method is now named dynesty, right?

Can you make sure the code still runs with -D dynesty, and change the above if necessary? Thanks!

JohannesBuchner commented 1 year ago

I am getting a None error with dynesty, but I am not sure this is related to this PR. I didn't try dynesty before I started the PR, so it may already have been broken (or it is my setup).

207 measurements, 12 free parameters.
Scale history:
[]
Traceback (most recent call last):
  File "/mnt/data/daten/PostDoc2/home/Downloads/MOSFiT/mosfit/samplers/nester.py", line 127, in run
    for li, res in enumerate(sampler.sample_initial(
  File "/home/user/.local/lib/python3.10/site-packages/dynesty/dynamicsampler.py", line 806, in sample_initial
    self.live_u, self.live_v, self.live_logl = initialize_live_points(
  File "/home/user/.local/lib/python3.10/site-packages/dynesty/dynamicsampler.py", line 369, in initialize_live_points
    for i, logl in enumerate(live_logl):
TypeError: iteration over a 0-d array

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/user/.local/bin/mosfit", line 33, in <module>
    sys.exit(load_entry_point('mosfit', 'console_scripts', 'mosfit')())
  File "/mnt/data/daten/PostDoc2/home/Downloads/MOSFiT/mosfit/main.py", line 977, in main
    Fitter(**fitargs).fit_events(**fitargs)
  File "/mnt/data/daten/PostDoc2/home/Downloads/MOSFiT/mosfit/fitter.py", line 474, in fit_events
    entry, p, lnprob = self.fit_data(
  File "/mnt/data/daten/PostDoc2/home/Downloads/MOSFiT/mosfit/fitter.py", line 592, in fit_data
    self._sampler.run(self._walker_data)
  File "/mnt/data/daten/PostDoc2/home/Downloads/MOSFiT/mosfit/samplers/nester.py", line 241, in run
    pickle.dump(sampler.results, open(
  File "/home/user/.local/lib/python3.10/site-packages/dynesty/dynamicsampler.py", line 629, in results
    if self.sampler.save_bounds:
AttributeError: 'NoneType' object has no attribute 'save_bounds'
JohannesBuchner commented 1 year ago

But it does successfully enter the dynesty part.

mnicholl commented 1 year ago

I just checked and dynesty works fine for me with your PR, so indeed the error is probably not related

On 22 Sep 2022, at 11:57, Johannes Buchner @.***> wrote:

But it does successfully enter the dynesty part.

— Reply to this email directly, view it on GitHub https://github.com/guillochon/MOSFiT/pull/211#issuecomment-1254861569, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZ74QVGIKZQSL3COFOKPDDV7Q3RHANCNFSM6AAAAAAQHUT3NE. You are receiving this because you modified the open/close state.