Skip to content

Refactor Multivariate distributions due to new meaning of size #5446

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
ricardoV94 opened this issue Feb 5, 2022 · 5 comments · Fixed by #5474
Closed

Refactor Multivariate distributions due to new meaning of size #5446

ricardoV94 opened this issue Feb 5, 2022 · 5 comments · Fixed by #5474

Comments

@ricardoV94
Copy link
Member

ricardoV94 commented Feb 5, 2022

The next Aesara release will include aesara-devs/aesara#446 which altered the meaning of size for multivariate distributions, to be more consistent with that of univariate distributions.

Size will now specify the number of replications + dimensions implied by batched parameters. It is equivalent to shape[:-ndim.supp]. We will likely need to update our multivariate distributions and/or some tests (e.g. in test_distributions_random.py)

These changes should also be incorporated in #5437

@ricardoV94 ricardoV94 added this to the v4.0.0 milestone Feb 5, 2022
@ricardoV94 ricardoV94 changed the title Refactor Multivariate distributions in line with new meaning of size Refactor Multivariate distributions due to new meaning of size Feb 5, 2022
@michaelosthege
Copy link
Member

Important detail:
It's size == shape[:len(shape)-ndim_supp], NOT shape[:-ndim_supp]

That's because

>>> (1,2,3)[:-0]
()

@ricardoV94
Copy link
Member Author

Important detail:
It's size == shape[:len(shape)-ndim_supp], NOT shape[:-ndim_supp]

That's because

>>> (1,2,3)[:-0]
()

Stupid -0 slicing heh :D

@twiecki
Copy link
Member

twiecki commented Feb 15, 2022

@michaelosthege Do you have time to work on this?

@michaelosthege
Copy link
Member

I will pick it up once the CI is green again on main. Don't want to solve two problems at the same time

@twiecki
Copy link
Member

twiecki commented Feb 15, 2022

OK, CI is green again.

ricardoV94 pushed a commit to michaelosthege/pymc that referenced this issue Feb 22, 2022
ricardoV94 added a commit to michaelosthege/pymc that referenced this issue Feb 22, 2022
michaelosthege pushed a commit that referenced this issue Feb 27, 2022
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