[ENH, WIP] Add Estimation of Significant Connectivity

mne-tools / mne-connectivity

Connectivity algorithms that leverage the MNE-Python API.

https://mne.tools/mne-connectivity/dev/index.html

BSD 3-Clause "New" or "Revised" License

66 stars 34 forks source link

[ENH, WIP] Add Estimation of Significant Connectivity #101

Open alexrockhill opened 2 years ago

alexrockhill commented 2 years ago

PR Description

Fixes https://github.com/mne-tools/mne-connectivity/issues/96.

Merge checklist

Maintainer, please confirm the following before merging:

[ ] All comments resolved
[ ] This is not your own PR
[ ] All CIs are happy
[ ] PR title starts with [MRG]
[ ] whats_new.rst is updated
[ ] PR description includes phrase "closes <#issue-number>"

alexrockhill commented 2 years ago

@adam2392, if you want to review this, that would be great. I'll link the artifact when it renders.

I'm not sure what to make of the data and also how to correct for multiple comparisons to have a reasonable estimation of greater-than-chance connectivity. I think a cross-validation approach might be a bit cleaner, that's going to take even more compute time though...

alexrockhill commented 2 years ago

https://output.circle-artifacts.com/output/job/5f2842e0-889a-4712-99e6-a3e81f5b795f/artifacts/0/dev/auto_examples/dynamic/mne_var_connectivity.html#sphx-glr-auto-examples-dynamic-mne-var-connectivity-py

alexrockhill commented 2 years ago

@adam2392 , when you have a chance, what do you think about this? I'm not sure the results make sense, perhaps it would be easier to use an example dataset where the brain activity is more well-known like MNE sample? That way you should see audio and visual connectivity to know things are generally sensible...

adam2392 commented 2 years ago

Sorry for the delay @alexrockhill . I was out of country on vacation till last night.

So I think overall this is a useful feature since the time-shuffled idea works in practice if you have some type of stationarity and you're interested in just estimating 1 connectivity pattern. I think the issue is interpreting it for this specific example.

Perhaps one thing we can say is the 'AST' channel region consistently has significant connections, which is interesting considering that region was "considered" epileptic by the clinicians. Reference: https://openneuro.org/datasets/ds003029

A few minor comments:

why are there multiple color bars?
can we make the plot a bit bigger to show the actual channel labels?

alexrockhill commented 2 years ago

Sorry for the delay @alexrockhill . I was out of country on vacation till last night.

It's all good, I hope you had a good time!

So I think overall this is a useful feature since the time-shuffled idea works in practice if you have some type of stationarity and you're interested in just estimating 1 connectivity pattern. I think the issue is interpreting it for this specific example.

Perhaps one thing we can say is the 'AST' channel region consistently has significant connections, which is interesting considering that region was "considered" epileptic by the clinicians. Reference: https://openneuro.org/datasets/ds003029

Oh super interesting, so maybe we should stick with this. If we do that, would you mind writing some of the interpretation?

A few minor comments:

why are there multiple color bars?

Because things don't work properly :) I can look into it

can we make the plot a bit bigger to show the actual channel labels?

Yeah I think so it would just take a bit of doing.

adam2392 commented 2 years ago

Oh super interesting, so maybe we should stick with this. If we do that, would you mind writing some of the interpretation?

Sounds good! Can do. Ping me when you have the other issues fixed and I'll add some inline comments to the text?

alexrockhill commented 2 years ago

Oh super interesting, so maybe we should stick with this. If we do that, would you mind writing some of the interpretation?

Sounds good! Can do. Ping me when you have the other issues fixed and I'll add some inline comments to the text?

I don't think the other issues really effect the interpretation so go for it whenever. I can work on fixing those next week.

adam2392 commented 2 years ago

@alexrockhill how's this going?

Are my additions sufficient for what you were thinking?

alexrockhill commented 2 years ago

I think it's close, I haven't had a chance to look into the failures. I think it's a bit hand wavy for my taste for package documentation, it would be nice if there was a bit more understanding of the connectivity and not just 'that looks somewhat reasonable'.

adam2392 commented 2 years ago

True, but that's a limitation in general for correlation based connectivity analysis.

I think as long as the assumption of stationarity is said, then the time shuffling is theoretically sound and not hand wavy.

The caveat is in practice you don't know whether or not something is stationary.

alexrockhill commented 1 year ago

Hey @adam2392, I had a minute to work on this but the results seem a bit odd; the connectivity of the null distribution are very non-normally distributed, most values are below ten and then some are in the hundred thousand or millions. I'm not sure how to account for this, it seems very difficult to compute reasonable statistics if these are the actual values... if you have a minute, it would be great if you could give it a run and see if you have any advice on how to make things more reasonable.

Also, unrelatedly, looks like the CIs need some fixes.

adam2392 commented 1 year ago

Hey @alexrockhill sorry for the late reply.

Statistical testing with especially non-symmetric connectivity matrices is a pretty challenging problem, so I'm not 100% sure how I would go about this tbh. I'm wary of trying too hard to make it work because I'm not that familiar with permutation block shuffling to do inference on connectivity matrices.