[WIP] VECM #3246

yogabonito · 2016-10-28T16:20:11Z

Continuing the work from #3070.

coveralls · 2016-10-30T17:42:48Z

Coverage decreased (-0.1%) to 87.698% when pulling d878c8f on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

coveralls · 2016-10-31T15:42:07Z

Coverage decreased (-0.1%) to 87.716% when pulling b00be6d on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

coveralls · 2016-11-02T21:32:29Z

Coverage increased (+0.3%) to 88.134% when pulling 0b932de on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

coveralls · 2016-12-28T22:55:54Z

Coverage increased (+0.5%) to 88.342% when pulling 06deff7 on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

coveralls · 2017-01-12T16:44:25Z

Coverage increased (+0.5%) to 88.339% when pulling 77ce5da on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

coveralls · 2017-01-17T15:23:47Z

Coverage increased (+0.4%) to 88.285% when pulling 99b5ff9 on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

coveralls · 2017-01-17T22:09:37Z

Coverage increased (+0.4%) to 88.285% when pulling 6c504a0 on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

coveralls · 2017-01-31T01:02:01Z

Coverage increased (+0.5%) to 88.338% when pulling f32ea35 on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

ChadFulton · 2017-02-28T01:25:38Z

Just wanted to let you know that I am going to make an effort to help review this.

yogabonito · 2017-03-01T22:13:45Z

Thank you @ChadFulton! I had an exam this week and will now finish the integration of Josef's code (cointegration test) ASAP. IMO the PR will be ready to be merged once this is done.

yogabonito · 2017-03-22T14:35:32Z

Travis is giving a FileNotFoundError while in AppVeyor all tests pass. @ChadFulton, may I ask you to have a look at it?

yogabonito · 2017-03-22T14:38:07Z

@josef-pkt I think that - apart from one open question - the code is ready to be merged. This question is related to the coint_trend argument of the coint_johansen function. This argument is not used in your tests and I haven't seen it in the corresponding Matlab code. How shall this argument be treated?

josef-pkt · 2017-03-22T14:52:01Z

AFAIR, I added it when a user asked for more flexible trend options. I only realized later that the options for the cointegration tests need to be limited to the cases for which we have the p-values or critical values for the test. The estimation of VECM can have more options, but the cointegration test can only allow the restricted set of options with p-values. We might be able to extend those a bit, but some combinations don't have a good (IIRC parameter or data independent) limiting distribution.

josef-pkt · 2017-03-22T15:19:26Z

From what I can see in setup.py txt and csv files are only included in the distribution if the directory ends with results. I don't know why appveyor would pick them up anyway.

It looks like travis is using python setup.py install and appveyor uses python setup.py develop. So only travis is testing that the sdist includes all required files.

josef-pkt · 2017-03-22T22:07:42Z

I don't know why travis doesn't find the csv file

One local check is to run python setup.py sdist and check in the created archive file if the csv/data files are included.

yogabonito · 2017-03-22T22:42:16Z

Thanks for your help @josef-pkt! I really appreciate it!
The tar.gz-archive produced by python3 setup.py sdist contains the csv-file on my machine.

coveralls · 2017-03-23T11:34:07Z

Coverage increased (+2.7%) to 90.561% when pulling 0c5c878 on yogabonito:gsoc-vecm into fc3584f on statsmodels:master.

yogabonito · 2017-03-23T12:11:47Z

@josef-pkt, now the tests are passing in Travis. To me it looks as if the AppVeyor build is failing randomly (this time a Python 2 build job failed, whereas after my last commit it was the Python 3 build job).

I think that the PR is now ready to be merged. What are the next steps? Shall I do a rebase using a copy of this branch (to avoid messing up this PR)?

josef-pkt · 2017-03-23T12:41:10Z

If the rebase goes well, then force pushing on this branch is fine.
However, given the large number of commits, I would create a new branch for the rebase in case the rebase or merge conflicts is messy. If there are no conflicts, then force pushing here should be safe, otherwise open a new PR.

