[WIP] design matrices may have different columns

[codecov[bot]]

Codecov Report

Merging #416 into master will increase coverage by 0.2%. The diff coverage is 100%.

@@            Coverage Diff            @@
##           master     #416     +/-   ##
=========================================
+ Coverage   92.52%   92.72%   +0.2%     
=========================================
  Files          39       40      +1     
  Lines        4349     4620    +271     
=========================================
+ Hits         4024     4284    +260     
- Misses        325      336     +11

Impacted Files	Coverage Δ
nistats/first_level_model.py	83.12% <ø> (+0.18%)	:arrow_up:
nistats/tests/test_first_level_model.py	100% <100%> (ø)	:arrow_up:
nistats/_utils/testing.py	100% <100%> (ø)	:arrow_up:
nistats/thresholding.py	96.11% <0%> (-3.89%)	:arrow_down:
nistats/tests/test_init.py	93.1% <0%> (-1.35%)	:arrow_down:
nistats/reporting/glm_reporter.py	99.08% <0%> (-0.46%)	:arrow_down:
nistats/tests/test_reporting.py	96.82% <0%> (-0.05%)	:arrow_down:
nistats/tests/test_glm_reporter.py	97.93% <0%> (ø)	:arrow_up:
nistats/tests/test_datasets.py	100% <0%> (ø)	:arrow_up:
nistats/tests/test_regression.py	100% <0%> (ø)	:arrow_up:
... and 13 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d86362f...7afbef4. Read the comment docs.

[bthirion]

I've added a test. @kchawla-pi need your feedback.

[kchawla-pi]

Also, does having design matrices and contrasts with different names affect the results of make_glm_report in an unforeseen manner?

[jeromedockes]

Also, does having design matrices and contrasts with different names affect the results of make_glm_report in an unforeseen manner?

yes see https://github.com/nistats/nistats/issues/350#issuecomment-567007349

[kchawla-pi]

@bthirion This seems good. You ready to merge it? Also, in light of

yes see #350 (comment)

Do you have suggestions about how you want to modify the GLM Reporter?

[kchawla-pi]

~If~ When we merge this, I want us to have a clear idea if and how the GLM Reporter should be modified so I can start on that immediately.

[kchawla-pi]

Huh. fetch_localizer_contrasts() has a permanent redirect, overnight??? Do we change the URL in the fetcher?

[kchawla-pi]

This is happenning in Nilearn's fetch_developmental_dataset() as well. Probably an osf.io server side error then. Let's wait and see if they fix it. I'll email them as well.

[bthirion]

On 16/01/2020 11:16, Kshitij Chawla wrote:

If we merge this, I want us to have a clear idea if and how the GLM Reporter should be modified so I can start on that immediately.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nistats/nistats/pull/416?email_source=notifications&email_token=AABZHVT52RAJLCFA6X6OLFTQ6AXXXA5CNFSM4J4SKO32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJDQ7OY#issuecomment-575082427, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABZHVU7QDAR2HEWJBHCRELQ6AXXXANCNFSM4J4SKO3Q.

Regarding the GLM reporter, I think that the set of provided contrasts should be a list of lists:

the main list corresponds to contrast, the inner list corresponds to sessions.

Do we agree on that ?

[kchawla-pi]

You mean,

contrasts = [1, 0, 0] is contrasts = [[1, 0, 0]]  # applied to each session.
contrasts = [[1, 0, 0], [1, 1, 0]]  # applied to 2 sessions, 1 each.
contrasts = [[1, 0, 0], [1, 1, 0]]  # error if number of sessions != 2.  
contrasts = 'a -b' is contrasts = [['a-b']]  # applied to each sesssion.

...and so on?

[bthirion]

Yes.

[kchawla-pi]

OKay, Do you have anything to add to this PR? If not, I'll merge this and then get to work on GLM Reporter.

[bthirion]

No, you can proceed.

[jeromedockes]

Regarding the GLM reporter, I think that the set of provided contrasts should be a list of lists:

the main list corresponds to contrast, the inner list corresponds to sessions.

Do we agree on that ?

I agree. Note that it can now go up to three dimensions: (contrast x session x column) e.g.

[
[[1, 0, 0], [0, 1, 0]] # first contrast, two sessions
[[-1, 1, 0], [1, -1, 0]] # second contrast, two sessions.
]

we may need to add some constraints to avoid ambiguities, e.g. remove the possibility of passing a single contrast rather than a list of contrasts -- otherwise we don't always know which dimension has been squeezed if we get less than 3 dimensions.

Also, a similar change should be made when the user provides a dictionary with contrast names as keys, ie values should become 2-dimensional (session x column)

[kchawla-pi]

I agree. Note that it can now go up to three dimensions

We should have a couple examples detailing multi-dimensional contrasts. How is it that none of our examples have that? (or do we?)

[bthirion]

the FIAC examples has that. In general all 'effects_of_interest' contrasts are mutli-dimensional.