Skip to content
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

transforms.ordered doesn't apply to samples found with .random() #4213

Closed
EdAyers opened this issue Nov 9, 2020 · 5 comments
Closed

transforms.ordered doesn't apply to samples found with .random() #4213

EdAyers opened this issue Nov 9, 2020 · 5 comments

Comments

@EdAyers
Copy link

EdAyers commented Nov 9, 2020

Please provide a minimal, self-contained, and reproducible example.

Modified version of example given at https://docs.pymc.io/api/distributions/discrete.html?highlight=ordered#pymc3.distributions.discrete.OrderedLogistic

import pymc3 as pm
import numpy as np
with pm.Model() as model:
    cutpoints = pm.Normal("cutpoints", mu=[-1,1], sigma=10, shape=2,
                          transform=pm.distributions.transforms.ordered, testval=np.asarray([.5, .9]))
    for i in range(10):
        c = cutpoints.random()
        print(c, c[0] < c[1])

Please provide the full traceback.

[-1.78978722  2.24029602] True
[ 7.27625258 -9.98526886] False
[-2.3456616   6.17737276] True
[-12.42092868  -2.94677656] True
[0.8365788  9.45209727] True
[-12.07594385  13.38498445] True
[ 4.94704378 17.77777978] True
[7.0118523  7.82261754] True
[ -3.387632   -10.23571222] False
[ 8.48851098 -3.25975408] False

Expected result: c[0] is always < c[1]

However the full demo given at the URL works fine. So I guess that one of the members of the Transform subclass has a bug in it?
Or maybe I misunderstood the API and when you call X.random() you are supposed to see the sample before the transform?
It seems quite likely this is a bug but apologies if this is actually correct behaviour.

Here is another one where someone is sampling an ordered pair but no output.
#3680

Versions and main components

  • PyMC3 Version: 3.9.3
  • Theano Version: 1.0.5
  • Python Version: 3.8.6
  • Operating system: Arch Linux
  • How did you install PyMC3: pip
@junpenglao
Copy link
Member

Thanks for reporting - we need a pm.Sort class similar to pm.Bound to solve this properly (I had a twitter thread for this actually https://twitter.com/junpenglao/status/1263927315194576898)

@ricardoV94
Copy link
Member

Could we resort to some costly rejection sampling in here? Or mix rejection sampling and truncated sampling for those distributions we have cdf / inverse cdf methods.

@junpenglao
Copy link
Member

junpenglao commented Mar 17, 2021

i dont think that's necessary - just need to sort the forward simulation

@ricardoV94
Copy link
Member

ricardoV94 commented Mar 17, 2021

i dont think that's necessary - just need to sort the forward simulation

That only works if the sorted distributions are identically distributed

@ricardoV94
Copy link
Member

ricardoV94 commented Jan 10, 2022

Random forward methods in general don't (and can't) respect transforms that change the density of a distribution (they are not even aware of them). We can create distributions that generate forward samplers that are equivalent to the transformed distribution that is sampled by MCMC, but otherwise users shouldn't expect the two forms of sampling to match when using non-default transformations. For the case of sorting we would need something like aesara-devs/aeppl#44

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

No branches or pull requests

3 participants