(There are two names in the previously rebased commits, but it looks fine in the network. It looks like the initial commits where with a different user name, but that wouldn't cause problems.)

josef-pkt

I went through the first part of vecm.py, up to beginning of VECM class, with looking mainly at the API and docstrings.

Some names are not very informative but the part I went through is mostly internal, so it's not urgent to come up with better names there.

josef-pkt · 2017-03-23T12:50:59Z

statsmodels/tsa/vecm/vecm.py

+                          seasons_centered=False, exog=None, exog_coint=None):
+    """
+    Use the VECM's deterministic terms to construct an array that is suitable
+    to convey this information to VAR in form of the exog-argument for VAR's


docstrings need to have a one line summary at the beginning (will be used in tables or lists of functions and methods in the generated documentation)

josef-pkt · 2017-03-23T12:52:24Z

statsmodels/tsa/vecm/vecm.py

+    Parameters
+    ----------
+    deterministic : str
+        See VECM's docstring for more information.


This should mention briefly what it is before pointing to the full VECM docstring

josef-pkt · 2017-03-23T12:54:55Z

statsmodels/tsa/vecm/vecm.py

+    deterministic : str
+        See VECM's docstring for more information.
+    seasons : int
+        Number of seasons.


could be more explicit, maybe number of periods in a seasonal cycle.
(I don't remember how this is usually explained in seasonal_decompose (using freq) or SARIMAX.

josef-pkt · 2017-03-23T12:57:48Z

statsmodels/tsa/vecm/vecm.py

+    return np.column_stack(exogs) if exogs else None
+
+
+def _mat_sqrt(_2darray):


_2darray is not a pretty name
but it's just internal

josef-pkt · 2017-03-23T13:00:25Z

statsmodels/tsa/vecm/vecm.py

+    """
+    u_, s_, v_ = svd(_2darray, full_matrices=False)
+    s_ = np.sqrt(s_)
+    return chain_dot(u_, np.diag(s_), v_)


this is just u_.dot(s_ * v_) or something like that

s_ is a 1D-array, so using * won't be possible.

s_ should broadcasts. I use this shortcut all the time with diagonal matrices. I just need to check each time whether it needs to be row or column s_ or s_[:, None]

josef-pkt · 2017-03-23T14:08:12Z

statsmodels/tsa/vecm/vecm.py

+
+    Parameters
+    ----------
+    endog_tot : array-like (nobs_tot x neqs)


just endog

josef-pkt · 2017-03-23T14:15:47Z

statsmodels/tsa/vecm/vecm.py

+    endog_tot : array-like (nobs_tot x neqs)
+        2-d endogenous response variable.
+    exog: ndarray (nobs_tot x neqs) or None
+        Deterministic terms outside the cointegration relation.


add "User provided deterministic terms and weakly exogenous variables ..."
add comment about deterministic, for example, "deterministic trends specified by the deterministic keyword will be added to it by the model"
That's what it does, does it?

also season dummies (?) will be added.

exog is not affected by deterministic and seasons. But it is possible to get all the estimators for the deterministic terms through the det_coef attribute of a VECMResults object. The parts of det_coef are also available via the const, seasonal, lin_trend, and exog_coefs attributes.

josef-pkt · 2017-03-23T14:18:39Z

statsmodels/tsa/vecm/vecm.py

+    References
+    ----------
+    .. [1] Lutkepohl, H. 2005. *New Introduction to Multiple Time Series Analysis*. Springer.
+    """


This needs a Notes section with more explanation about the options.
e.g. the exog, deterministic and seasons and their relationship could be explained here.

josef-pkt · 2017-03-23T14:20:10Z

statsmodels/tsa/vecm/vecm.py

+        See :class:`statsmodels.tsa.base.tsa_model.TimeSeriesModel` for more
+        information.
+    freq : str, optional
+        See :class:`statsmodels.tsa.base.tsa_model.TimeSeriesModel` for more


what is freq used for?
It should be the same as specifying season if it applies.
?

I forgot freq and missing which were part of the scaffold I started from. After the last rebase I get warnings related to freq. Shall I just use freq instead of seasons?

josef-pkt · 2017-03-23T14:21:27Z

statsmodels/tsa/vecm/vecm.py

+        See :class:`statsmodels.tsa.base.tsa_model.TimeSeriesModel` for more
+        information.
+    missing : str, optional
+        See :class:`statsmodels.base.model.Model` for more information.


Is this really correctly handled for time series? Might need a warning or explanation, that dropping observations might not make sense.

Shall I just remove missing?

yogabonito · 2017-03-23T14:31:43Z

@josef-pkt, I have seen your suggestions right after force pushing. I will go through them now. Thanks for your feedback!
The two usernames stem from changing the git config during the development.

josef-pkt · 2017-03-23T14:34:32Z

statsmodels/tsa/vector_ar/irf.py

@@ -277,7 +287,7 @@ def errband_mc(self, orth=False, svar=False, repl=1000,
                                        signif=signif, seed=seed,
                                        burn=burn, cum=False)
        else:
-            return model.irf_errband_mc(orth=orth, repl=repl, T=periods,
+            return model.irf_errband_mc(orth=orth, repl=repl, T=periods,  # TODO: irf_errband relies on self.intercept --> solve with exog-refactor


long comments (if beyond 80 character line limit) should go into separate line

josef-pkt · 2017-03-23T14:36:03Z

statsmodels/tsa/vector_ar/output.py

-    null_hyp = 'H_0: %s do not Granger-cause %s' % (variables, equation)
+def causality_summary(results, variables, equation, kind, inst_caus=False):
+    """
+


one line summary missing (doc rendering will have problems)

josef-pkt · 2017-03-23T14:45:41Z

@yogabonito Do you have any notebooks with for example the test cases against jmulti?
It would be good to add some to the documentation to illustrate the usage and features of VECM.

We don't commit the output in notebooks (to avoid increasing repo memory with rendered output and plots), so it's better to post draft versions somewhere else, e.g. in a gist or scratch/working repo on github.

yogabonito · 2017-03-23T14:50:18Z

I will make a notebook after I have gone through your suggestions :)

…ng todos The code was buggy because VAR() was called without specifying trend and exog so always the default values (i.e. a constant deterministic term) were used.

…improvement * documentation improvements: - Added classes from this PR to `tsa`'s documentation - Made sure LaTeX, enumerations, and a table in docstrings are rendered nicely by Sphinx. - Made links in the documentaion (e.g. from a method to its result-class) - small fixes * API-changes: - for lags in levels: `p` --> `k_ar` - for lags in difference: `diff_lags` --> `k_ar_diff` - added an underscore to a few methods to mark them as internal * coverage improvement: Achieved by adding `# pragma: no cover`-comment to lines which are only meant for debugging purposes.

This also closes statsmodels#3615.

- change signif to alpha instead of 1-alpha (e.g. 0.05 instead of 0.95) - fix bug in the `summary`-method of `CointRankResults` which caused an `IndexError` if `rank==neqs`.

coveralls · 2017-06-27T15:04:08Z

Coverage increased (+0.3%) to 90.811% when pulling a0be32c on yogabonito:gsoc-vecm into 1c7f487 on statsmodels:master.

yogabonito · 2017-06-27T15:13:23Z

@josef-pkt I have moved vecm to vector_ar. I have also rebased again today since there were a few new commits in upstream/master. Now this PR should be ready to get merged.

josef-pkt · 2017-06-27T15:35:45Z

@yogabonito Thanks, I will have another quick look, but expect to merge it by tomorrow, assuming all is green. (Costa Brava is calling.:)

coveralls · 2017-06-27T15:55:30Z

Coverage increased (+0.3%) to 90.804% when pulling bf2d2b0 on yogabonito:gsoc-vecm into 1c7f487 on statsmodels:master.

coveralls · 2017-06-27T16:20:33Z

Coverage increased (+0.02%) to 90.56% when pulling bf2d2b0 on yogabonito:gsoc-vecm into 1c7f487 on statsmodels:master.

yogabonito · 2017-06-27T22:22:40Z

All is green 😀 Have a nice holiday, Josef!

josef-pkt · 2017-06-28T09:53:48Z

@yogabonito I'm merging this as is, move and rebase look good.
This is a BIG contribution, and thanks for the GSOC.

Given the lack of reviewer time, there are still some details left to review and change. Followup issue is #3775
One immediate question: Why does HypothesisTestResults and subclasses implement __eq__? What's the intention and purpose for this?

yogabonito · 2017-06-28T14:05:29Z

Why does HypothesisTestResults and subclasses implement __eq__?

Finding out whether two tests are testing the same thing probably isn't an indispensable feature so the __eq__ method can safely be removed. I just felt like implementing it ;)

yogabonito · 2017-06-28T14:05:53Z

It's great to see VECM merged! 😃 Thank you @josef-pkt and @bashtage for leading me through the GSoC - it was a great experience!

bashtage · 2017-06-28T15:03:50Z

Thanks for the great contribution. Of course, future contributions always welcome!

yogabonito · 2017-06-28T16:17:22Z

future contributions always welcome!

@bashtage for now I will concentrate on my GSoC 2017 but it's definitely something I will consider!

jbrockmendel · 2017-06-29T19:40:10Z

statsmodels/tsa/vector_ar/vecm.py

+    .. [1] Lutkepohl, H. 2005. *New Introduction to Multiple Time Series Analysis*. Springer.
+    """
+
+    # todo: rewrite m such that a big (TxT) matrix is avoided


Can the nobs x nobs problem be avoided by separating out the left-multiplication by delta_x.T?:

not_m = inv(delta_x.dot(delta_x.T)).dot(delta_x) # shape --> (k_ar_diff*neqs, nobs) pre_r0 = delta_y_1_T.dot(delta_x.T) # shape --> (neqs, k_ar_diff*neqs) pre_r1 = y_lag1.dot(delta_x.T) # shape --> (neqs, k_ar_diff*neqs) r0 = delta_y_1_T - pre_r0.dot(not_m) r1 = y_lag1 - pre_r1.dot(not_m)

josef-pkt mentioned this pull request Feb 5, 2017

DESIGN: AR, VAR, SVAR signature deprecations in 0.9 #3434

Open

josef-pkt mentioned this pull request Feb 14, 2017

ENH/Reference: time series bootstrap VAR/VECM #3143

Open

josef-pkt mentioned this pull request Mar 3, 2017

Attach dates to VAR forecasts #3531

Closed

josef-pkt reviewed Mar 23, 2017

View reviewed changes

yogabonito force-pushed the gsoc-vecm branch from 0c5c878 to 3fc5c4e Compare March 23, 2017 14:25

josef-pkt reviewed Mar 23, 2017

View reviewed changes

yogabonito added 7 commits June 27, 2017 15:43

VECM: docstrings (add Notes about deterministics,...)

580ce41

VAR: fix bugs, improve readability of edited code, delete correspondi…

bc545ed

…ng todos The code was buggy because VAR() was called without specifying trend and exog so always the default values (i.e. a constant deterministic term) were used.

Change code according to @jbrockmendel's suggestions in the WIP-PR

daf220e

This also closes statsmodels#3615.

VECM: cointegration rank tests

f1c5f50

- change signif to alpha instead of 1-alpha (e.g. 0.05 instead of 0.95) - fix bug in the `summary`-method of `CointRankResults` which caused an `IndexError` if `rank==neqs`.

move VECM to vector_ar

8577909

add test results which were missing after last commit

a0be32c

yogabonito force-pushed the gsoc-vecm branch from 8ceb7bd to a0be32c Compare June 27, 2017 13:45

VECM: delete unneeded files which haven't been moved to vector_ar

91eade6

remove vecm-module also from tsa/api.py

bf2d2b0

josef-pkt merged commit 09bd0c5 into statsmodels:master Jun 28, 2017

yogabonito deleted the gsoc-vecm branch June 28, 2017 13:53

josef-pkt mentioned this pull request Jun 28, 2017

[WIP] [GSoC 2016] VECM #3013 #3070

Closed

jbrockmendel reviewed Jun 29, 2017

View reviewed changes

josef-pkt mentioned this pull request Jul 10, 2017

WIP: ENH: Johansen's Cointegration test and VECM #453

Closed

josef-pkt mentioned this pull request Sep 29, 2017

GSOC 2016: VECM #3013

Closed

josef-pkt added this to the 0.9 milestone Apr 15, 2018

josef-pkt mentioned this pull request Apr 15, 2018

release 0.9 #3076

Closed

		return np.column_stack(exogs) if exogs else None


		def _mat_sqrt(_2darray):

[WIP] VECM #3246

[WIP] VECM #3246

Conversation

yogabonito commented Oct 28, 2016

coveralls commented Oct 30, 2016

coveralls commented Oct 31, 2016

coveralls commented Nov 2, 2016

coveralls commented Dec 28, 2016

coveralls commented Jan 12, 2017

coveralls commented Jan 17, 2017

coveralls commented Jan 17, 2017

coveralls commented Jan 31, 2017

ChadFulton commented Feb 28, 2017

yogabonito commented Mar 1, 2017

yogabonito commented Mar 22, 2017

yogabonito commented Mar 22, 2017

josef-pkt commented Mar 22, 2017

josef-pkt commented Mar 22, 2017

josef-pkt commented Mar 22, 2017

yogabonito commented Mar 22, 2017

coveralls commented Mar 23, 2017

yogabonito commented Mar 23, 2017

josef-pkt commented Mar 23, 2017

josef-pkt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yogabonito commented Mar 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josef-pkt commented Mar 23, 2017

yogabonito commented Mar 23, 2017

coveralls commented Jun 27, 2017 • edited Loading

yogabonito commented Jun 27, 2017

josef-pkt commented Jun 27, 2017

coveralls commented Jun 27, 2017

coveralls commented Jun 27, 2017 • edited Loading

yogabonito commented Jun 27, 2017

josef-pkt commented Jun 28, 2017

yogabonito commented Jun 28, 2017

yogabonito commented Jun 28, 2017

bashtage commented Jun 28, 2017

yogabonito commented Jun 28, 2017

Choose a reason for hiding this comment

coveralls commented Jun 27, 2017 •

edited

Loading

coveralls commented Jun 27, 2017 •

edited

Loading