-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Importance sampling unexpected behaviour [and plot gallery broken] #132
Comments
Hi Stefan, the thing with nested sampling is that you need to watch out that the In any case, changing the |
Hi Stefan, You should be able to get the behavior you're looking for by loading samples as/turning samples into MCMCSamples and then reweighting. The critical difference here is that at the moment you are reweighting NestedSamples into another valid nested sampling run (with all the advantages e.g. can compute evidences and KL divergences). Suffice to say that this process is non-trivial and Lukas and I plan to write this up in the near future so that the theory behind this is made much clearer (rather than being hidden across several pull requests). Best, |
Just to be sure we’re on the same page, in general, we expect this function to alter the weights. But in the specific case of adding a constant to the log-likelihood, we expect no change (up to normalisation). Yet your plot shows something else going on. Indeed it might be the logL births subtlety @lukashergt mentioned. I don’t remember why logL_birth isn’t updated simultaneously as logL. Was that a choice? Or is it a bug? Why shouldn’t Stefan’s code do logL += 0.1 and logL_birth += 0.1? Leaving the weights unchanged (up to normalisation) |
@lukashergt Thanks, that explains the cutoff. I'd still expect the weights to approximately stay the same though, at least for the +0.1 case. (But I might well be wrong, noticing I don't know about all the Nested Sampling details.) @williamjameshandley Thanks! I don't need the evidences for this part of my analysis so converting to MCMCSamples works and then importance sampling gives the expected results. |
@Stefan-Heimersheim What exactly is the first plot showing? How did you generate it? Normally the samples should be ordered by weights if I am not mistaken... |
I just took them in the order |
@Stefan-Heimersheim Importance sampling only works in the bulk posterior region of parameter space (that goes for both nested sampling and MCMC. With a Gaussian As an example I'd try maybe with a mean |
The x axis is the index of the weight list, y axis is the weight value. Sorry I was lazy omitting the x axis argument in the original post, here is the equivalent line:
Edit: Original code
|
Same. (I would still expect a shift towards the left in the former case)
|
In the last example the |
Just talked to @lukashergt, it seems my chains might be a bit weird due to cobaya's treatment of non-uniform priors (see this pull request) and that might be causing some of the issues with NestedSamples. The importance sampling function of MCMCSamples seems to be wrong, missing the parts where weights are changed. I'll make a separate bug report for that. |
Hi @Stefan-Heimersheim. Many thanks for this extremely useful working example, which demonstrates the issue unambiguously. This does show that there is clearly a problem with the importance sampling method for nested samples. In theory the evidence should work, but the fact that contour-wise grey!=red means that there is a deeper problem. This is something @lukashergt and I have discussed in passing (e.g. it's kind of clear that the current approach would fail if one added a constant log-likelihood). I will look into this in the next two weeks, as I have a good idea as to how one fixes this problem -- in analogy to how nested sampling can compute evidences and posteriors at different thermodynamic temperatures, but working this into an importance sampling framework is a small project which I'll have time for in april. In the meantime, your approach using the posterior average of the importance weights is both mathematically and functionally correct, and is in the spirit of nested sampling that a base run can give you the overall normalisation, and then the importance sampled run giving you the adjusted Bayes factor for the new likelihood. If you want to get the errors on this, this will be dominated by the evidence error, which can be computed as usual by sampling using |
Very strange, I did test something very similar in the past, getting this plot: where the Unfortunately the mock data for the corresponding notebook is still on a ship on the Atlantik... so I can't rerun this, yet. But this could indicate that this worked as expected in the past, but was broken subsequently...? Could the |
@lukashergt, this came up in conversation with @htjb this afternoon -- did you get a chance to investigate the discrepancy? |
This sounds plausible. class NestedSamples(MCMCSamples):
# ...
def importance_sample(self, logL_new, action='add', inplace=False):
# ...
samples = super().importance_sample(logL_new, action=action) Previously |
Sorry I didn't get around to this sooner. I just did a quick mock test (anesthetic version 2.0.0-beta.12), and my impression is that things still work. I've used the following likelihoods, once only likelihood:
L1: 'lambda x, y: stats.multivariate_normal.logpdf([x, y], mean=[ 0, 0], cov=1) the other time both likelihood:
L1: 'lambda x, y: stats.multivariate_normal.logpdf([x, y], mean=[ 0, 0], cov=1)
L2: 'lambda x, y: stats.multivariate_normal.logpdf([x, y], mean=[0.5, 0], cov=0.25) I then importance sample the run that only used ns3 = ns1.importance_sample(logL_new=stats.multivariate_normal.logpdf(ns1[['x', 'y']].values, mean=[0.5, 0], cov=0.25),
action='add',
inplace=False) Orange and green should and do agree: I haven't taken a closer look at @Stefan-Heimersheim's example, yet. I might find some time later today. |
Huh, that's curious. Turns out that these test cases depend a lot on the choice of scale/std... I've reduced the example to two parameters. Case 1: this works fine2D-Gaussian with
|
Thanks for those experiments @lukashergt! Can you check if Case 2 behaves similarly if you convert the |
Nice test! And indeed, converting the nested samples from Case 2 to MCMC samples and then doing the importance sampling gets you to the correct posterior: mc7 = MCMCSamples(data=ns7[['a', 'b']], logL=ns7.logL)
mc8 = MCMCSamples(data=ns8[['a', 'b']], logL=ns8.logL)
mc9 = mc7.importance_sample(logL_new=stats.multivariate_normal.logpdf(ns7[['a']].values, mean=[0], cov=0.1**2),
action='add',
inplace=False) So it looks like it isn't a numerical issue... But then why does |
Describe the bug
NestedSamples.importance_sample changes weights in a weird way. E.g. importance sampling by a constant offset of
logL_new=0.1
changes the weights which it should not (I think)a=chain.importance_sample(0.1, action="add")
. This change of weights influences posteriors, mean, std etc. I think the easiest is to look at the weights.Resampling with a negative offset also appears to shorten the chain, I would not expect that either.
I might just be misunderstanding the importance sampling procedure in Nested Sampling, but this is not at all what I expected from e.g. MCMC samples. Edit: My original goal was using importance sampling to understand which of my likelihoods produces which constraints on my parameters, this simple "add 0.1" example was just the minimal thing behaving weirdly.
To Reproduce
Expected behavior
I would expect the weights to stay the same, except for re-normalization. Instead they change.
Screenshots
Here I just simply plot the weights.
Additional context
My samples have been generated with cobaya running pypolychord, then loading the "_polychord_raw" directory with anesthetic. I tried both the current git master branch and version python-anesthetic-git r319.2351380-2 from the AUR).
I tried to reproduce the issue with the sample file from the plot gallery but the code there does no longer work:
returns
RuntimeError: Not a valid nested sampling run. Require logL > logL_birth.
The text was updated successfully, but these errors were encountered: