-
Notifications
You must be signed in to change notification settings - Fork 87
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
Add Diebold-Mariano test #180
Conversation
Once you've computed the test statistic, this is "just" a t-test. So you should make Line 31 in 7f21943
|
I would recommend adding the Clark-West test (for nested models), where the test is based on
assuming that f1 is the smaller model and that EDIT: I believe it is well recognized that applying a DM test for nested models is a non-trivial mistake and that either (a) a bootstrap; (b) moving data window (rather than an expanding ones) or (c) a CW test are the possible ways to handle this case. In case it's too late for introducing a CW test in this PR, please let me know so I'll do it in a new PR. |
please see my updated comment (above) |
@PaulSoderlind I think it is better to make the Clark-West test in another PR. |
how do we move on with the Clark-West? I am happy to contribute, but I must admit that I am not really an expert on the API of this package. One idea would be to just have it as an option in the DM test. This would be easy to both implement and make a PR of. What do you think? |
@PaulSoderlind I think the easiest way to implement this is basically to copy this file in a new one named clark_west.jl and change the expression for d for the one you commented. Moving into a new After making the new file you should add some tests and add it to the time series tests section on the documentation. |
@guilhermebodin OK, sounds right. Maybe quicker if you take the lead? (I could review.) |
@ararslan is it good to merge? |
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Alex Arslan <ararslan@comcast.net>
Co-authored-by: Moritz Schauer <moritzschauer@web.de>
Looks quite good to me. @PaulSoderlind, I am unsure how to read your comment, do you think this is good to go as well? |
My comment referred to the idea of adding another PR for a Clark-West test.
Anyhow I could review this PR. Just a stupid question before a try to do
that: how do I see the latest version of the PR (after all the updates)?
It's a bit of a hassle to wade through all the diffs.
…On Wed, 20 May 2020 at 22:26, Moritz Schauer ***@***.***> wrote:
Looks quite good to me. @PaulSoderlind <https://github.com/PaulSoderlind>,
I am unsure how to read your comment, do you think this is good to go as
well?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#180 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACHRTVL57YUHO5HD3VJDCZDRSQ4H5ANCNFSM4KUBXAIA>
.
|
The "files changed" tab above shows the accumulated changes, https://github.com/JuliaStats/HypothesisTests.jl/pull/180/files |
On the src/diebold_mariano.jl: the DM test looks fine (and gives the same result as I get from my own home-brewed code). I don't understand the |
Closes the issue #179 .
All tests were based on the Diebold-Mariano implementation on the forecast package of R. The script is the following.