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.

Sign up for GitHub

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

Jump to bottom

.random() methods missing from some timeseries distributions #4337

Closed
michaelosthege opened this issue Dec 14, 2020 · 8 comments
Closed

.random() methods missing from some timeseries distributions #4337

michaelosthege opened this issue Dec 14, 2020 · 8 comments
Labels
enhancements v3 wontfix

Comments

@michaelosthege
Copy link
Member

Timeseries distributions with a .random() method:

  • GaussianRandomWalk

Timeseries distributions where .random is None:

  • MvGaussianRandomWalk
  • MvStudentTRandomWalk
  • AR
  • AR1
  • GARCH11
  • EulerMaruyama

At least for the two Mv distributions is should be straightforward to implement.

Make sure to include them in test_distributions_random.py, just like GuassianRandomWalk already is!
@michaelosthege michaelosthege mentioned this issue Dec 14, 2020
Merged
6 tasks
@awray3
Copy link
Contributor

awray3 commented Dec 23, 2020

Hi, I am new to contributing and would like to work on this issue. Can I work on it?

@michaelosthege
Copy link
Member Author

@awray3 of course, go ahead!
Just make sure to mention this issue when opening a PR, so it gets linked.
Also check out the wiki pages. We have some code style guidance there.

@awray3
Copy link
Contributor

awray3 commented Dec 24, 2020

Is MvGaussianRandomWalk intended to only take one of cov, tau, or chol, much like how GaussianRandomWalk only accepts one of tau and sigma but not both?

Also, are each of the random walks conventionally taken to start at 0 like GaussianRandomWalk?

@AlexAndorra
Copy link
Contributor

Is MvGaussianRandomWalk intended to only take one of cov, tau, or chol, much like how GaussianRandomWalk only accepts one of tau and sigma but not both?

Yes, these three parameters are just three different ways of expressing the covariance matrix. You can take a look at MvNormal to get a better idea if needed.

Also, are each of the random walks conventionally taken to start at 0 like GaussianRandomWalk?

Yes, I think so. I'm guessing it's like Gaussian Processes: you can just add the mean separately afterwards to center the random walk on something else than 0

@AlexAndorra AlexAndorra mentioned this issue Dec 24, 2020
Closed
@awray3
Copy link
Contributor

awray3 commented Dec 24, 2020 •
edited
Loading

@AlexAndorra Thank you for the pointer; it lead me to _QuadFormBase which had exactly what I needed.

@awray3
Copy link
Contributor

awray3 commented Dec 24, 2020

General question on time series in PyMC3, which I could not find in the documentation: what should the shape parameter be for a multivariate time series random variable? My guess after sorting through some of the code was that it should be something like shape = (time, n_dims), but I found no definite answer. I will consider adding shape to the docstrings for these functions to eliminate any guesswork.

@awray3 awray3 mentioned this issue Dec 27, 2020
Merged
Sayam753 added a commit that referenced this issue Jan 3, 2021
@awray3 @Sayam753
Add .random method to MvGaussianRandomWalk (#4388)
2824027 
* Implement random for MvGaussianRandomWalk

Implements the random method for MvGaussianRandomWalk,
partially fixing #4337.

* Update docstring for GaussianRandomWalk._random()

* Add example to random docstring

* Modify MvGaussianRandomWalk.random

Improves the implementation by using MvNormal.random as suggested by
@Sayam753. Also updated its docstring to add more examples.

* Add TestMvGaussianRandomWalk

* Pass entire shape to MvNormal

This commit rewrites the random method to pass the entire shape
into MvNormal. MvGaussianRandomWalk also now supports an integer shape
that must match the dimensionality of mu. In this case it is assumed
that there are no time steps, and a random sample from the multivariate
normal distribution will be returned.

* Fix rebase issue

* Update pymc3/distributions/timeseries.py

Remove equality since the shape parameter cannot have more than 2 dimensions.

Co-authored-by: Sayam Kumar <sayamkumar753@yahoo.in>

* Update comment pymc3/distributions/timeseries.py

Co-authored-by: Sayam Kumar <sayamkumar753@yahoo.in>

* Update pymc3/distributions/timeseries.py

Adds a more explicit conditional on the time_axis statement.

Co-authored-by: Sayam Kumar <sayamkumar753@yahoo.in>

* Fix syntax error

* Add tests for MvGaussianRandomWalk with RV params

Added tests for both chol and cov to be random variables.

* Improve comment clarity in MvNormal._random

Moves the brittle comment in MvRandom._random's docstring to the actual location in the code for clarity.

* Update docstring formatting

* Update RELEASE-NOTES.md

Co-authored-by: Sayam Kumar <sayamkumar753@yahoo.in>
@ricardoV94 ricardoV94 mentioned this issue Jan 19, 2021
Closed
@Sayam753 Sayam753 mentioned this issue Jan 28, 2021
Closed
@mjhajharia
Copy link
Member

Entire time series is being refactored so maybe we can close this @michaelosthege ?

@ricardoV94
Copy link
Member

I would leave it open until those are implemented, as a reminder issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancements v3 wontfix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@michaelosthege @ricardoV94 @AlexAndorra @awray3 @mjhajharia