Replies: 16 comments 3 replies
-
That's a really neat idea. I strongly support it. Could you mock up an example Distribution to see what it would like in code? An alternative would be to add new distributions e.g. |
Beta Was this translation helpful? Give feedback.
-
I support the idea of adding a new distribution. Additional parameter is not always visible (especially for new users). |
Beta Was this translation helpful? Give feedback.
-
As to parameter vs new class: The implementation could look something like this: class AffineTransform(pm.distributions.transforms.ElemwiseTransform):
name = 'affine'
def __init__(self, loc, scale):
self.scale = scale
self.loc = loc
def forward(self, x):
return (x - self.loc) / self.scale
def backward(self, y):
return self.scale * y + self.loc
def jacobian_det(self, y):
return tt.log(self.scale)
class Normal_(pm.Normal):
def __init__(self, mu, sd, parametrization='centered', *args, **kwargs):
if parametrization == 'centered':
transform = None
elif parametrization == 'non-centered':
transform = AffineTransform(mu, sd)
else:
raise ValueError('Unknown parametrization: %s' % parametrization)
super().__init__(mu, sd, *args, **kwargs, transform=transform) We could also take advantage of the fact that is easier to compute the logp in the untransformed space in this case (using inheritance instead of changing the original implementation for clarity): class TransformedDistribution(pm.distributions.transforms.TransformedDistribution):
def __init__(self, dist, transform, untransformed_dist, *args, **kwargs):
super().__init__(dist, transform, *args, **kwargs)
self.untransformed_dist = untransformed_dist
def logp(self, x):
if self.untransformed_dist is not None:
return self.untransformed_dist.logp(x)
else:
return super().logp(x)
class UnitNormal(pm.Continuous):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.mean = self.mode = self.median = 0
self.variance = 1
def logp(self, x):
return - 0.5 * (x ** 2 + tt.log(2 * np.pi))
class AffineTransform(pm.distributions.transforms.ElemwiseTransform):
name = 'affine'
def __init__(self, loc, scale, untransformed_dist=None):
self.scale = scale
self.loc = loc
self.untransformed_dist = untransformed_dist
def forward(self, x):
return (x - self.loc) / self.scale
def backward(self, y):
return self.scale * y + self.loc
def jacobian_det(self, y):
return tt.log(self.scale)
def apply(self, dist):
return TransformedDistribution.dist(dist, self, self.untransformed_dist)
class Normal_(pm.Normal):
def __init__(self, mu, sd, parametrization='centered', *args, **kwargs):
if parametrization == 'centered':
transform = None
elif parametrization == 'non-centered':
transform = AffineTransform(mu, sd, UnitNormal.dist())
else:
raise ValueError('Unknown parametrization: %s' % parametrization)
super().__init__(mu, sd, *args, **kwargs, transform=transform) |
Beta Was this translation helpful? Give feedback.
-
I would vote for a subclass of |
Beta Was this translation helpful? Give feedback.
-
I am with @fonnesbeck on this one and think it's a great idea. |
Beta Was this translation helpful? Give feedback.
-
A lack of a non-centered MvNormal is a bit of a blocker for me on the gp stuff at the moment. I wouldn't mind taking this on. Has anyone been working on this? |
Beta Was this translation helpful? Give feedback.
-
Not really. I've been working a bit on a refactoring of the distribution base class and the transformations, and I'm hoping that this will pretty much write itself with that. But I don't know if this refactoring will ever see the light of day. |
Beta Was this translation helpful? Give feedback.
-
I suggest the following: Create a new class in I still firmly believe they should be a new class of distribution instead of just a kwarg inside of the Distribution we already have. As different parameterization is actually a different model and should be treated differently. |
Beta Was this translation helpful? Give feedback.
-
This is still a cool idea. |
Beta Was this translation helpful? Give feedback.
-
Super-useful. But don't forget NonCenteredTruncatedNormal. Even @aseyboldt's original proposal is useful, as it allowed me to check my parameterization of a non-centered lognormal. |
Beta Was this translation helpful? Give feedback.
-
@bridgeland Want to do a PR with @aseyboldt's code? Would be super useful. |
Beta Was this translation helpful? Give feedback.
-
Indeed. But I am not the right guy, at least not yet. I am still a pymc3 noob. |
Beta Was this translation helpful? Give feedback.
-
@bridgeland Fair enough, but note that this is the best way to learn and we'd help getting it through. |
Beta Was this translation helpful? Give feedback.
-
Per meeting discussion symbolic pymc has this as its use case but there still might be value in having this natively in PyMC |
Beta Was this translation helpful? Give feedback.
-
What could we do here now with aeppl? CC @ricardoV94 @brandonwillard |
Beta Was this translation helpful? Give feedback.
-
This was always an API issue, not a real backend limitation. As @aseyboldt showed, this could be easily achieved with an |
Beta Was this translation helpful? Give feedback.
-
Non-centered parametrizations are often a better choice than the centered parametrizations that are more natural to code in pymc3:
vs
Couldn't we add a parameter
parametrization
or similar to distributions, such that we can automate that? This would also make it easier to for new users to try a couple of different parametrizations if something does not work well. I think we could reuse the framework we have in place for transformations. All we need for the non-centered parametrization is an affine transformation.Some useful candidates:
Do you think this would be useful / would work? Other ideas for common reparametrizations (or better names ;)?
Beta Was this translation helpful? Give feedback.
All reactions