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

Add that minimum number of channels for defining cluster is 0 in documentation #13008

Open
kerrencasper opened this issue Dec 5, 2024 · 3 comments
Labels

Comments

@kerrencasper
Copy link

Proposed documentation enhancement

In mne.stats.permutation_cluster_test there is no way of defining the minimum number of neighbouring points that defines a cluster. This is a major error in the code, which makes the code invalid. This has been discussed by many of the developers for years, but nothing has been done (see issue #10604, #10604). Most people 1) don't know why this is a problem; 2) blindly trust the function. The least you can do if you are not ready to change it is to add to the documentation that this is a shortcoming of the function and that it should not be used until it has been solved. To exclude clusters with less than n number of channels (as has been discussed in the above-linked thread) is not a valid way forward as the t-values for these clusters have already been part of the t-distribution for defining the threshold for what can be considered significant.

Copy link

welcome bot commented Dec 5, 2024

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴

@larsoner
Copy link
Member

larsoner commented Dec 5, 2024

Hmm... I think our minimum number is effectively 1, which should make it so things are valid, right?

@sappelhoff
Copy link
Member

I think the answer to this issue lies in this comment: #10604 (comment)

Specifically this part:

For data that are sufficiently smooth I don't think it shouldn't matter much in practice because single-node "clusters" will have very small counts and stat values (which you get/use depends on t_power) so won't be the largest cluster(s) for each permutation used in the end for the maximal statistic.

I agree that in most cases it probably does not matter in practice, because data for which one would use cluster perm tests would usually also be "sufficiently smooth".

However, pre-defining the cluster extent could be a nice feature (currently hardcoded to "0", that is, zero neighbors required ... so one could also say "1" significant node). It exists in fieldtrip, see: https://www.fieldtriptoolbox.org/tutorial/stats/cluster_permutation_timelock/#the-configuration-settings

cfg.minnbchan = 2; % minimum number of neighborhood channels that is
% required for a selected sample to be included
% in the clustering algorithm (default=0).

☝ note how also in fieldtrip, the default is 0.

. To exclude clusters with less than n number of channels (as has been discussed in the above-linked thread) is not a valid way forward as the t-values for these clusters have already been part of the t-distribution for defining the threshold for what can be considered significant.

☝ this is true ... excluding clusters posthoc (AFTER the clustering) through masking would not be valid.

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

No branches or pull requests

3 participants