MIssing plots in granger_fmri.html about nitime HOT 14 CLOSED

@arokem It seems that the fix is failing the test_granger.py test. Now I actually think that proposed fix is not ideal, it would break with old behavior. We need to be more careful.

from nitime.

Thanks for spotting this. Looks like a lot of nan's in the results from Granger Causality calculation. Since it's been a while since the docs were built, I'll probably need to bisect back in history to see where this happened. Unfortunately, it might take me a little while to get to it.

from nitime.

from nitime.

from nitime.

Thanks. Just curious because the issue got closed but the problem seems to remain...? There is also a question on the Neuroimaging mailing list about it,

Subject: [Neuroimaging] GC analysis with nitime
Date: Fri, Jul 5, 2019 at 7:37 PM

Should the issue be reopened until it gets addressed?

from nitime.

Clearly.

from nitime.

Hi, I had the same issue when trying to run the Granger causality example for fMRI on my own computer. I tried rolling back to older versions of nitime (0.7, 0.6, and 0.5) but they had their own problems with dependent packages. Is there any way on my end to make the Granger causality module functional? It is exactly the tool I am looking for in my research. Thanks.

from nitime.

Hi all,
I spotted the same issue. Although being totally ignorant about the inner maths about Granger, I think I figured out where the problem lies.

The GrangerAnalyzer puts its outputs in the upper triangle:

        if ij is None:
            # The following gets the full list of combinations of
            # non-same i's and j's:
            x, y = np.meshgrid(np.arange(self._n_process),
                               np.arange(self._n_process))
            self.ij = list(zip(x[tril_indices_from(x, -1)],
                          y[tril_indices_from(y, -1)]))

Note the order is x then y, so self.ij indexes into the upper triangle.

While in making the plot, drawmatrix_channels() leaves only the lower triangle:

    # Null the upper triangle, so that you don't get the redundant and the
    # diagonal values:
    idx_null = triu_indices(m.shape[0])
    m[idx_null] = np.nan

So we ended up with nothing.

I suppose the fix is to transpose the ordering in GrangerAnalyzer like so:

           self.ij = list(zip(y[tril_indices_from(y, -1)],
                          x[tril_indices_from(x, -1)]))

This gives the following image for the fig03 in the tutorial:

fig04 is like this:

But, I'm not familiar with the plotting conventions people use in such like of analyses. Do we assume the plot in fig03 is the "causality" from row to column, or the other way around? Please help verity the proposed fix.

from nitime.

Thanks for figuring this out! I think that the best fix would probably be to alter the behavior of drawmatrix_channels. Do you want to try to make a PR for that?

from nitime.

@arokem I'm not sure what exact changes to make to drawmatrix_channels(), because it's working fine for other plots and only fails for Granger. How to change it so we don't break compatibility with other tutorials and with older versions?

from nitime.

Gotcha. Yeah - I think that your original solution makes more sense then. Arguably, this is the design we should have implemented to begin with. Could you make a PR with that implemeted?

from nitime.

Yes - we need to adjust the test and warn about the break to backwards compatibility.

If you reopen the PR, we can discuss that there.

from nitime.

I made some back-n-forth changes, hopefully not creating too much confusion to the git history.
In the latest PR I changed the drawmatrix_channels() instead to use the upper triangle but transpose it to lower before plotting. This doesn't affect other tutorials because coherence, correlation etc. are all symmetrical. I've tested all scripts in examples that calls drawmatrix_channels() and the figures all look good.

from nitime.

Closed through #193

from nitime.