BradGreig / Hybrid21CM

1 stars 3 forks source link

QSO prob < 0 #21

Closed steven-murray closed 5 years ago

steven-murray commented 5 years ago

Referring to this code:

https://github.com/BradGreig/Hybrid21CM/blob/5cca1eb3548bc483e94b2ef2f84f40628b8af354/src/py21cmmc/mcmc/likelihood.py#L736

When we get a prob of less than zero, should we not rather just return -inf, rather than some arbitrary "small number"?

BradGreig commented 5 years ago

I think the argument here is that it might be a numerical artefact of the interpolation rather than an actual zero probability. The PDF from the prior is not defined at xHI = 0, so an extrapolation of the splined PDF might is required. A very small number at least allows it to be probabilistically possible rather than completely impossible. I'd have to look at it closer to properly decide. However, it shouldn't really matter which is used.

steven-murray commented 5 years ago

Hmm wonder if extrapolating in log space might be a good option. That way it's definitely positive.

On Tue, Dec 11, 2018, 4:02 PM BradGreig <notifications@github.com wrote:

I think the argument here is that it might be a numerical artefact of the interpolation rather than an actual zero probability. The PDF from the prior is not defined at xHI = 0, so an extrapolation of the splined PDF might is required. A very small number at least allows it to be probabilistically possible rather than completely impossible. I'd have to look at it closer to properly decide. However, it shouldn't really matter which is used.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BradGreig/Hybrid21CM/issues/21#issuecomment-446395936, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNo3hFsg4LDW_DU_rurj4BHyAulK4Oaks5u4DlfgaJpZM4ZOBHW .

BradGreig commented 5 years ago

Ok, looking at the data, it has already been interpolated/extrapolated. However, for the first entry (nf = 0) the PDF is currently negative, and the last entry (nf=1) the PDF is zero.

I agree that interpolating in log space is better, but these datapoints need to be updated. You can just switch the first value to positive, and set the last value to be the same as the first (as it is vanishingly small). Once that is done, switching to log sampling will be fine, and QSO_prob < 0 can never occur.

steven-murray commented 5 years ago

OK I fixed this in-code (rather than changing data) by constructing the spline only from the positive values, and then using a log-spline. See e159edea4db4b55801f41a7c0b2eab86bb3ef25b.