Skip to content

Feature request: Add offset parameter to Exponential class (akin to mu in Laplace class) #4507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jtstanley opened this issue Mar 7, 2021 · 8 comments · Fixed by #6361
Closed

Comments

@jtstanley
Copy link

Could we include an additional parameter to the Exponential class that allows for the whole distribution to be shifted? This would be equivalent to the mu parameter in the Laplace class. Given the similarity of the two distributions (meaningfully differing only by their support), most of the changes could simply be copied from the relevant lines in Laplace.

A usecase was provided by @harcel on the discourse.
See: https://discourse.pymc.io/t/exponential-likelihood-at-different-location/6919

@ricardoV94
Copy link
Member

This could potentially be useful for more distributions other than the exponential

@jtstanley
Copy link
Author

I second @ricardoV94's comment. For example, HalfNormal, HalfCauchy, Gamma, ChiSquared.

@harcel
Copy link

harcel commented Mar 8, 2021

Indeed I was the one suggesting this. I have not yet contributed before, but I would like to, and I think I am capable of fixing this issue (which isn't critical and there probably isn't any hurry, right?). I have found the guidelines for contributing, no issues there, but I would like to have some(-one as) back-up available for noob questions if I commit myself to doing this (imposter and all that...).

@harcel
Copy link

harcel commented Mar 10, 2021

Also, it seems like location parameters are named according to their standard statistical symbol (often mu, sometimes alpha) and never 'loc', like what is sometimes seen in scipy.stats. The Exponential and the distributions mentioned by @jtstanley above don't typically come with a location parameter (but I don't see a reason why not to allow that, and my discourse post showed a use case for when it could be helpful), so I guess we are free to use whatever symbol we like for it. Some unity would be good, so maybe alpha (given that mu is often reserved for the mean) is a good idea. When alpha is taken, 'loc' could be a good alternative.
Changing the location shifts the average, the logp and the logcdf by alpha and leaves the variance unchanged. The code seems pretty easy to adapt.

@ricardoV94
Copy link
Member

ricardoV94 commented Mar 10, 2021

I liked the argument shift in the discourse. Makes it clear what it means, and since it's doesn't have a conventional greek symbol I don't think calling it alpha or something else would be very useful.

One thing that could be considered is having a generic class pm.Shift similar to pm.Bound that wraps around a distribution and gives the shifted version:

shifted_exponential = pm.Shift(pm.Exponential, shift=5)
likelihood = shifted_exponential('likelihood', lam=2, observed=x)

This makes it obvious that we are doing something on top of the "textbook" distribution, and that it does not interact with other parameters that may affect the central location of a distribution.

@jtstanley
Copy link
Author

@ricardoV94 with that construction would one still be able to set shift parameter equal to a model parameter, for sampling purposes? As long as that's possible, I like format.

@ricardoV94
Copy link
Member

ricardoV94 commented Mar 10, 2021

Yes it should be possible. AFAIK the lower and upper parameters in pm.Bound can also be random variables.

If the shift is a one timer it can also fit in one line

b0 = pm.Normal('b0')
b1 = pm.Normal('b1')
likelihood = pm.Shift(pm.Exponential, shift=b0+x*b1)('likelihood', lam=2, observed=y)

@ricardoV94
Copy link
Member

ricardoV94 commented Mar 11, 2021

Checking STAN and TFP it sounds like this would be something akin to an Affine transformation:

https://www.tensorflow.org/probability/api_docs/python/tfp/bijectors/Affine
https://discourse.mc-stan.org/t/linearly-scaled-unconstrained-parameters/5886/18

Which would enable not only arbitrary shifting but also scaling...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants