[DOC] Combine similar two-runs GLM examples #3191

tsalo · 2022-03-24T17:29:15Z

Closes #2771
Closes #4211

Changes proposed in this pull request:

Combine plot_fiac_analysis.py and plot_fixed_effects.py into plot_two_session_model.py.
Update a link in first_level_model.rst that originally went to plot_fiac_analysis.py.
Example now relies on the fact that Nilearn will zero pad contrast matrices (uncovered a bug that got fixed)
Update some of the code to use run instead of session
Fix plotting of unpadded contrast matrices and add test for it

github-actions · 2022-03-24T17:40:27Z

👋 @tsalo 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"
Code is PEP8-compliant.
(Bug fixes) There is at least one test that would fail under the original bug conditions.
(New features) There is at least one unit test per new function / class.
(New features) The new feature is demoed in at least one relevant example.

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

codecov · 2022-03-24T17:49:28Z

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (6764c9e) 92.09% compared to head (256bfff) 92.00%.
Report is 8 commits behind head on main.

Files	Patch %	Lines
nilearn/datasets/func.py	71.42%	2 Missing ⚠️
nilearn/glm/_utils.py	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3191      +/-   ##
==========================================
- Coverage   92.09%   92.00%   -0.10%     
==========================================
  Files         144      144              
  Lines       16366    16389      +23     
  Branches     3426     3431       +5     
==========================================
+ Hits        15073    15078       +5     
- Misses        754      767      +13     
- Partials      539      544       +5

Flag	Coverage Δ
macos-latest_3.10_test_plotting	`91.87% <89.65%> (-0.02%)`	⬇️
macos-latest_3.11_test_plotting	`91.87% <89.65%> (-0.02%)`	⬇️
macos-latest_3.12_test_plotting	`91.87% <89.65%> (-0.02%)`	⬇️
macos-latest_3.8_test_plotting	`91.83% <89.65%> (-0.02%)`	⬇️
macos-latest_3.9_test_plotting	`91.83% <89.65%> (-0.02%)`	⬇️
ubuntu-latest_3.10_test_plotting	`91.87% <89.65%> (-0.02%)`	⬇️
ubuntu-latest_3.11_test_plotting	`91.87% <89.65%> (-0.02%)`	⬇️
ubuntu-latest_3.12_test_plotting	`91.87% <89.65%> (-0.02%)`	⬇️
ubuntu-latest_3.12_test_pre	`91.87% <89.65%> (-0.02%)`	⬇️
ubuntu-latest_3.8_test_min	`68.91% <27.58%> (-0.08%)`	⬇️
ubuntu-latest_3.8_test_plot_min	`?`
ubuntu-latest_3.8_test_plotting	`91.83% <89.65%> (-0.02%)`	⬇️
ubuntu-latest_3.9_test_plotting	`91.83% <89.65%> (-0.02%)`	⬇️
windows-latest_3.10_test_plotting	`91.84% <89.65%> (-0.02%)`	⬇️
windows-latest_3.11_test_plotting	`91.84% <89.65%> (-0.02%)`	⬇️
windows-latest_3.12_test_plotting	`91.84% <89.65%> (-0.02%)`	⬇️
windows-latest_3.8_test_plotting	`91.80% <89.65%> (-0.02%)`	⬇️
windows-latest_3.9_test_plotting	`91.81% <89.65%> (-0.02%)`	⬇️

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.

examples/04_glm_first_level/plot_two_session_model.py

bthirion

Thx !
I think we can still improve a little bit.

examples/04_glm_first_level/plot_two_session_model.py

Remi-Gau · 2023-02-24T12:04:20Z

@tsalo do you want me to take over or do you prefer to wrap it up?

tsalo · 2023-02-24T15:19:58Z

@Remi-Gau, I think this is waiting on #3203, but I could be misremembering.

Remi-Gau · 2023-02-25T13:40:43Z

@Remi-Gau, I think this is waiting on #3203, but I could be misremembering.

My bad: I misread a comment in the thread.

bthirion · 2023-02-25T16:12:30Z

#3203 needs a bit of love, but I'm willing to finish it. Thx for the remainder !

Remi-Gau · 2023-09-28T09:02:43Z

#3203 has been merged so this should unblock this PR.

@tsalo let me know if you have the bandwidth to get back to it.

Remi-Gau · 2024-01-10T18:18:39Z

@tsalo am taking over this PR before it drifts too far

tsalo · 2024-01-10T18:20:12Z

@Remi-Gau thanks for taking over. Sorry I didn't get around to it back in September.

…ng function with tests

Remi-Gau · 2024-01-10T20:31:12Z

No worries at all.

Remi-Gau · 2024-01-10T20:33:14Z

To probably tackle in a separate PR but it seems that the contrast matric plotting function gives weird results when it is given unpadded contrast matrices.

bthirion · 2024-01-10T22:35:56Z

LGTM, but the docu build seems broken.

examples/04_glm_first_level/plot_two_runs_model.py

Remi-Gau · 2024-01-11T09:53:53Z

TODO:

update changelog

tsalo · 2024-01-11T15:28:05Z

@tsalo do you remember what specifically was still needed?

@Remi-Gau I thought the only issue was that compute_fixed_effects wasn't producing correct results for F contrasts, so the two figures being compared didn't match. I don't think there were any actual bugs in the example back when we paused it, so it could be that some things are out of date now.

bthirion

LGTM otherwise, thx !

examples/04_glm_first_level/plot_two_runs_model.py

Remi-Gau · 2024-01-16T11:29:57Z

nilearn/plotting/matrix_plotting.py

@@ -409,6 +403,54 @@ def plot_contrast_matrix(
    return ax


+def pad_contrast_matrix(contrast_def, design_matrix):


Added to fix #4211 in this PR to have valid glm reports in this example

Can extract in separate PR if needed.

Remi-Gau · 2024-01-16T11:30:49Z

nilearn/plotting/matrix_plotting.py

+    contrast_def : :obj:`str` or :class:`numpy.ndarray` of shape[1] <= n_col \
+        where ``n_col`` is the number of columns of the design matrix.
+        The string can be a formula compatible
+        with :meth:`pandas.DataFrame.eval`.
+        Basically one can use the name of the conditions
+        as they appear in the design matrix of the fitted model
+        combined with operators +-
+        and combined with numbers with operators +-`*`/.


as far as I can tell the code in this function cannot handle list of strings or list of arrays as inputs

Remi-Gau · 2024-01-16T11:52:00Z

Adding a report of the outputs before and after: could not spot any difference.

reports.zip

ymzayek

@Remi-Gau thanks it's looking good! Some minor comments

ymzayek · 2024-01-16T14:44:05Z

doc/references.bib

@@ -180,6 +180,17 @@ @article{Craddock2012
 	year = {2012}
 }

+@article{dehaene2006functional,


since we alphabetized this list this should move to after Davidson2004

good catch

thanks!!!

examples/04_glm_first_level/plot_two_runs_model.py

Co-authored-by: Yasmin <63292494+ymzayek@users.noreply.github.com>

examples/04_glm_first_level/plot_two_runs_model.py

bthirion

Many improvements in one PR, thx !

tsalo added 2 commits March 24, 2022 13:27

Combine plot_fiac_analysis and plot_fixed_effects.

e618fdd

Update link to example.

1a603a9

tsalo added the Documentation for documentation related questions or requests label Mar 24, 2022

tsalo commented Mar 24, 2022

View reviewed changes

examples/04_glm_first_level/plot_two_session_model.py Outdated Show resolved Hide resolved

bthirion mentioned this pull request Mar 25, 2022

compute_fixed_effects currently does not support F statistics (multi-dimensional) #3194

Closed

Switch example to a t contrast.

4576c6e

bthirion reviewed Mar 27, 2022

View reviewed changes

examples/04_glm_first_level/plot_two_session_model.py Outdated Show resolved Hide resolved

examples/04_glm_first_level/plot_two_session_model.py Outdated Show resolved Hide resolved

bthirion mentioned this pull request Apr 1, 2022

Examples using FIAC dataset should have a better design matrix #3208

Closed

Remi-Gau added the GLM Issues/PRs related to the nilearn.glm module. label Feb 20, 2023

Remi-Gau added the Blocked label Jun 13, 2023

Remi-Gau removed the Blocked label Sep 28, 2023

Remi-Gau added 3 commits January 10, 2024 18:12

Merge remote-tracking branch 'upstream/main' into combine-glm-examples

7851e11

lint

06f5f94

use cell separators

2663ff0

Remi-Gau added 2 commits January 10, 2024 20:04

remove padding helper function and fix bugs in nilearn internal paddi…

550aef8

…ng function with tests

enforce semantic line break

9cb0a94

Remi-Gau mentioned this pull request Jan 10, 2024

Replace session with run where appropriate to align with the BIDS usage of the words #4206

Closed

Remi-Gau changed the title ~~[DOC] Combine similar two-session GLM examples~~ [DOC] Combine similar two-runs GLM examples Jan 10, 2024

Remi-Gau reviewed Jan 11, 2024

View reviewed changes

examples/04_glm_first_level/plot_two_runs_model.py Outdated Show resolved Hide resolved

print output dir

5a353c5

Remi-Gau mentioned this pull request Jan 11, 2024

[BUG] contrast matrices figures are not displayed properly when handling unpadded contrasts #4211

Closed

9 tasks

__file__ cannot be used in examples

836cc22

Remi-Gau mentioned this pull request Jan 11, 2024

[FIX] fix plotting of an image with color bar when maximum value is exactly zero #4204

Merged

update changelog

fc3f558

style fix

cf128da

bthirion reviewed Jan 15, 2024

View reviewed changes

examples/04_glm_first_level/plot_two_runs_model.py Outdated Show resolved Hide resolved

examples/04_glm_first_level/plot_two_runs_model.py Outdated Show resolved Hide resolved

Remi-Gau added 3 commits January 16, 2024 11:24

fix contrast padding

0d5238a

add test for contrast padding and update doc string

1b7030d

refactor

bfb005d

Remi-Gau reviewed Jan 16, 2024

View reviewed changes

add title

310de33

Remi-Gau added 2 commits January 16, 2024 12:54

fix test

eb0e462

typo

ae8e0d9

Remi-Gau requested review from bthirion and ymzayek January 16, 2024 12:27

ymzayek reviewed Jan 16, 2024

View reviewed changes

Remi-Gau reviewed Jan 16, 2024

View reviewed changes

examples/04_glm_first_level/plot_two_runs_model.py Outdated Show resolved Hide resolved

Apply suggestions from code review

ae2d58d

Co-authored-by: Yasmin <63292494+ymzayek@users.noreply.github.com>

Remi-Gau reviewed Jan 16, 2024

View reviewed changes

examples/04_glm_first_level/plot_two_runs_model.py Outdated Show resolved Hide resolved

Remi-Gau added 2 commits January 16, 2024 17:37

Apply suggestions from code review

0058d7f

reorganize ref

256bfff

bthirion approved these changes Jan 16, 2024

View reviewed changes

Remi-Gau merged commit 881816d into nilearn:main Jan 17, 2024
31 of 32 checks passed

Remi-Gau mentioned this pull request Jan 31, 2024

[DOC] update maintenance documentation for the releases #4250

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DOC] Combine similar two-runs GLM examples #3191

[DOC] Combine similar two-runs GLM examples #3191

tsalo commented Mar 24, 2022 •

edited by Remi-Gau

github-actions bot commented Mar 24, 2022 •

edited by tsalo

codecov bot commented Mar 24, 2022 •

edited

bthirion left a comment

Remi-Gau commented Feb 24, 2023

tsalo commented Feb 24, 2023

Remi-Gau commented Feb 25, 2023

bthirion commented Feb 25, 2023

Remi-Gau commented Sep 28, 2023

Remi-Gau commented Jan 10, 2024

tsalo commented Jan 10, 2024

Remi-Gau commented Jan 10, 2024

Remi-Gau commented Jan 10, 2024

bthirion commented Jan 10, 2024

Remi-Gau commented Jan 11, 2024 •

edited

tsalo commented Jan 11, 2024

bthirion left a comment

Remi-Gau Jan 16, 2024

Remi-Gau Jan 16, 2024

Remi-Gau Jan 16, 2024

Remi-Gau commented Jan 16, 2024

ymzayek left a comment

ymzayek Jan 16, 2024

Remi-Gau Jan 16, 2024

bthirion left a comment

		@@ -409,6 +403,54 @@ def plot_contrast_matrix(
		return ax


		def pad_contrast_matrix(contrast_def, design_matrix):

[DOC] Combine similar two-runs GLM examples #3191

[DOC] Combine similar two-runs GLM examples #3191

Conversation

tsalo commented Mar 24, 2022 • edited by Remi-Gau

github-actions bot commented Mar 24, 2022 • edited by tsalo

codecov bot commented Mar 24, 2022 • edited

Codecov Report

bthirion left a comment

Choose a reason for hiding this comment

Remi-Gau commented Feb 24, 2023

tsalo commented Feb 24, 2023

Remi-Gau commented Feb 25, 2023

bthirion commented Feb 25, 2023

Remi-Gau commented Sep 28, 2023

Remi-Gau commented Jan 10, 2024

tsalo commented Jan 10, 2024

Remi-Gau commented Jan 10, 2024

Remi-Gau commented Jan 10, 2024

bthirion commented Jan 10, 2024

Remi-Gau commented Jan 11, 2024 • edited

tsalo commented Jan 11, 2024

bthirion left a comment

Choose a reason for hiding this comment

Remi-Gau Jan 16, 2024

Choose a reason for hiding this comment

Remi-Gau Jan 16, 2024

Choose a reason for hiding this comment

Remi-Gau Jan 16, 2024

Choose a reason for hiding this comment

Remi-Gau commented Jan 16, 2024

ymzayek left a comment

Choose a reason for hiding this comment

ymzayek Jan 16, 2024

Choose a reason for hiding this comment

Remi-Gau Jan 16, 2024

Choose a reason for hiding this comment

bthirion left a comment

Choose a reason for hiding this comment

tsalo commented Mar 24, 2022 •

edited by Remi-Gau

github-actions bot commented Mar 24, 2022 •

edited by tsalo

codecov bot commented Mar 24, 2022 •

edited

Remi-Gau commented Jan 11, 2024 •

edited