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

Improvements to Granger Causality evaluation #433

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

DavideNuzzi
Copy link

Modified bst_granger.m and bst_granger_spectral.m
Their results were not correct (when compared to other popular frameworks like MVGC). In particular, bst_granger.m seemed to rely on a double VAR model identification, which is known to yield incorrect results. My correction solves the Yule-Walker equations under arbitrary constraints (of the VAR model couplings) and can be readily generalized to evaluate conditional Granger causality and multivariate Granger causality (both in the time and frequency domains).

@ftadel
Copy link
Member

ftadel commented Aug 20, 2021

@HosseinShahabi @rcassani @richardmleahy
Could you please review and discuss these requested modifications?
Thanks

@HosseinShahabi
Copy link
Contributor

Dear @DavideNuzzi

Thank you very much for submitting this pull request. We appreciate your interest in Brainstorm and your desire to improve it.

We are glad that you raised this important issue and we understand that the recent research has emphasized the pros of the new methodology for the computation of Granger causality in comparison to the traditional approach.

As you mentioned, the MVGC toolbox is a well-known software for GC computation. When I checked your code, I realized the approach you employed for the time-domain GC is similar to one in the MVGC framework. However, your code for the spectral GC is based on another article.

While we are trying to improve our software based on your suggestions, I have the following questions from you:

Is there any specific reason that you selected not to use the MVGC approach for spectral GC? Also, does your code uses a better algorithm for the computation of GC?

I understand that you personalized the new algorithm for our toolbox but do you think there will be any problem if we fully integrate the MVGC toolbox in Brainstorm?

Looking forward to collaborating with you,
Hossein

@ftadel
Copy link
Member

ftadel commented Sep 23, 2021

@HosseinShahabi
Thank you for reviewing this PR and for your report.

@DavideNuzzi
Our interest for using an existing toolbox instead of customized code is related mostly to the maintenance and the support of the software. If the code is maintained directly by the authors of the methods, it is a guarantee that all the bugs will be fixed and that it will be updated to work with the future versions of Matlab, and we can redirect all the users' questions and bug reports to the library authors. It means much less work us on the long term. When possible, reusing existing packages without modifications is a good open-source practice.

Brainstorm includes now an easy-to-use package manager to download third-party libraries, so downloading the MVGC toolbox would require only to add a few lines of code to bst_plugins: https://neuroimage.usc.edu/brainstorm/Tutorials/Plugins

Let us know what you think - Thanks

@ftadel
Copy link
Member

ftadel commented Dec 8, 2021

@HosseinShahabi
Could you please provide an update on your work?
Thanks

@ftadel
Copy link
Member

ftadel commented Feb 3, 2022

@DavideNuzzi Please be patient: @HosseinShahabi is still planning on working on this.

@ftadel
Copy link
Member

ftadel commented Apr 18, 2022

@HosseinShahabi Any update?

@AdelleBernal
Copy link

What is the status of this modification and how does it affect the current GC measures in the current BrainStorm version?

@Edouard2laire
Copy link
Contributor

Hi.

I just started reading an article using Brainstorm for functional connectivity (including Granger causality).

Their results were not correct (when compared to other popular frameworks like MVGC). In particular, bst_granger.m seemed to rely on a double VAR model identification, which is known to yield incorrect results

Based on this sentence, how much should we trust the result using brainstorm GC ? Is there any plan to move forward with this PR ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants