nilearn / nistats

Modeling and statistical inference on fMRI data in Python
BSD 3-Clause "New" or "Revised" License
95 stars 55 forks source link

[FIX] Design matrices are wrong if multiple events in the same regressor have the same onset or offset #442

Open mwegrzyn opened 4 years ago

mwegrzyn commented 4 years ago

Dear nistats team,

using first_level_model.make_first_level_design_matrix() I noticed that in some cases the baseline of a regressor can shift and never return to zero, instead being stuck at another value (e.g. -1, +1 etc.). I think the issue is that in the function hemodynamic_models._sample_condition() each onset (e.g. a stick function of +1) has to be balanced by an offset of the same, but negative value (e.g. -1). However, if multiple events start at the same time (+1) but end at different times (-1 + -1 = -2), the baseline will permanently shift downwards. The opposite effect occurs when different onsets have the same offset (then the baseline will be permanently shifted in the positive direction). For some examples, including a possible solution, please see the following notebook:

https://gist.github.com/mwegrzyn/80a382f30260f5982e8b5dc5c89867fb

I hope that the suggested pull request is helpful. It's my first one ever, so please let me know if something is not quite ok. Thank you very much!

Best,
Martin

codecov[bot] commented 4 years ago

Codecov Report

Merging #442 into master will increase coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #442      +/-   ##
==========================================
+ Coverage   92.71%   92.71%   +<.01%     
==========================================
  Files          40       40              
  Lines        4707     4709       +2     
==========================================
+ Hits         4364     4366       +2     
  Misses        343      343
Impacted Files Coverage Δ
nistats/hemodynamic_models.py 98.11% <100%> (+0.03%) :arrow_up:

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 7b17e51...7f600e7. Read the comment docs.

bthirion commented 4 years ago

The diagnosis is right, thx.

bthirion commented 4 years ago

Thx for the fix. An important thing to add is a test that breaks with the previous code and passes with the fix. That would be awesome. Thx a lot.

kchawla-pi commented 4 years ago

Hi @mwegrzyn ! thanks for this! We have incorporated all of Nilstats functionality into Nilearn's master and if you are still interested, i would urge you to reopen this PR there.

We are archiving this repo, so it is read-only.

We deeply appreciate your contribution, and I'm hoping to see you on Nilearn!