beast-dev / beast-mcmc

Bayesian Evolutionary Analysis Sampling Trees
http://beast.community
GNU Lesser General Public License v2.1
192 stars 73 forks source link

bound at infinity for rootHeight prior in BEAUti #990

Closed GuyBaele closed 6 years ago

GuyBaele commented 6 years ago

Steps to reproduce: load data into BEAUti, go to Priors panel and try to alter the prior on the rootHeight to e.g. an exponential prior. By default, the truncate option is selected with an upper bound at +infinity, which will generate the following error when running the XML:

Error parsing '' element with id, 'null': Uniform prior uniformPrior cannot take a bound at infinity, because it returns 1/(high-low) = 1/inf Error thrown at: dr.inferencexml.distribution.PriorParsers$1.parseXMLObject(Unknown Source)

I suggest to no longer allow specifying +infinity as an upper or lower bound for (truncated/uniform) priors and hence for BEAUti not to allow generating the resulting XML until a different bound has been entered/selected.

rambaut commented 6 years ago

I guess the issue is using the uniform as a way of truncating another density. It should be valid to have the product of an improper uniform and a proper other density - it is just the normalization should be on the product. This is done using a TruncatedDistribution which wraps another distribution.

rambaut commented 6 years ago

So 1) the prior on root height should not be truncated by default.

2) If we are using a Uniform prior to do the truncation then we cannot allow Infinite bounds in BEAUti.

3) However, we could allow it by adding the truncation to a proper prior using TruncatedDistribution (this is what is internally used by BEAUti). We should probably take this option because the density will be correctly normalized (unlike using a separate uniform prior).

rambaut commented 6 years ago

Fixed using 3)