[ENH] Allow Decoder to work with surface objects #4205

ymzayek · 2024-01-10T15:49:41Z

Part of [META] Next steps for development on experimental surface module #4027

Changes proposed in this pull request:

Add tests for decoder methods with surface object
Adapt screening percentile function
Adapt ReNA clustering

TODO

Examples to adapt:

examples/02_decoding/plot_haxby_searchlight_surface.py

github-actions · 2024-01-10T15:49:56Z

👋 @ymzayek Thanks for creating a PR!

Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft.

Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.

PR has an interpretable title.
PR links to Github issue with mention Closes #XXXX (see our documentation on PR structure)
Code is PEP8-compliant (see our documentation on coding style)
Changelog or what's new entry in doc/changes/latest.rst (see our documentation on PR structure)

For new features:

There is at least one unit test per new function / class (see our documentation on testing)
The new feature is demoed in at least one relevant example.

For bug fixes:

There is at least one test that would fail under the original bug conditions.

We will review it as quick as possible, feel free to ping us with questions if needed.

codecov · 2024-01-10T16:25:37Z

Codecov Report

Attention: Patch coverage is 98.21429% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 92.01%. Comparing base (abb80ff) to head (7f44d33).
Report is 78 commits behind head on main.

Files	Patch %	Lines
nilearn/regions/rena_clustering.py	97.87%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4205      +/-   ##
==========================================
+ Coverage   91.85%   92.01%   +0.16%     
==========================================
  Files         144      145       +1     
  Lines       16419    16724     +305     
  Branches     3434     3548     +114     
==========================================
+ Hits        15082    15389     +307     
+ Misses        792      766      -26     
- Partials      545      569      +24

Flag	Coverage Δ
macos-latest_3.11_test_plotting	`?`
ubuntu-latest_3.10_test_plotting	`91.80% <98.21%> (-0.06%)`	⬇️
ubuntu-latest_3.11_test_plotting	`91.80% <98.21%> (?)`
ubuntu-latest_3.12_test_plotting	`91.80% <98.21%> (?)`
ubuntu-latest_3.8_test_min	`68.81% <98.21%> (?)`
ubuntu-latest_3.8_test_plot_min	`91.47% <98.21%> (?)`
ubuntu-latest_3.8_test_plotting	`91.76% <98.21%> (?)`
windows-latest_3.11_test_plotting	`91.79% <98.21%> (?)`
windows-latest_3.12_test_plotting	`91.79% <98.21%> (?)`
windows-latest_3.8_test_plotting	`91.75% <98.21%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ymzayek · 2024-01-15T14:56:11Z

@bthirion WDYT about handling FREM in the decoder? Is it applicable/used for surface analysis? This would require adapting ReNA. If not priority, I would leave for later

bthirion · 2024-01-15T15:02:32Z

Adapting ReNA (and Ward) to the surface is technically easy. We should definitely do that.

ymzayek · 2024-01-15T16:19:04Z

Adapting ReNA (and Ward) to the surface is technically easy. We should definitely do that.

Well yes and no. I think writing a new image check function that wraps check_niimg and handles surface image separately (we don't want to call load_niimg) might make this and a lot of other cases easier.

bthirion · 2024-01-15T16:36:35Z

Ye, but behind that, the algorithm will be a bit different, given that the connectivity structure is then defined by the mesh and no longer by the voxel grid.

ymzayek · 2024-01-17T11:26:02Z

What would be the equivalent calculation of edges and weights for surface data? _make_edges_and_weights is a blocker for FREM using ReNA and it depends on several functions that need volumetric data so maybe it would be preferable to write a _make_edges_and_weights_surface function that is called from _weighted_connectivity_graph when the input is a surface object

bthirion · 2024-01-17T21:34:50Z

edges are the mesh edges: each triangle results into three edges.
Given some data X weights are then simply computed as $|| X_i - X_j ||^2$ for edge (i, j).

ymzayek · 2024-01-18T13:27:00Z

The comment from #4227 (review) is meant to be here

We probably also have to adapt Decoder heuristics with the percentile selection ? What about clustering ?

ymzayek · 2024-01-19T16:37:27Z

nilearn/masking.py

-    mask : :class:`numpy.ndarray`, shape (nx, ny, nz)
+    mask : :class:`numpy.ndarray`


there is nothing in this function that requires that shape

ymzayek · 2024-01-19T16:39:58Z

nilearn/regions/rena_clustering.py

+    else:
+        edges, weight = _make_edges_and_weights(X, mask_img)
+
+    # TODO : deal with dict results if surface analysis


Right now failure is here because edges and weights are returned in a dictionary with keys representing the surface image object mesh and data keys for the different hemispheres

@bthirion I'm a bit stuck here. Does it make more sense to let ReNA run on each hemisphere separately and then concatenate the results in X_train on the level of the call from the decoder?

We could great a unique graph that would stack the data from the two hemispheres. This graph would have 2 connected components. This would have the advantage to make the use on surface transparent for other functions.

ymzayek · 2024-01-19T16:44:15Z

TODO:

Finish rena clustering adaptation
validate screening percentile and clustering for surface

bthirion

Thx ! Thi is great

nilearn/decoding/space_net.py

bthirion · 2024-01-19T22:12:33Z

nilearn/regions/rena_clustering.py

+    faces : ndarray
+        The vertex indices corresponding the mesh triangles.
+
+    mask : boolean


I don't see how to make sense of mask in this function, given that the edge ordering is unclear at that point ?

maks is on vertices, right ? Then it makes sense. But the dosctring has to be improved.

ymzayek · 2024-01-22T18:47:00Z

nilearn/regions/rena_clustering.py

@@ -171,11 +291,20 @@ def _weighted_connectivity_graph(X, mask_img):
    """
    n_features = X.shape[1]

-    edges, weight = _make_edges_and_weights(X, mask_img)
+    if isinstance(mask_img, SurfaceImage):


I did not finish this section under the conditional for surface. It probably needs to be updated according to this comment #4205 (comment)

And then the whole pipeline needs to be validated

Remi-Gau · 2024-06-03T15:47:48Z

Examples to adapt:

* [ ]  examples/02_decoding/plot_haxby_searchlight_surface.py

Will be easier to adapt this example once: [ENH] expand surface plotting with new surface object #4431 has been merged, probably in a follow up PR.

bthirion

There remain a few comments to address, but it looks good overall.

bthirion · 2024-06-03T20:57:08Z

nilearn/decoding/tests/test_decoder.py

+    assert model.scoring == "roc_auc"
+
+    model.score(X, y)
+    accuracy_score(y, y_pred)


Maybe we could additionally check that the accuracy score are reasonable (close to chance I guess) ?

bthirion · 2024-06-03T20:57:14Z

nilearn/decoding/tests/test_decoder.py

+    assert model.scoring == "r2"
+
+    model.score(X, y)
+    r2_score(y, y_pred)


Maybe we could additionally check that the accuracy score are reasonable (close to chance I guess) ?

ymzayek mentioned this pull request Jan 17, 2024

[WIP][ENH] Move surface image API from experimental to stable API #4229

Closed

9 tasks

ymzayek added the Blocked label Jan 17, 2024

ymzayek mentioned this pull request Jan 18, 2024

[FIX] Let decoder accept list of paths as input #4227

Merged

ymzayek added 12 commits January 19, 2024 15:35

Add more decoder surface tests

9aea331

Handle screening percentile

5712580

Add test

fdf2d34

Allow screening percentile for surface object

3a7abfa

Remove monkey patch

5ac751d

space net NotimplementedError

394dc0e

Add tests

b1f6703

Add test for screening percentile

cca9dc8

Split tests

c0cb5e8

Start adapting frem and rena

81bc380

Function structure for surface edges/weights

aabc3c4

Update partial code

00980ea

ymzayek force-pushed the decoder_surface branch from 7bf7590 to 00980ea Compare January 19, 2024 14:37

ymzayek added 2 commits January 19, 2024 16:53

Fix _make_edges_and_weights_surface

2203d9a

Update var names

39c37c9

ymzayek commented Jan 19, 2024

View reviewed changes

bthirion reviewed Jan 19, 2024

View reviewed changes

ymzayek added 2 commits January 22, 2024 12:07

Fix error message

d57d0b3

Add conn for each part

9906a76

ymzayek commented Jan 22, 2024

View reviewed changes

Remi-Gau added the Surface Related to surface data or surface analysis. label Jan 23, 2024

Remi-Gau added this to the release 0.11.0 milestone Jan 24, 2024

Remi-Gau mentioned this pull request Jan 24, 2024

[META] Next steps for development on experimental surface module #4027

Open

42 tasks

Remi-Gau modified the milestones: release 0.10.3, Release 0.11.0 Jan 24, 2024

Remi-Gau added 2 commits May 31, 2024 16:57

Merge branch 'main' into decoder_surface

6727e52

Merge branch 'main' into decoder_surface

7bc04eb

Remi-Gau removed the Blocked label Jun 3, 2024

fix tests

7f44d33

bthirion reviewed Jun 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Allow Decoder to work with surface objects #4205

[ENH] Allow Decoder to work with surface objects #4205

ymzayek commented Jan 10, 2024 •

edited by Remi-Gau

github-actions bot commented Jan 10, 2024

codecov bot commented Jan 10, 2024 •

edited

ymzayek commented Jan 15, 2024

bthirion commented Jan 15, 2024

ymzayek commented Jan 15, 2024

bthirion commented Jan 15, 2024

ymzayek commented Jan 17, 2024

bthirion commented Jan 17, 2024 •

edited by Remi-Gau

ymzayek commented Jan 18, 2024

ymzayek Jan 19, 2024

ymzayek Jan 19, 2024

ymzayek Jan 22, 2024

bthirion Jan 22, 2024

ymzayek commented Jan 19, 2024

bthirion left a comment

bthirion Jan 19, 2024

bthirion Jan 19, 2024

ymzayek Jan 22, 2024

Remi-Gau commented Jun 3, 2024

bthirion left a comment

bthirion Jun 3, 2024

bthirion Jun 3, 2024

		mask : :class:`numpy.ndarray`, shape (nx, ny, nz)
		mask : :class:`numpy.ndarray`

[ENH] Allow Decoder to work with surface objects #4205

Are you sure you want to change the base?

[ENH] Allow Decoder to work with surface objects #4205

Conversation

ymzayek commented Jan 10, 2024 • edited by Remi-Gau

TODO

github-actions bot commented Jan 10, 2024

codecov bot commented Jan 10, 2024 • edited

Codecov Report

ymzayek commented Jan 15, 2024

bthirion commented Jan 15, 2024

ymzayek commented Jan 15, 2024

bthirion commented Jan 15, 2024

ymzayek commented Jan 17, 2024

bthirion commented Jan 17, 2024 • edited by Remi-Gau

ymzayek commented Jan 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ymzayek commented Jan 19, 2024

bthirion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Remi-Gau commented Jun 3, 2024

bthirion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ymzayek commented Jan 10, 2024 •

edited by Remi-Gau

codecov bot commented Jan 10, 2024 •

edited

bthirion commented Jan 17, 2024 •

edited by Remi-Gau