Voxelwise glm residuals/R2 #173

Gilles86 · 2018-03-16T10:51:57Z

This commit makes it possible to calculate voxelwise residuals and R-squareds for a given GLM. Note that you can only do so when the "full" RegressionResults are returned (by setting minimize_memory of the FirstlevelModel to False). When this is asked from a SimpleRegressionResults, an exception with explanation is thrown.

I also fixed this issue:
https://github.com/nistats/nistats/issues/150

…or consistence

…ally residuals, predicted signal and R-squared

bthirion · 2018-03-18T22:46:25Z

I'll check more in detail tomorrow -- hopefully-- but you need to add test for these functionalities to ensure that they work properly.

KamalakerDadi · 2018-03-19T09:24:43Z

If I understood correctly, you are trying to fix two issues in the same PR. It makes reviewing harder. I think it is better to fix one issue in a PR. My general comment.

Gilles86 · 2018-03-19T11:15:11Z

If I understood correctly, you are trying to fix two issues in the same PR. It makes reviewing harder. I > think it is better to fix one issue in a PR. My general comment.

OK, I made a separate pull request for that #174

…and FirstLevelModel

Gilles86 · 2018-03-19T13:56:09Z

I'll check more in detail tomorrow -- hopefully-- but you need to add test for these functionalities to ensure that they work properly.

OK. I added some tests in last commit.

bthirion · 2018-05-12T17:18:13Z

nistats/first_level_model.py

+        else:
+            return output
+
+    @property


We tend to avoid properties, which makes the code hard to undertand and debug.

bthirion · 2018-05-12T17:19:04Z

nistats/regression.py


-    @setattr_on_read
+    @property
    def resid(self):


residuals. I would not make it a property.

bthirion · 2018-05-12T17:19:34Z

nistats/regression.py

