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

Implementation of Maxwell RV #440

Closed
wants to merge 2 commits into from
Closed

Conversation

tvwenger
Copy link
Contributor

Motivation for these changes

Following this discussion, I have submitted this PR to implement a random variable (RV) that follows a Maxwell distribution. The Maxwell distribution appears often in physical systems and having access to it in pymc and pytensor would be generally useful.

Implementation details

I have attempted to simply follow the style for other RV implementations that extend ScipyRandomVariable. I have also added tests to tests/tensor/random/test_basic.py, which is now running (slowly...).

I've added the relevant implementation and tests to numba and jax, but I would appreciate someone double checking what I've done since I was unsuccessful in running tests/link/numba/test_random.py, and tests/link/jax/test_random.py (the tests are skipped, probably because I don't have jax/numba set up in my environment).

Checklist

Major / Breaking Changes

  • None

New features

  • New MaxwellRV implementation

Bugfixes

  • None

Documentation

  • Included docstrings as necessary

Maintenance

  • N/A

@tvwenger
Copy link
Contributor Author

As I suspected, the numba and jax tests are failing. I didn't really know how to set up these tests, so any help would be appreciated!

@ricardoV94
Copy link
Member

ricardoV94 commented Sep 13, 2023

Hi @tvwenger, there's no need to split this in 2 libraries, both the RV and distribution should be implemented in pymc-experimental.

For more details in how to implement and test see this guide: https://www.pymc.io/projects/docs/en/latest/contributing/implementing_distribution.html A CustomDist sounds enough so that guide isn't needed

@tvwenger
Copy link
Contributor Author

Related PR on pymc-experimental: pymc-devs/pymc-extras#239

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.

2 participants