-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Adding moment for Moyal distribution and corresponding tests #5179
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
Adding moment for Moyal distribution and corresponding tests #5179
Conversation
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.
Looks great. Thanks for opening the PR.I have just a small change suggestion.
pymc/distributions/continuous.py
Outdated
@@ -3727,6 +3727,14 @@ def dist(cls, mu=0, sigma=1.0, *args, **kwargs): | |||
|
|||
return super().dist([mu, sigma], *args, **kwargs) | |||
|
|||
def get_moment(rv, size, mu, sigma): | |||
mu, sigma = at.broadcast_arrays(mu, sigma) |
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.
mu, sigma = at.broadcast_arrays(mu, sigma) |
This is not needed here. The parameters will be automatically broadcasted below as long as they show up in the same expression
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. Thanks.
Codecov Report
@@ Coverage Diff @@
## main #5179 +/- ##
==========================================
- Coverage 78.09% 78.06% -0.03%
==========================================
Files 88 88
Lines 14149 14171 +22
==========================================
+ Hits 11050 11063 +13
- Misses 3099 3108 +9
|
Looks good. Waiting to see if the tests pass |
Thanks for the help @jlopezarriaza! |
Add Moyla moment and corresponding tests to help with #5078