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

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

alexrockhill
Copy link
Contributor

PR Description

Fixes #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
Copy link
Contributor Author

alexrockhill commented Jun 21, 2022

@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
Copy link
Contributor Author

@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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

@alexrockhill how's this going?

Are my additions sufficient for what you were thinking?

@alexrockhill
Copy link
Contributor Author

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
Copy link
Member

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.

doc/references.bib Outdated Show resolved Hide resolved
@alexrockhill
Copy link
Contributor Author

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
Copy link
Member

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.

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.

[ENH] Add conn.plot_circle to Dynamic Connectivity Example (and Estimation of Significance?)
3 participants