@@ -310,28 +311,35 @@ def norm_resid(self):
        """
        return self.resid * pos_recipr(np.sqrt(self.dispersion))

-    @setattr_on_read
+    @property


Same comment.

bthirion · 2018-05-12T17:22:13Z

nistats/tests/test_first_level_model.py

-    # assert that we only have one theta as there is only on voxel in our image
-    assert_true(first_level_model.results_[0][0].theta.shape == (1, 1))
-    # assert that the theta is equal to the one voxel value
+#def test_high_level_glm_one_session():


Please do not leave commented code:

jdkent · 2018-11-05T20:06:18Z

@Gilles86: Do you still have time to work on this?

Just an interested third party since I would be rather happy to investigate regional model fit using nistats.

Gilles86 · 2018-11-06T09:50:03Z

Hey @jdkent ,

Yes. Time is always so precious, since so much is expected from new postdocs. I would still really like to finish this sometime soon though. Your ping motivates me to put it on my todo for the coming weeks again :).

- Don’t report peaks in matrix space. - Report cluster size in mm3 instead of voxels. - Report top 3 subpeaks more than 8mm separated when available. - Add argument for voxel-connectivity for cluster definition.

Also, I removed the connectivity argument for get_clusters_table. I kept the separate neighborhood matrix function (`_get_conn`), just in case. Currently, it uses 6-connectivity (aka NN1, faces-only connectivity, or first-nearest neighbor clustering).

Also, I’ve made the subpeak detection explicitly work around binary clusters. Now it just reports the center of mass.

Numpy did not have `cbrt` function in 1.8.2, the version used for Python2.7 CI.

_local_max now checks distance between subpeaks explicitly, since the approach used in the StackOverflow post wasn’t quite doing it.

- Use minimum distance in millimeters in _local_max. - Requires including the image affine in the arguments as well. - Should also be robust to non-isotropic voxels. - Add min_distance argument to get_clusters_table, for minimum distance in millimeters. Default is 8mm. - Address PEP8 problems.

…ered rebuilds

…ntless.

…mination) - Renamed manual-cache-timestamp.txt to manual-cache-timestamp

…building

Gilles86 · 2018-12-07T14:35:04Z

OK. I rebased and revised the code a bit.

I notice that in the regression-module, the setattr_on_read-class attribute is now used. E.g., here:
https://github.com/nistats/nistats/blob/d97ea36198fe36c8846dd98c3c8118f3ed27080c/nistats/regression.py#L286

I thus now also used it in the FirstLevelModel-object

kchawla-pi · 2018-12-07T14:58:19Z

Hey @Gilles86 thanks so much for attending to this. It has been a while so the conflicts in master might take more work than usual to resolve. Let me know if I can help.

…o voxelwise_glm_stats

Gilles86 · 2018-12-07T15:17:08Z

Yes. I'm sorry that the commit history got a bit messy. I'm not very used to work with multiple branches like this. We'll get there though.

Gilles86 · 2018-12-07T16:21:29Z

Yup. Tests passed. Anyone?

kchawla-pi · 2018-12-07T16:23:21Z

The good news is CI is green. The not yet good news is the commit history now makes it impossible to review.
We'll figure out how to fix that. I made this mistake as well once (so far) as a contributor. #gitlife
@Gilles86
Could you write the exact command you used here to bring your branch up to date with master?
Your other PR #174 also has this problem, could you type the commands you used to update that one as well?
Update: Okay well maybe not impossible, but if this can e cleaned up we should do that.

Gilles86 · 2018-12-07T16:54:25Z

I'm sorry. I'm learning.

I made a clean version of this pull request:
#298

kchawla-pi · 2018-12-07T18:40:55Z

Hey @Gilles86 that's totally fine, you are helping us out here.
Thanks for this!

kchawla-pi · 2018-12-07T19:28:24Z

@Gilles86 Never mind the cleaned up one. let's continue working here. I was concerned about GIthub showing number of files changed to be 49 something (which was in your other PR and makes it very difficult to review) but in this one it's showing the files pertinent to this PR only so it's fine.

Bertrand reviewed the PR earlier, we can start by working on those.

Gilles86 added 7 commits March 14, 2018 15:00

Fixed SimpleRegressionResults

a82e612

Make resid() and norm_resid() in SimpleRegressionResults @Property, f…

b45b294

…or consistence

Fixed signal_scaling

9de71c6

Merge branch 'fix_first_level_signal_scaling' into fix_firstlevel

93a8fd2

Merged upstream

285aa81

Allow for various voxelwise statistics in first level model, specific…

d00e77c

…ally residuals, predicted signal and R-squared

Small typo

78e2dfc

Gilles86 changed the title ~~Voxelwise glm stats~~ Voxelwise glm residuals/R2 Mar 16, 2018

Unit tests for residuals, predictions and rsq at level of Regression …

01cfa46

…and FirstLevelModel

bthirion reviewed May 12, 2018

View reviewed changes

kchawla-pi assigned kchawla-pi and Gilles86 Dec 6, 2018

tsalo added 12 commits December 7, 2018 15:13

Move get_clusters_table from thresholding to reporting.

4ff98a0

- Don’t report peaks in matrix space. - Report cluster size in mm3 instead of voxels. - Report top 3 subpeaks more than 8mm separated when available. - Add argument for voxel-connectivity for cluster definition.

Add skimage to requirements.

eff5d5e

Also, I removed the connectivity argument for get_clusters_table. I kept the separate neighborhood matrix function (`_get_conn`), just in case. Currently, it uses 6-connectivity (aka NN1, faces-only connectivity, or first-nearest neighbor clustering).

Move get_clusters_table tests from thresholding to reporting.

75c923a

Also, I’ve made the subpeak detection explicitly work around binary clusters. Now it just reports the center of mass.

Fix get_clusters_table documentation and improve warnings.

6e1ba9e

Switch from skimage to scipy for subpeak identification.

7def86a

Remove skimage dependency.

76f0b18

Switch np.cbrt(n) with np.power(n, 1/3.) for Travis CI.

e3fa100

Numpy did not have `cbrt` function in 1.8.2, the version used for Python2.7 CI.

Remove _get_conn and hardcode in 6-connectivity.

3e932cf

Improve _local_max and add test.

c28511d

_local_max now checks distance between subpeaks explicitly, since the approach used in the StackOverflow post wasn’t quite doing it.

Fix examples and docs.

e31d134

Group scipy imports together to trigger CI

02a832e

kchawla-pi and others added 13 commits December 7, 2018 15:25

Spurious commit to trigger the disappeared TravisCI

53e7a9b

Fixed: param img_filters no longer has a mutable arg ([]) as default

909c705

Reformatted commands for clarity after discussion with Gael

26ef3cf

Added mechanism to generate new cache by external contributors

544deb5

Renamed cache timestamp's filename; clarify its only for manual trigg…

e5f9bdd

…ered rebuilds

Changed date format to be internation for manual cache rebuild trigger

e848331

Manually triggering a fresh cache build for testing

920faf4

Removed the provision for caching post master merge since it is poi…

5dd538a

…ntless.

Replaced clean-cache.sh with clean-cache.py (more reliable path deter…

ca370d5

…mination) - Renamed manual-cache-timestamp.txt to manual-cache-timestamp

Cleaned up some python code, fixed the incorrect circleci log upload

92299f4

Added a README.md in .circleci to explain to users how force cache re…

f3ba736

…building

Fixed SimpleRegressionResults

20a906b

Revise code according to suggestions @bthirion in issue nilearn#173

13a0c75

Gilles86 added 4 commits December 7, 2018 16:08

Fix example from master branch

13b1b76

Fix example from master branch

e079bdc

Merge branch 'voxelwise_glm_stats' of github.com:Gilles86/nistats int…

bf4ad93

…o voxelwise_glm_stats

Merged latest master branch

416c63e

Gilles86 mentioned this pull request Dec 7, 2018

Voxelwise glm residuals/R2 (cleaned up pull request) #298

Closed

jdkent pushed a commit to jdkent/nistats that referenced this pull request Nov 20, 2019

Revise code according to suggestions @bthirion in issue nilearn#173

98ccf0d

jdkent pushed a commit to jdkent/nistats that referenced this pull request Nov 20, 2019

Revise code according to suggestions @bthirion in issue nilearn#173

5a39413

jdkent mentioned this pull request Nov 20, 2019

First level residuals #410

Merged

kchawla-pi closed this Dec 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Voxelwise glm residuals/R2 #173

Voxelwise glm residuals/R2 #173

Gilles86 commented Mar 16, 2018

bthirion commented Mar 18, 2018

KamalakerDadi commented Mar 19, 2018

Gilles86 commented Mar 19, 2018

Gilles86 commented Mar 19, 2018

bthirion May 12, 2018

bthirion May 12, 2018

bthirion May 12, 2018

bthirion May 12, 2018

jdkent commented Nov 5, 2018

Gilles86 commented Nov 6, 2018

Gilles86 commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018

Gilles86 commented Dec 7, 2018

Gilles86 commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018 •

edited

Loading

Gilles86 commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018

Voxelwise glm residuals/R2 #173

Voxelwise glm residuals/R2 #173

Conversation

Gilles86 commented Mar 16, 2018

bthirion commented Mar 18, 2018

KamalakerDadi commented Mar 19, 2018

Gilles86 commented Mar 19, 2018

Gilles86 commented Mar 19, 2018

bthirion May 12, 2018

Choose a reason for hiding this comment

bthirion May 12, 2018

Choose a reason for hiding this comment

bthirion May 12, 2018

Choose a reason for hiding this comment

bthirion May 12, 2018

Choose a reason for hiding this comment

jdkent commented Nov 5, 2018

Gilles86 commented Nov 6, 2018

Gilles86 commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018

Gilles86 commented Dec 7, 2018

Gilles86 commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018 • edited Loading

Gilles86 commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018

kchawla-pi commented Dec 7, 2018 •

edited

Loading