search

sccn / SIFT

SIFT is an EEGLAB-compatible toolbox for analysis and visualization of multivariate causality and information flow between sources of electrophysiological (EEG/ECoG/MEG) activity. It consists of a suite of command-line functions with an integrated Graphical User Interface for easy access to multiple features. There are currently six modules: data preprocessing, model fitting and connectivity estimation, statistical analysis, visualization, group analysis, and neuronal data simulation.
https://sccn.ucsd.edu/wiki/SIFT
Other
31 stars 5 forks source link

stat_surrogateStats calculates right-tailed p-values, even for statTest.tail='left' #2

Open tstenner opened 3 years ago

tstenner commented 3 years ago

stat_surrogateStats forwards the statTest.tail parameter to stat_surrogate_pvals which does a two-tailed test when statTest.tail is both and a right-tailed test for all other inputs.

Despite this, stat_surrogateStats prints

'This is a left-sided test for significant differences between conditions'

even though the p values are for a right sided test.

arnodelorme commented 3 years ago

The solution would thus be to disable left for statTest.tail then, right?

tstenner commented 3 years ago

That's one possibility, but I'm currently thinking about implementing it instead. Would be easier if Matlab had a binary search built in, but I'll manage without it.

arnodelorme commented 3 years ago

Yes, would be great if you can implement. Cheers,

Arno

On Feb 24, 2021, at 8:20 AM, Tristan Stenner notifications@github.com wrote:

That's one possibility, but I'm currently thinking about implementing it instead. Would be easier if Matlab had a binary search built in, but I'll manage without it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

trmullen commented 3 years ago

Tristan, thanks for this catch - I don't have the code in front of me, but if it doesn't have the left-tailed test and you feel up to adding/fixing that and submitting a PR (should be a one line change for the new case) that will be much appreciated!

I believe you also had some other bug fixes - want to add those as PRs too?

Thanks! Tim

On Wed, Feb 24, 2021 at 10:24 AM Arnaud Delorme notifications@github.com wrote:

Yes, would be great if you can implement. Cheers,

Arno

On Feb 24, 2021, at 8:20 AM, Tristan Stenner notifications@github.com wrote:

That's one possibility, but I'm currently thinking about implementing it instead. Would be easier if Matlab had a binary search built in, but I'll manage without it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sccn/SIFT/issues/2#issuecomment-785278858, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5LRG5QBXCON5C4WXBOC43TAU75DANCNFSM4YENQ5EQ .

tstenner commented 3 years ago

As far as I can see, the SIFT function is functionally identical to the one in eeglab\functions\statistics\, so rather than fix this one I propose to remove it from SIFT and once https://github.com/sccn/eeglab/pull/299 is merged add a check against the EEGLAB version.