-
Notifications
You must be signed in to change notification settings - Fork 34
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
base: main
Are you sure you want to change the base?
Conversation
@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... |
@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... |
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:
|
It's all good, I hope you had a good time!
Oh super interesting, so maybe we should stick with this. If we do that, would you mind writing some of the interpretation?
Because things don't work properly :) I can look into it
Yeah I think so it would just take a bit of doing. |
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. |
@alexrockhill how's this going? Are my additions sufficient for what you were thinking? |
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'. |
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. |
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. |
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. |
PR Description
Fixes #96.
Merge checklist
Maintainer, please confirm the following before merging: