Fcma classification by yidawang · Pull Request #122 · brainiak/brainiak

yidawang · 2016-09-29T19:46:21Z

add correlation-based classification of FCMA
restructure the example code of FCMA
fix minor typos

mihaic · 2016-09-29T20:07:17Z

+
+        Parameters
+        ----------
+        X: a list of numpy array in shape [nun_TRs, num_voxels]


Typo, num_TRs

mihaic · 2016-09-29T20:12:04Z

+        Parameters
+        ----------
+        X: a list of numpy array in shape [nun_TRs, num_voxels]
+            len(X) equals num_test_samples


As far as I understand, you are defining the variable num_test_samples here in the docstring. This is confusing. Use len(X) in this docstring, not num_test_samples.

mihaic · 2016-09-29T20:16:51Z

+                 epochs_per_subj=0):
+        self.clf = clf
+        self.epochs_per_subj = epochs_per_subj
+        self.training_data = None


Only set parameters passed to __init__. So do not set training_data, num_voxels, and num_samples. If you are thinking about it, do not add them as parameters to __init__, because they are data dependent. See the scikti-learn documentation:
http://scikit-learn.org/stable/developers/contributing.html#instantiation

mihaic · 2016-09-29T20:21:17Z

+        num_samples = len(X)
+        num_voxels = X[0].shape[1]  # see assumption above
+        self.num_voxels = num_voxels
+        self.num_samples = num_samples


You should add an _ to attributes set based on the data:
http://scikit-learn.org/stable/developers/contributing.html#estimated-attributes

mihaic · 2016-09-29T20:22:25Z

+            len(X) equals num_test_samples
+            if num_test_samples > 0: normalization is done
+            on all test samples
+            num_voxels equals the one used in the model


self.num_voxels_?

mihaic · 2016-09-29T20:25:13Z

+        """ use a trained model to predict correlation data
+
+        the input data X is activity data filtered by top voxels
+        and prepared for correlation computation.


Move this explanation to the definition of X below.

mihaic · 2016-09-29T20:25:55Z

+        X needs to be first converted to correlation,
+        and then normalized across all samples in the list
+        if len(X) > 1,
+        and then predicted via self.clf.


If this is the description of what the method does, change to phrasing from "needs to be" to "is".

mihaic · 2016-09-29T20:39:51Z

+                     create_epoch(), create_epoch(),
+                     create_epoch(), create_epoch(),
+                     create_epoch(), create_epoch(),
+                     create_epoch(), create_epoch()]


How about [create_epoch() for i in range(20)]?

yidawang

thanks for the review

yidawang · 2016-09-29T21:57:41Z

+
+        Parameters
+        ----------
+        X: a list of numpy array in shape [nun_TRs, num_voxels]


yidawang · 2016-09-29T21:58:02Z

+        """ use a trained model to predict correlation data
+
+        the input data X is activity data filtered by top voxels
+        and prepared for correlation computation.


yidawang · 2016-09-29T21:58:14Z

+        X needs to be first converted to correlation,
+        and then normalized across all samples in the list
+        if len(X) > 1,
+        and then predicted via self.clf.


yidawang · 2016-09-29T21:58:22Z

+        Parameters
+        ----------
+        X: a list of numpy array in shape [nun_TRs, num_voxels]
+            len(X) equals num_test_samples


yidawang · 2016-09-29T21:58:27Z

+            len(X) equals num_test_samples
+            if num_test_samples > 0: normalization is done
+            on all test samples
+            num_voxels equals the one used in the model


yidawang · 2016-09-29T21:58:48Z

+                     create_epoch(), create_epoch(),
+                     create_epoch(), create_epoch(),
+                     create_epoch(), create_epoch(),
+                     create_epoch(), create_epoch()]


yidawang · 2016-09-29T21:59:03Z

+        num_samples = len(X)
+        num_voxels = X[0].shape[1]  # see assumption above
+        self.num_voxels = num_voxels
+        self.num_samples = num_samples


mihaic · 2016-09-29T23:08:01Z

+
+        Parameters
+        ----------
+        X: a list of numpy array in shape [num_TRs, num_voxels]


How about self.num_voxels_? Then you can also remove the last line below.

mihaic · 2016-09-29T23:09:37Z

+
+logger = logging.getLogger(__name__)
+
+def readActivityData(dir, file_extension, mask_file):


PEP 8 names, please.

* Ignore /build directory * Some Windows and Visual Studio .gitignores

* First attempt at fixing psubscribe. psubscribe_success_test will fail * psubscribe test * SUBSCRIBE returns the number of subscriptions, not success * Comment out failing test.

yidawang added 17 commits September 26, 2016 21:37

start working on fcma classification

550825d

use gemm to compute kernel matrix for training

abcf911

prediction is in

3ce46f3

use blas to compute similarity matrix, test

f676368

use gemm to compute similarity matrix in prediction

7e77275

Merge remote-tracking branch 'upstream/master' into fcma_classification

4f21899

add model dump and load; try logestic regression

4dac89d

formatting; add test code for fcma classification

af6026b

use Hamming distance to measure the results and the expected results

ab5449f

more samples for fcma classification testing

05b8efa

docstrings to cython_blas.pyx

927df34

add number of training samples as an element of Classifier of FCMA

932c514

typo

ee6e36c

Merge remote-tracking branch 'upstream/master' into fcma_classification

e6bcf64

Merge remote-tracking branch 'upstream/master' into fcma_classification

e910a5a

docstrings of classifier.py

f843f72

typo

8974ca6

mihaic requested changes Sep 29, 2016

View reviewed changes

impose a pattern to test_classification; address comments of PR reviews

a2dd999

yidawang commented Sep 29, 2016

View reviewed changes

add Attributes to the class docstring

c80b884

mihaic requested changes Sep 29, 2016

View reviewed changes

use PEP8 name convention; escape for in docstring

d1581df

mihaic approved these changes Sep 30, 2016

View reviewed changes

mihaic merged commit 33cc14b into brainiak:master Sep 30, 2016

yidawang deleted the fcma_classification branch September 30, 2016 04:11

danielsuo pushed a commit that referenced this pull request Nov 16, 2017

Some Visual Studio gitignores (#122)

68d752c

* Ignore /build directory * Some Windows and Visual Studio .gitignores


		logger = logging.getLogger(__name__)

		def readActivityData(dir, file_extension, mask_file):

Conversation

yidawang commented Sep 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaic Sep 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yidawang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mihaic Sep 29, 2016 •

edited

Loading