-
Notifications
You must be signed in to change notification settings - Fork 273
Add Types to numpyro.distributions.distribution
#2122
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
Conversation
numpyro/distributions/continuous.py
Outdated
| from jax.scipy.stats import norm as jax_norm | ||
| from jax.typing import ArrayLike | ||
|
|
||
| from numpyro._typing import DistributionT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just depend on Distribution, instead of DistributionT? We have some success in constraints.py and transforms.py, which do not use ConstraintT and TransformT. We want to reduce the scope of using DistributionT through the library. Ideally, it's great to not use those protocols at all to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed them in 3303b4a :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe grep DistributionT and remove at other scripts too? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Started on c27121b and fixing edge cases in the following commits :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Do you also want to remove ConstraintT and TransformT? I'll try to make a release this weekend.
Related to #2036