Skip to content

Distribution class refactoring: an example using the Weibull distribution #4668

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
wants to merge 6 commits into from
Closed

Conversation

larryshamalama
Copy link
Member

This PR addresses a topic that @brandonwillard brought up in issue 4518 (and 4515) regarding the Distribution interface for random variables. I came across these refactoring challenges when learning about the replacement of random functions to rng_fn.

Overall, I'm trying to understand how Distribution classes are to be updated, so here is my attempt in which I worked on the Weibull distribution. Following the example code in issue 4518, a summary of what I did (and am understanding) is as follows:

  • We build upon weibull = WeibullRV() from aesara.tensor.random.basic
  • Object attributes mean, variance and mode are removed.
  • Function signatures follow other examples in the same file, but overall I'm not sure about this.

Question

  • Not all RV objects in aesara.tensor.random.basic have __call__ and rng_fn implemented. Should these be implemented? Please let me know if my questions are answered in another thread.

Next steps

  • Continue reworking some other Distribution objects in continuous.py and discrete.py in pymc3.distributions
  • Implement rng_fn class methods in Aesara

@ricardoV94
Copy link
Member

ricardoV94 commented Apr 26, 2021

Hi @larryshamalama, thanks you for opening a PR.

Unfortunately these changes are already being taken care of in this PR #4640

The distributions after that PR will require implementing our own RandomOps that are not available in Aesara. You can find an example of this in #4615 (and how to integrate with the new random tests in #4608).

If you would like to help with that effort instead that would be great!

Edit: Feel free to ask if you have any questions!

@larryshamalama
Copy link
Member Author

Hi @ricardoV94, thank you very much for your comments! I'm still new to contributing to PyMC3, so hopefully I can learn more from your ongoing work and help out in other ways. I'll let you know if I have any questions. Closing this PR

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

Successfully merging this pull request may close these issues.

2 participants