maho3 / ltu-ili

Robust ML in Astro
https://ltu-ili.readthedocs.io/en/latest/
43 stars 8 forks source link

32 fix snle #54

Closed maho3 closed 1 year ago

maho3 commented 1 year ago

Fixes SBIRunner to allow for SNLE and SNRE models. Solves #32

maho3 commented 1 year ago

Results of the new SNLE integration shows robustness for the toy example: plot_single_posterior coverage predictions rankplot

maho3 commented 1 year ago

Added emcee samplers for sbi modules, and integrated it in with a modular interface in the validation step. Now, users can choose which sampler (emcee or sbi/Pyro) they want to use for each metric in the validation phase. This could be useful if you want high resolution sampling for a single example, but low resolution sampling for the entire test set (to check posterior coverage).

Here's an example posterior constraint from using emcee on an SNLE model. plot_single_posterior copy

This is built to work fluidly with the pydelfi version, but hasn't yet been tested. It should address #47 if everything works well.

maho3 commented 1 year ago

Pydelfi version works with emcee! Here's an example:

plot_single_posterior (3)

maho3 commented 1 year ago

This is another fairly extensive PR, so here's a summary of major changes:

And here are some minor changes:

Questions:

A non-exhaustive list of things to test:

CompiledAtBirth commented 1 year ago

(sbi backend only) On a new Python 3.10.12 environment with ili-summarizer also installed, SNRE, SNPE and SNLE work without issue in the toy_sbi example. Changing the _samplemethod attribute to emcee instead of "slice_np_vectorize" for the utils.samplers instances also runs without apparent issue. Next I'm going to look at the actual posterior coverage, try to change the configurations and check the type A/B/C instances (toy_sbi.py only for now).

CompiledAtBirth commented 1 year ago

Temporary workarounds with emcee sampler instances invoked in metrics.py:

CompiledAtBirth commented 1 year ago

[sbi backend] Hello! I do not understand the second major bullet point, namely that emcee- and pyro-backed MCMC samplers are available for SNLE or SNRE instances.

  1. Did I miss where we modify the sampling backends for SNRE/SNLE models? Currenlty ili.utils.samplers is not imported in ili/inference/*.py
  2. If not, do we want to run with the default sbi behavior when dealing with trained likelihood(-ratio)s, or should we include the possibility to use MCMC(emcee or pyro) , VI or rejection as in sbi?
maho3 commented 1 year ago

Hey Nicolas, thanks for looking into this.

Yes so here are the answers to some questions:

CompiledAtBirth commented 1 year ago

Hello, moving now to testing the toy examples with the pydelfi backend on a fresh Python==3.6.0 environment. Have you installed the version of this PR as such ? There is no compatible version of "mpsort" when installing ili-summarizer, looking into it.

DeaglanBartlett commented 1 year ago

When do you get this issue? If I run

git clone -b 32-fix-snle git@github.com:maho3/ltu-ili.git
git clone https://github.com/florpi/ili-summarizer.git
conda create -n ili-pydelfi python=3.6 -y
conda activate ili-pydelfi
pip install -e "ltu-ili[pydelfi]"
pip install -e ili-summarizer

then everything seems to install fine. Does this happen at installation stage or when you run a particular script?

CompiledAtBirth commented 1 year ago

It does work with that way actually, the fault was me being stubborn on using a venv ==3.6 instead of conda and asking for trouble (installation stage) Thank you

CompiledAtBirth commented 1 year ago

Question

Answer about MCMC for SNPE (sbi) If sbi already allows MCMC sampling for trained posteriors from SNPE instances, we should include this possibility as well.

Some notes before finishing tests for review

maho3 commented 1 year ago

Hey Nicolas,

Thanks a lot for the detailed review. Some notes:

maho3 commented 1 year ago

Hey Nicolas,

Additonal notes: