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

AR1 time series distribution [WIP] #5388

Closed
wants to merge 4 commits into from
Closed

AR1 time series distribution [WIP] #5388

wants to merge 4 commits into from

Conversation

mjhajharia
Copy link
Member

@mjhajharia mjhajharia commented Jan 25, 2022

  • Add RV
  • Add Distribution
  • Add Logp
  • aesara scan stuff
  • Tests

closes #5291

@mjhajharia mjhajharia changed the title AR time series distribution AR time series distribution [WIP] Jan 25, 2022
@mjhajharia mjhajharia marked this pull request as draft January 25, 2022 15:04
@mjhajharia mjhajharia added the v4 label Jan 25, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #5388 (75f865a) into main (eb5177a) will decrease coverage by 0.01%.
The diff coverage is 51.51%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5388      +/-   ##
==========================================
- Coverage   80.44%   80.43%   -0.02%     
==========================================
  Files          82       82              
  Lines       14132    14150      +18     
==========================================
+ Hits        11369    11381      +12     
- Misses       2763     2769       +6     
Impacted Files Coverage Δ
pymc/distributions/timeseries.py 28.19% <51.51%> (+5.83%) ⬆️
pymc/parallel_sampling.py 86.71% <0.00%> (-1.00%) ⬇️

@mjhajharia mjhajharia changed the title AR time series distribution [WIP] AR1 time series distribution [WIP] Jan 26, 2022
@@ -0,0 +1,249 @@
{
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ricardoV94 any chance this error happens because I use a for loop in the logp instead of scan


Reply via ReviewNB

@@ -32,49 +37,94 @@
"MvStudentTRandomWalk",
]

class AR1rv(RandomVariable):
name = "AR1"
ndim_supp = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ndim_supp = 0
ndim_supp = 1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i saw this comment on grw pr as well and for some reason ndim_supp = 1 throws an error when i add observed, something like "dimension mismatch of 2 and 1" as in data is 1d or something, i dont know why does it happen though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's related to the comments below, you haven't yet told Aesara how to infer the shape of the distribution given the parameters (and size). It tries to do something by default, but usually fails with multivariate distributions where there is not a simple 1-1 map between parameter shapes and distribution shape

@ricardoV94
Copy link
Member

ricardoV94 commented Jan 26, 2022

@mjhajharia I would suggest you try to run the default "check_rv_size" and "check_pymc_params_match_rv_op" tests as in here:

class TestStickBreakingWeights(BaseTestDistribution):
pymc_dist = pm.StickBreakingWeights
pymc_dist_params = {"alpha": 2.0, "K": 19}
expected_rv_op_params = {"alpha": 2.0, "K": 19}
sizes_to_check = [None, 17, (5,), (11, 5), (3, 13, 5)]
sizes_expected = [
(20,),
(17, 20),
(
5,
20,
),
(11, 5, 20),
(3, 13, 5, 20),
]
tests_to_run = [
"check_pymc_params_match_rv_op",
"check_rv_size",

You need to tell Aesara how to infer the shape of the distribution given the parameters, and those checks cover that

@ricardoV94
Copy link
Member

ricardoV94 commented Jan 26, 2022

Actually the recent StickBreakingWeights distribution is a very good template in general for this type of multivariate distributions:

class StickBreakingWeightsRV(RandomVariable):

Edit: For the shape inference look here:

def _infer_shape(self, size, dist_params, param_shapes=None):
alpha, K = dist_params
size = tuple(size)
return size + (K + 1,)

@canyon289
Copy link
Member

Just a heads up. I havent forgotten about this. I know youre busy with Data Umbrella and also I still dont exactly know what im doing with new distributions. Once both of those challenges are over in the near future I'll revisit this

@ricardoV94 ricardoV94 closed this May 5, 2022
@ricardoV94 ricardoV94 deleted the ar branch November 2, 2022 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add autoregressive model to V4
3 participants