Add Durbin-Watson test #102

BenjaminBorn · 2017-08-07T17:58:03Z

Implementation of the Durbin-Watson test for serial correlation in the residuals of a regression model.

WIP as I still have to implement Pan's algorithm (Farebrother, 1980) for the computation of exact p-values in small samples.

Everything else is ready and I would be glad to get feedback.

One question: I tried to include a displayed latex equation in the docstring as described in the documentation (and I understand it will not render the Latex code in the REPL or Juno console) but it does not even display the Latex code correctly.

Thanks as always for your help!

ararslan · 2017-08-07T19:36:46Z

src/durbin_watson.jl

+vector of residuals. Note that the Durbin-Watson test is not valid if `X`
+includes a lagged dependent variable. The test statistic is computed as
+```math
+    DW = \frac{\sum_{t=2}^n (e_t - e_{t-1})^2}{\sum_{t=1}^n e_t^2}


You'll need to escape the backslashes

BenjaminBorn · 2017-08-13T20:10:44Z

I have now added Pan's algorithm (Farebrother, 1980) for the computation of exact p-values in small samples. Tests are comparing output to R.

@ararslan It seems like the p-value is computed three times when calling t = DurbinWatsonTest(X, resid; p_compute = :exact). You can see this when running the last example of the test file (line 193). The warning is printed three times. Is this a general problem of the package that we should investigate or did I do something stupid?

ararslan · 2017-08-13T20:19:51Z

src/durbin_watson.jl

+function pan_algorithm(a::AbstractArray, x::Float64, m::Int, n::Int)
+
+    try # catch case where ν is empty
+        ν = find(a.>=x)[1]


findfirst(ai -> ai >= x, a)

will be more efficient

This is great as it allows me to get rid of the try-catch construct.

ararslan · 2017-08-13T20:20:41Z

src/durbin_watson.jl

+            pin = pi / (2n)
+            sum = 0.5 * (k + 1)
+            sgn = k / n
+            n2  = n + n -1


ararslan · 2017-08-13T20:21:43Z

src/durbin_watson.jl

+    catch
+        sum = 1.0
+    end
+


Missing a return here?

It seems I don't need one here as it returns the last value automatically. I eliminated the try-catch construct anyways. I have now also added tests to check these corner cases.

ararslan · 2017-08-13T20:22:13Z

src/durbin_watson.jl

+        # p-vales based on Pan's algorithm (see Farebrother, 1980)
+
+        # the following setup is, e.g, described in Durbin and Watson (1971)
+        A = diagm(-1 * ones(x.n - 1), -1) + diagm(-1 * ones(x.n - 1), 1) +diagm(


Can just use -ones(x.n - 1) rather than multiplying by -1

ararslan · 2017-08-13T20:30:08Z

src/durbin_watson.jl

+            end
+
+            pin = pi / (2n)
+            sum = 0.5 * (k + 1)


Typically it's preferred to avoid float literals in computations such as this, since float literals are always Float64, which will promote smaller float types to Float64 when they're used together in a computation. So in this case it would be sum = (k + 1) / 2 (though also note that it doesn't really matter, since that will promote to Float64 anyway, but the general point still stands 😛). It might be worthwhile to check @code_warntype pan_algorithm(<some args>) to see whether the function remains type-stable.

Thanks, makes sense. Never thought about that. I had checked for type instability in the algorithm with @code_warntype before and everything looked ok. Rechecked after making the changes and it's still fine.

ararslan · 2017-08-13T20:35:50Z

src/durbin_watson.jl

+
+export DurbinWatsonTest
+
+struct DurbinWatsonTest <: HypothesisTest


The package still supports Julia 0.5 but struct only parses on 0.6 and later. So this should be immutable until we drop 0.5 support.

ararslan · 2017-08-13T20:36:59Z

Is this a general problem of the package that we should investigate

Yeah, I could be wrong but I'm pretty sure it's the fault of the general show method and not a fault of this particular test.

ararslan · 2017-08-14T18:16:26Z

src/durbin_watson.jl

+
+    ν = findfirst(ai -> ai >= x, a)
+    if ν == 0
+        return sum = 1.0


Shouldn't need the sum =, right?

BenjaminBorn · 2017-08-15T19:31:13Z

I have opened an issue about the double computation of the p-value in #105.

ararslan

I'm not intimately familiar with Durbin-Watson but this implementation looks good to me, plus it matches the results from R.

BenjaminBorn · 2017-08-16T07:13:56Z

Thanks for the quick and very helpful review Alex!

BenjaminBorn · 2017-08-22T16:53:25Z

Any other comments?

nalimilan

Thanks, that's really cool! I would have had a few stylistic comments.

nalimilan · 2017-08-23T10:15:27Z

src/durbin_watson.jl

+export DurbinWatsonTest
+
+immutable DurbinWatsonTest <: HypothesisTest
+    xmat::Array{Float64}  # regressor matrix


If that's a matrix it should be Matrix{Float64} to use a concrete type.

nalimilan · 2017-08-23T10:16:03Z

src/durbin_watson.jl

+```
+where `n` is the number of observations.
+
+By default, the choice of approach to compute p-values depends on the sample size (`p_compute=:ndep`). For small samples (n<100), Pan's algorithm (Farebrother, 1980) is


Break line at 92 chars.

nalimilan · 2017-08-23T10:16:26Z

src/durbin_watson.jl

+  ](https://en.wikipedia.org/wiki/Durbin–Watson_statistic)
+"""
+function DurbinWatsonTest{T<:Real}(xmat::AbstractArray{T}, e::AbstractArray{T};
+    p_compute::Symbol = :ndep)


Incorrect indentation.

nalimilan · 2017-08-23T10:17:49Z

src/durbin_watson.jl

+    end
+
+    if exact_problem_flag == 1 || (x.p_compute == :ndep && x.n > 100
+        ) || x.p_compute == :approx


Parenthesis should be on previous line.

nalimilan · 2017-08-23T10:22:44Z

src/durbin_watson.jl

+    p_compute::Symbol = :ndep)
+
+    n = length(e)
+    DW = sum(diff(e) .^2) / sum(e .^2)


I don't know how much it can matter for performance, but you can use sum(abs2, diff(e))/sum(abs2, e) to avoid allocating copies. (diff itself allocates a vector, which could be avoided by using a small loop.)

nalimilan · 2017-08-23T10:24:47Z

src/durbin_watson.jl

+
+"""
+function pan_algorithm(a::AbstractArray, x::Float64, m::Int, n::Int)
+


The convention used in Julia and in JuliaStats packages is not to add blank lines at the top nor at the end of functions.

BenjaminBorn · 2017-08-23T10:53:57Z

Thanks Milan! I'll revisit this after the smoke has cleared from the documentation migration.

ararslan reviewed Aug 7, 2017

View reviewed changes

Initial commit Durbin-Watson test

6b25daa

BenjaminBorn force-pushed the durbin_watson branch from 6e80b73 to 6b25daa Compare August 11, 2017 17:19

BenjaminBorn changed the title ~~WIP: Durbin-Watson test~~ Add Durbin-Watson test Aug 13, 2017

ararslan reviewed Aug 13, 2017

View reviewed changes

BenjaminBorn force-pushed the durbin_watson branch from eeef5a2 to f63b98c Compare August 14, 2017 10:47

ararslan reviewed Aug 14, 2017

View reviewed changes

Add Pan's algorithm for exact p-values

a767e60

BenjaminBorn force-pushed the durbin_watson branch from f63b98c to a767e60 Compare August 14, 2017 19:09

BenjaminBorn mentioned this pull request Aug 15, 2017

P-value computed twice when calling a test #105

Open

ararslan approved these changes Aug 15, 2017

View reviewed changes

ararslan merged commit cf392fc into JuliaStats:master Aug 22, 2017

BenjaminBorn deleted the durbin_watson branch August 22, 2017 20:58

nalimilan reviewed Aug 23, 2017

View reviewed changes

BenjaminBorn mentioned this pull request Aug 25, 2017

List of tests common to time series analysis #16

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Durbin-Watson test #102

Add Durbin-Watson test #102

BenjaminBorn commented Aug 7, 2017

ararslan Aug 7, 2017

BenjaminBorn commented Aug 13, 2017

ararslan Aug 13, 2017

BenjaminBorn Aug 14, 2017

ararslan Aug 13, 2017

ararslan Aug 13, 2017

BenjaminBorn Aug 14, 2017 •

edited

Loading

ararslan Aug 13, 2017

ararslan Aug 13, 2017

BenjaminBorn Aug 14, 2017 •

edited

Loading

ararslan Aug 13, 2017

ararslan commented Aug 13, 2017

ararslan Aug 14, 2017

BenjaminBorn commented Aug 15, 2017

ararslan left a comment

BenjaminBorn commented Aug 16, 2017

BenjaminBorn commented Aug 22, 2017

nalimilan left a comment

nalimilan Aug 23, 2017

nalimilan Aug 23, 2017

nalimilan Aug 23, 2017

nalimilan Aug 23, 2017

nalimilan Aug 23, 2017

nalimilan Aug 23, 2017

BenjaminBorn commented Aug 23, 2017


		export DurbinWatsonTest

		struct DurbinWatsonTest <: HypothesisTest


		"""
		function pan_algorithm(a::AbstractArray, x::Float64, m::Int, n::Int)

Add Durbin-Watson test #102

Add Durbin-Watson test #102

Conversation

BenjaminBorn commented Aug 7, 2017

Choose a reason for hiding this comment

BenjaminBorn commented Aug 13, 2017

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

BenjaminBorn Aug 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjaminBorn Aug 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan commented Aug 13, 2017

Choose a reason for hiding this comment

BenjaminBorn commented Aug 15, 2017

ararslan left a comment

Choose a reason for hiding this comment

BenjaminBorn commented Aug 16, 2017

BenjaminBorn commented Aug 22, 2017

nalimilan 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

BenjaminBorn commented Aug 23, 2017

BenjaminBorn Aug 14, 2017 •

edited

Loading

BenjaminBorn Aug 14, 2017 •

edited

Loading