search

dragozzine / multimoon

fit solar system small body multiple systems with non-Keplerian orbits including quadrupole shapes
MIT License
5 stars 0 forks source link

Wrong prior probability formulas #8

Closed benp175 closed 4 years ago

benp175 commented 4 years ago

https://github.com/dragozzine/multimoon/blob/28541c9f65a665d1e31ff83119ae739a01fb2a25/src/mm_priors.py#L44

In mm_priors, the formulas for calculating the probability are wrong. Shown above is the equation for the uniform prior. It assigns a probability of 1 with it being inside of the uniform distribution. The formula should actually come from PDF for the uniform distribution (as seen here: https://en.wikipedia.org/wiki/Uniform_distribution_(continuous)). I think this may also be a problem in other probability distributions.

dragozzine commented 4 years ago

I'd have to check, but I'm not sure these are very wrong. First of all, I think Dallin's code uses log likelihoods where log(1)=0 so he just ignores it if it is in range. Second, we're actually most interested in the relative likelihood, so really the uniform distribution just needs to return -np.inf if it is out of range and everything else doesn't have deep meaning. Still, no reason not to give the correct log likelihood if it is easy to calculate. Good idea for Dallin (and maybe Ben) to double-check.

benp175 commented 4 years ago

That's a good point. Since relative probabilities are all that matters, it doesn't matter in the end if you include the prior or not, if the prior probability is the same among all the parameters. I'll close the issue for now since it's not actually a problem, but I'll make sure to go back and look it to make sure everything is correct.