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

New Continuous Process: Mixed Poisson #7

Closed
Gabinou opened this issue Jul 27, 2018 · 7 comments
Closed

New Continuous Process: Mixed Poisson #7

Gabinou opened this issue Jul 27, 2018 · 7 comments

Comments

@Gabinou
Copy link
Contributor

Gabinou commented Jul 27, 2018

This continuous stochastic process generates points distributed according to a rate generated from an arbitrary random distribution. I refer to the random distribution used to generate this rate is known as the information distribution.

The user would need to supply a random distribution in its parameters upon first call (numpy functions such as numpy.random.uniform). Then, a rate would be generated upon initialization. Upon each call of the sample method, a new random rate would be generated using the information distribution.

Since this is a Poisson Process, the suggested MixedPoissonProcess class is very similar to the PoissonProcess class. Additionnal methods are setters and getters for the information process and parameters. Upon each call of the setters, a new rate is generated. Another additional method is the genrate method, which generates a new random rate using the previously supplied information distribution and parameters.

@crflynn
Copy link
Owner

crflynn commented Jul 27, 2018

I saw your PR for this process but I think it might be simpler to just subclass PoissonProcess and make use of the already existing code. Additionally, you would need to

  • provide init args consisting of an rate_func, as well as rate_args, and rate_kwargs for that function to ensure it is generalized enough for any rng package (random, numpy, scipy, etc.).
  • override the rate attribute to call rate_func(*rate_args, **rate_kwargs)

@Gabinou
Copy link
Contributor Author

Gabinou commented Jul 28, 2018

Following your comments and suggestions, I have implemented changes:

  • MixedPoissonProcess now inherits from Poisson process. I have eliminated some amount of code repetition.
  • Eliminated some other repetitions by way of the gen_rate() method.
  • *rate_args and **rate_kwargs are implemented.
  • New test fixtures for rate_kwargs. The tests seem to be in working order.
  • Modified the continuous.rst to include the MixedPoissonProcess class.

Still to be implemented is:

  • Making sure the code fits style standards. I will work on that for the implementation of the appropriate features, mostly.

Is the pull request in a more acceptable state?
And thank you for your comments and help.

@crflynn
Copy link
Owner

crflynn commented Jul 28, 2018

This is ok but there are still a lot of issues that need to be addressed.

I think I might merge this into a separate dev branch and clean it up if you don't mind. I'm hoping that maybe you'll track the changes closely and if needed I can explain the reasoning for them. Is that ok?

@Gabinou
Copy link
Contributor Author

Gabinou commented Jul 28, 2018

This sounds marvelous. When it is up to scratch, I will use it as a template for my future pull requests.

Thank you very much!

@crflynn
Copy link
Owner

crflynn commented Jul 29, 2018

I requested some changes in the PR. Please address them. Thanks.

@Gabinou
Copy link
Contributor Author

Gabinou commented Jul 30, 2018

All changes were addressed. If this makes this feature acceptable, I will get to upgrading other features also.

@crflynn
Copy link
Owner

crflynn commented Aug 19, 2018

@Gabinou I have merged your PR #8 and made several changes that I think make the implementation a bit cleaner. You may refer to the commits in the mixed branch for the changes. Thanks for contributing.

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

No branches or pull requests

2 participants