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

Add DiscreteWeibull moment #5407

Merged
merged 1 commit into from Jan 27, 2022
Merged

Conversation

zoj613
Copy link
Contributor

@zoj613 zoj613 commented Jan 27, 2022

Adds the median of the discrete weibull distribution as mentioned in #5078 (comment)

Closes #5078

@zoj613
Copy link
Contributor Author

zoj613 commented Jan 27, 2022

I think it is worth noting that according to the parametrization used for the distribution, the q parameter must be in the interval (0, 1), and so it makes sense to check this when instantiating the DisreteWeibull class. currently, this parameter validation is not done at all.

cc @ricardoV94

@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #5407 (ac99306) into main (71b18e6) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5407      +/-   ##
==========================================
+ Coverage   80.45%   80.48%   +0.02%     
==========================================
  Files          82       82              
  Lines       14151    14157       +6     
==========================================
+ Hits        11385    11394       +9     
+ Misses       2766     2763       -3     
Impacted Files Coverage Δ
pymc/distributions/discrete.py 98.81% <100.00%> (+0.01%) ⬆️
pymc/__init__.py 100.00% <0.00%> (ø)
pymc/parallel_sampling.py 87.70% <0.00%> (+0.99%) ⬆️

@ricardoV94
Copy link
Member

ricardoV94 commented Jan 27, 2022

I think it is worth noting that according to the parametrization used for the distribution, the q parameter must be in the interval (0, 1), and so it makes sense to check this when instantiating the DisreteWeibull class. currently, this parameter validation is not done at all.

cc @ricardoV94

@zoj613 We only to those type of checks when drawing random numbers or evaluating the logp / logcdf, as the values may be symbolic when the distribution is initialized. The moments should not have to worry about invalid parameters

return check_parameters(res, 0 < q, q < 1, 0 < beta, msg="0 < q < 1, beta > 0")

@ricardoV94 ricardoV94 mentioned this pull request Jan 27, 2022
51 tasks
@ricardoV94
Copy link
Member

Failing test seems unrelated: #5409

@ricardoV94 ricardoV94 merged commit 854fd04 into pymc-devs:main Jan 27, 2022
@ricardoV94
Copy link
Member

Thanks @zoj613 this was the last one!

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

Successfully merging this pull request may close these issues.

Bring back distribution moments
2 participants