exoplanet-dev / exoplanet

Fast & scalable MCMC for all your exoplanet needs!
https://docs.exoplanet.codes
MIT License
206 stars 52 forks source link

Allow an upper limit on the eccentricity distribution #96

Closed mrtommyb closed 4 years ago

mrtommyb commented 4 years ago

Is your feature request related to a problem? Please describe. I would like to include an upper limit on the eccentricity (a physical motivation for this could be to prevent crossing orbits

Describe the solution you'd like Ideally the function call would be

xo.eccentricity.kipping13('ecc', upper=0.8)

or

bound_ecc = pm.Bound(xo.eccentricity.kipping13, upper=0.8)
bound_ecc('ecc', upper=0.8)

Describe alternatives you've considered I tried several methods to implement solutions including:

mrtommyb commented 4 years ago

It does look like part of my problem is an upstream issue, e.g.

with pm.Model():
    bound_beta = pm.Bound(pm.Beta, upper=0.3)
    bound_beta('ex', alpha=1.12, beta=3.09, testval=0.01,)
    trace = pm.sample()

fails with SamplingError: Bad initial energy

jiayindong commented 4 years ago

Adding a lower bound to bound_beta may help with the sampling, e.g.,

with pm.Model():
    bound_beta = pm.Bound(pm.Beta, lower=0., upper=0.3)
    bound_beta('ex', alpha=1.12, beta=3.09, testval=0.01)
    trace = pm.sample()

I tested this on my system without the bad initial energy error.

mrtommyb commented 4 years ago

Thanks @jiayindong that does fix that issue. So I think I could get most of what I want by defining my own eccentricity distribution function rather than using xo.eccentricity.kipping13.

jiayindong commented 4 years ago

Thanks @jiayindong that does fix that issue. So I think I could get most of what I want by defining my own eccentricity distribution function rather than using xo.eccentricity.kipping13.

Great! If you want to consider uncertainties on alpha and beta instead of the medians,

alpha_mu = 1.12
alpha_sd = 0.1
beta_mu = 3.09
beta_sd = 0.3
with pm.Model():    
    bounded_normal = pm.Bound(pm.Normal, lower=0.)
    alpha = bounded_normal("alpha", mu=alpha_mu, sd=alpha_sd, testval=alpha_mu)
    beta = bounded_normal("beta", mu=beta_mu, sd=beta_sd, testval=beta_mu)
    ecc = pm.Bound(pm.Beta, lower=0., upper=0.3)('ecc', alpha=alpha, beta=beta)
    trace = pm.sample()

adopted from xo.eccentricity.kipping13.

mrtommyb commented 4 years ago

perfect, thanks!

dfm commented 4 years ago

Thanks @jiayindong!

@mrtommyb: it looks like the other error you were finding was also caused by a missing lower. It looked different because the parameters of the Beta are variables.

Either way, it was easy enough to add so I pushed this feature.