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

Adding statistical tests to ISC and ISFC #310

Merged
merged 7 commits into from Dec 13, 2017
Merged

Adding statistical tests to ISC and ISFC #310

merged 7 commits into from Dec 13, 2017

Conversation

@cbaldassano
Copy link
Collaborator

@cbaldassano cbaldassano commented Dec 11, 2017

Adding statistical testing through phase randomization, as requested by https://groups.google.com/forum/#!topic/brainiak/vZnOgGoV7q4 and as described in Simony et al. (2015, DOI: 10.1038/ncomms12141)

@cbaldassano cbaldassano requested a review from mihaic Dec 11, 2017
pos_freq = np.arange(1, (D.shape[1] - 1) // 2 + 1)
neg_freq = np.arange(D.shape[1] - 1, (D.shape[1] - 1) // 2, -1)

shift = np.random.rand(D.shape[0], len(pos_freq), D.shape[2]) * 2 * math.pi

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

We try to avoid np.random.rand in favor of np.random.RandomState.rand, according to the Scikit-learn guidelines:
https://github.com/brainiak/brainiak/blob/master/CONTRIBUTING.rst#standards
http://scikit-learn.org/stable/developers/contributing.html#random-numbers

D[:, :, 2] = \
[[-0.26593192, -0.56914734, 0.28397317, 0.30276589, -0.42637472],
[-0.67598379, -0.51549055, -0.64196342, 1.60686666, 0.04127293]]
[0.04127293, -0.67598379, -0.51549055, -0.64196342, 1.60686666]]

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

Why is the reordering necessary?

This comment has been minimized.

@cbaldassano

cbaldassano Dec 12, 2017
Author Collaborator

Added a comment to clarify, I wanted to create a null ISC to make sure it was not significant.

@@ -35,13 +40,20 @@ def test_ISFC():

assert D.shape == (4, 5, 2), "Loaded data has incorrect shape"

ISFC = brainiak.isfc.isfc(D)
np.random.seed(0)

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

Don't forget to remove this seed setting when you switch to using RandomState.

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

Did you actually forget to remove the line? :)

return ISFC, p


def phase_randomize(D):

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

This function is not really ISFC specific. I think it fits better in utils/utils.py.

back into the time domain. This yields timecourses with the same power
spectrum (and thus the same autocorrelation) as the original timecourses,
but will remove any meaningful temporal relationships between the
timecourses.

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

Do you have a paper reference by any chance?

return np.real(ifft(F, axis=1))


def ecdf(x):

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

utils/utils.py?

return ISFC[..., 0]


def p_from_null(X, two_sided=False):

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

I think this function also belongs in utils/utils.py.

@@ -35,13 +40,20 @@ def test_ISFC():

assert D.shape == (4, 5, 2), "Loaded data has incorrect shape"

ISFC = brainiak.isfc.isfc(D)
np.random.seed(0)

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

Did you actually forget to remove the line? :)

@@ -667,3 +671,78 @@ def usable_cpu_count():
except AttributeError:
result = os.cpu_count()
return result


def phase_randomize(D, random_state=0):

This comment has been minimized.

@mihaic

mihaic Dec 12, 2017
Contributor

Please add tests for the new functions. They can be as simple as you want.

@mihaic
mihaic approved these changes Dec 13, 2017
Copy link
Contributor

@mihaic mihaic left a comment

Thank you for the prompt changes, @cbaldassano.

@mihaic mihaic merged commit b0a34e7 into brainiak:master Dec 13, 2017
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@buildbot-princeton
linux Build finished.
Details
@buildbot-princeton
macos Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants