pymc-devs / pymc

Bayesian Modeling and Probabilistic Programming in Python
https://docs.pymc.io/
Other
8.48k stars 1.97k forks source link

Implement `Ordered` distribution factory #7297

Open ricardoV94 opened 2 months ago

ricardoV94 commented 2 months ago

Description

For univariate IID, adding a transform=ordered is equivalent to sorting the raw draws (forward pass). The logp is proportional to the density of the original draws + ordered transform jacobian. We would just need to figure out the normalization constant so it defines a proper multivariate variable that integrates to 1.

With this users would have a generative graph for ordered variables, that they can also do prior_predictive sampling from. Also default initvals would work out of the box. Right now users always need to provide them.

This would also pretty much also obliviate the need for default_transform and transform that we added in #5674 and simplify the API.

The normalization constant is probably just size!: https://en.wikipedia.org/wiki/Order_statistic#The_joint_distribution_of_the_order_statistics_of_an_absolutely_continuous_distribution

The API could look something like:

with pm.Model() as m:
  x = pm.Ordered("x", pm.Normal.dist(), shape=(3,))
michaelosthege commented 1 month ago

Just stressing (citing you) that

Transforms have no role in forward sampling methods (prior/posterior predictive)

from https://github.com/pymc-devs/pymc/issues/7040#issuecomment-1831724024 because we just got startled by this (again).

ricardoV94 commented 1 month ago

Just stressing (citing you) that

Transforms have no role in forward sampling methods (prior/posterior predictive)

from #7040 (comment) because we just got startled by this (again).

This is now emphasized in https://www.pymc.io/projects/docs/en/latest/api/distributions/transforms.html

With the distinction between transform and default_transform we can also start emitting warnings in prior/posterior predictive sampling like we do with Potentials, that non default transforms will be ignored

AlexAndorra commented 3 weeks ago

Also default initvals would work out of the box. Right now users always need to provide them.

What do you mean by that @ricardoV94 ? I have definitely had cases where it worked without specifying initivals, IIRC

ricardoV94 commented 3 weeks ago

Also default initvals would work out of the box. Right now users always need to provide them.

What do you mean by that @ricardoV94 ? I have definitely had cases where it worked without specifying initivals, IIRC

Then you were lucky, or had a non-iid prior like mu=[-1, 0, 1]

AlexAndorra commented 3 weeks ago

Aaaah right, makes more sense now -- I indeed always order the mus. Thanks Ricardo ;)

ricardoV94 commented 3 weeks ago

Note that my suggestion above only works for iid components. It wouldn't accept stuff with different mus for example