xpsi-group / xpsi

X-PSI: X-ray Pulse Simulation and Inference
Other
34 stars 21 forks source link

Gsl integration bugfix #300

Closed thjsal closed 1 year ago

thjsal commented 1 year ago

Numerical problems in likelihood computation were fixed for cases with zero counts, preventing also the code from being incorrectly optimized on Intel CPUs when using Intel compilers (producing incorrect GSL integration results and likelihoods). For the tested cases, the effect of these fixes seems non-detectable for the results in the systems where the optimization was already working correctly.

Treatment of the special cases in the likelihood computation in xpsi/likelihoods/default_background_marginalisation.pyx was changed so that taking the logarithm of zero is not allowed anymore. Previously, that could happen if the modelled counts were zero, but the observed counts were not.

In addition, in case they both are zero, we now add 0 (i.e., log(1)) to the log-likelihood, instead of 1 added before.

thjsal commented 1 year ago

To explain a bit more about the additional change in the code: The reason why I think adding 0 (instead of 1) is correct, is that it seems reasonable that the probability for 0 observed counts is 1, and the probability for any other number of observed counts is 0, if the model predicts 0 zero counts. And since we are summing here log probabilities, log(1)=0 should be added and not 1. Does this make sense for others too?