Skip to content

Corr comp - #159

Merged
mihaic merged 12 commits into
brainiak:masterfrom
yidawang:corr_comp
Jan 13, 2017
Merged

mihaic merged 12 commits into
brainiak:masterfrom
yidawang:corr_comp

Conversation

@yidawang

Copy link
Copy Markdown
Member

Add a high-performance correlation computation function using BLAS wrapped in Cython to brainiak/fcma/util. Running at least one order of magnitude faster than np.corrcoef.

Ours: 100 loops, best of 3: 14.2 ms per loop
np.corrcoef: 10 loops, best of 3: 153 ms per loop

Comment thread brainiak/fcma/util.py Outdated
Correlate the rows of set1 with the rows of set2.
If set1 == set2, it is auto-correlation computation
resulting in a symmetric correlation matrix.
The number of columns MUST agree between set1 and set2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a reference for the method of correlation computation you are using? Some Latex code could help too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread brainiak/fcma/util.py Outdated
return data


def compute_correlation(set1, set2):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the parameters be called array1/2 or matrix1/2?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are essentially two matrices, depicting two sets of variables. I can use matrix1/2

Comment thread brainiak/fcma/util.py Outdated
is the standard deviation of variable X

Reducing the correlation computation to matrix multiplication
and use BLAS GEMM API wrapped by Scipy can speedup the numpy build-in

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use->using, build-in->built-in

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread brainiak/fcma/util.py Outdated
import math


def normalize_for_correlation(data, axis):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary for this function to be public?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May or may not be used by others in the future. But for now, let me make it private

@mihaic
mihaic merged commit 6d4c11d into brainiak:master Jan 13, 2017
@yidawang
yidawang deleted the corr_comp branch January 13, 2017 07:49
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.

2 participants