[ENH]: Add scrubbing support in fMRIPrepConfoundRemover #421

Merged
synchon merged 14 commits from feat/fmriprepconfoundremover-dvars-scrub into main 2025-02-13 17:05:38 +00:00
synchon commented 2025-01-23 18:27:18 +00:00 (Migrated from github.com)
  • description of feature/fix
  • tests added/passed
  • add an entry for the latest changes

This PR adds scrubbing support in fMRIPrepConfoundRemover by using std_dvars from fMRIPrep output.

* [x] description of feature/fix * [x] tests added/passed * [x] add an entry for the latest changes This PR adds scrubbing support in `fMRIPrepConfoundRemover` by using `std_dvars` from fMRIPrep output.
codecov[bot] commented 2025-01-23 18:34:38 +00:00 (Migrated from github.com)

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.73%. Comparing base (02e0b6f) to head (e5ada55).
Report is 15 commits behind head on main.

❌ Your project status has failed because the head coverage (85.72%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #421      +/-   ##
==========================================
+ Coverage   85.67%   85.73%   +0.05%     
==========================================
  Files         133      133              
  Lines        5633     5656      +23     
  Branches      954      958       +4     
==========================================
+ Hits         4826     4849      +23     
  Misses        618      618              
  Partials      189      189              
Flag Coverage Δ
junifer 85.72% <100.00%> (+0.05%) ⬆️

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

Files with missing lines Coverage Δ
.../preprocess/confounds/fmriprep_confound_remover.py 99.46% <100.00%> (+0.07%) ⬆️
## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/421?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 85.73%. Comparing base [(`02e0b6f`)](https://app.codecov.io/gh/juaml/junifer/commit/02e0b6fe0051e15f47c7b39820834a6793762057?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) to head [(`e5ada55`)](https://app.codecov.io/gh/juaml/junifer/commit/e5ada553ac8db5a07299720a80c8cc37baad096d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml). > Report is 15 commits behind head on main. :x: Your project status has failed because the head coverage (85.72%) is below the target coverage (90.00%). You can increase the head coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage. <details><summary>Additional details and impacted files</summary> [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/421/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://app.codecov.io/gh/juaml/junifer/pull/421?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #421 +/- ## ========================================== + Coverage 85.67% 85.73% +0.05% ========================================== Files 133 133 Lines 5633 5656 +23 Branches 954 958 +4 ========================================== + Hits 4826 4849 +23 Misses 618 618 Partials 189 189 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/421/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [junifer](https://app.codecov.io/gh/juaml/junifer/pull/421/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `85.72% <100.00%> (+0.05%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/juaml/junifer/pull/421?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [.../preprocess/confounds/fmriprep\_confound\_remover.py](https://app.codecov.io/gh/juaml/junifer/pull/421?src=pr&el=tree&filepath=junifer%2Fpreprocess%2Fconfounds%2Ffmriprep_confound_remover.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#diff-anVuaWZlci9wcmVwcm9jZXNzL2NvbmZvdW5kcy9mbXJpcHJlcF9jb25mb3VuZF9yZW1vdmVyLnB5) | `99.46% <100.00%> (+0.07%)` | :arrow_up: | </details>
fraimondo (Migrated from github.com) requested changes 2025-01-23 21:08:57 +00:00
fraimondo (Migrated from github.com) commented 2025-01-23 21:08:49 +00:00

std_dvars should not be added as a regressor to perform scrubbing, but used to generate a mask that is then passed on to signal.clean

std_dvars should not be added as a regressor to perform scrubbing, but used to generate a mask that is then passed on to `signal.clean`
synchon (Migrated from github.com) reviewed 2025-01-24 10:13:29 +00:00
synchon (Migrated from github.com) commented 2025-01-24 10:13:28 +00:00

So the mask would be temporal which should remove the timepoints from both BOLD image and confounds table and then pass the result to nilearn.image.clean_img()?

So the mask would be temporal which should remove the timepoints from both BOLD image and confounds table and then pass the result to `nilearn.image.clean_img()`?
github-actions[bot] commented 2025-01-24 17:12:00 +00:00 (Migrated from github.com)
PR Preview Action v1.6.0

🚀 View preview at
https://juaml.github.io/junifer/pr-preview/pr-421/

Built to branch gh-pages at 2025-01-29 11:06 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.6.0 :---: | <p></p> :rocket: View preview at <br> https://juaml.github.io/junifer/pr-preview/pr-421/ <br><br> | <h6>Built to branch [`gh-pages`](https://github.com/juaml/junifer/tree/gh-pages) at 2025-01-29 11:06 UTC. <br> Preview will be ready when the [GitHub Pages deployment](https://github.com/juaml/junifer/deployments) is complete. <br><br> </h6> <!-- Sticky Pull Request Commentpr-preview -->
fraimondo commented 2025-01-27 07:45:17 +00:00 (Migrated from github.com)

Two comments:

  1. Why using DVARS? where did you get the reference for this method? I've seen scrubbing also using the FD
  2. As it is, it nows also adds DVARS as a regressor.

I think the implementation should come from nilearn: https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#nilearn.interfaces.fmriprep.load_confounds

Basically here you can set the fd_threshold and std_dvars_threshold and it will give you the sample_mask

Two comments: 1) Why using DVARS? where did you get the reference for this method? I've seen scrubbing also using the FD 2) As it is, it nows also adds DVARS as a regressor. I think the implementation should come from nilearn: https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#nilearn.interfaces.fmriprep.load_confounds Basically here you can set the `fd_threshold` and `std_dvars_threshold` and it will give you the `sample_mask`
synchon commented 2025-01-27 10:33:53 +00:00 (Migrated from github.com)

Two comments:

  1. Why using DVARS? where did you get the reference for this method? I've seen scrubbing also using the FD

nilearn.image.load_confounds has a parameter std_dvars_threshold for scrubbing, so I went for it as well. We support FD via the spike parameter of fMRIPrepConfoundRemover.

  1. As it is, it nows also adds DVARS as a regressor.

It generates a mask for indexing the time dimension and passes it to nilearn.image.clean as you suggested in your previous comment.

I think the implementation should come from nilearn: https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#nilearn.interfaces.fmriprep.load_confounds

Basically here you can set the fd_threshold and std_dvars_threshold and it will give you the sample_mask

We don't use load_confounds but select the confounds we want to remove and pass it to nilearn.image.clean_img. Also, load_confounds relies on the confound file by following the path from the image file. Is that something we go for?

> Two comments: > > 1. Why using DVARS? where did you get the reference for this method? I've seen scrubbing also using the FD `nilearn.image.load_confounds` has a parameter `std_dvars_threshold` for scrubbing, so I went for it as well. We support FD via the `spike` parameter of `fMRIPrepConfoundRemover`. > 2. As it is, it nows also adds DVARS as a regressor. It generates a mask for indexing the time dimension and passes it to `nilearn.image.clean` as you suggested in your previous comment. > I think the implementation should come from nilearn: https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#nilearn.interfaces.fmriprep.load_confounds > > Basically here you can set the `fd_threshold` and `std_dvars_threshold` and it will give you the `sample_mask` We don't use `load_confounds` but select the confounds we want to remove and pass it to `nilearn.image.clean_img`. Also, `load_confounds` relies on the confound file by following the path from the image file. Is that something we go for?
fraimondo commented 2025-01-28 07:56:46 +00:00 (Migrated from github.com)

Two comments:

  1. Why using DVARS? where did you get the reference for this method? I've seen scrubbing also using the FD

nilearn.image.load_confounds has a parameter std_dvars_threshold for scrubbing, so I went for it as well. We support FD via the spike parameter of fMRIPrepConfoundRemover.

spike is to add as a regressor, which is not scrubbing: github.com/juaml/junifer@e392c7bef7/junifer/preprocess/confounds/fmriprep_confound_remover.py (L411)

  1. As it is, it nows also adds DVARS as a regressor.

It generates a mask for indexing the time dimension and passes it to nilearn.image.clean as you suggested in your previous comment.

I see that you use input["confounds"] for the scrub mask and not confounds_df. We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of clean_img

I think the implementation should come from nilearn: https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#nilearn.interfaces.fmriprep.load_confounds
Basically here you can set the fd_threshold and std_dvars_threshold and it will give you the sample_mask

We don't use load_confounds but select the confounds we want to remove and pass it to nilearn.image.clean_img. Also, load_confounds relies on the confound file by following the path from the image file. Is that something we go for?

We need to somehow "mimic" that behaviour. That's what I meant. The scrubbing mask can be either from dvars or fd.

> > Two comments: > > > > 1. Why using DVARS? where did you get the reference for this method? I've seen scrubbing also using the FD > > `nilearn.image.load_confounds` has a parameter `std_dvars_threshold` for scrubbing, so I went for it as well. We support FD via the `spike` parameter of `fMRIPrepConfoundRemover`. `spike` is to add as a regressor, which is not scrubbing: https://github.com/juaml/junifer/blob/e392c7bef75d7ba3bc49a44b06b596f576fc4543/junifer/preprocess/confounds/fmriprep_confound_remover.py#L411 > > > 2. As it is, it nows also adds DVARS as a regressor. > > It generates a mask for indexing the time dimension and passes it to `nilearn.image.clean` as you suggested in your previous comment. I see that you use `input["confounds"]` for the scrub mask and not `confounds_df`. We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of `clean_img` > > > I think the implementation should come from nilearn: https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#nilearn.interfaces.fmriprep.load_confounds > > Basically here you can set the `fd_threshold` and `std_dvars_threshold` and it will give you the `sample_mask` > > We don't use `load_confounds` but select the confounds we want to remove and pass it to `nilearn.image.clean_img`. Also, `load_confounds` relies on the confound file by following the path from the image file. Is that something we go for? We need to somehow "mimic" that behaviour. That's what I meant. The scrubbing mask can be either from dvars or fd.
synchon commented 2025-01-29 09:48:22 +00:00 (Migrated from github.com)

I see that you use input["confounds"] for the scrub mask and not confounds_df. We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of clean_img

Do the new commits address it?

> I see that you use input["confounds"] for the scrub mask and not confounds_df. We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of clean_img Do the new commits address it?
fraimondo commented 2025-01-29 20:11:43 +00:00 (Migrated from github.com)

I see that you use input["confounds"] for the scrub mask and not confounds_df. We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of clean_img

Do the new commits address it?

We are still missing the fd_threshold and the scrub parameter. Also check the reference for scrubbing: https://www.sciencedirect.com/science/article/abs/pii/S1053811913009117?via%3Dihub

Coverage is low, meaning that we are missing tests.

> > I see that you use input["confounds"] for the scrub mask and not confounds_df. We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of clean_img > > Do the new commits address it? We are still missing the `fd_threshold` and the `scrub` parameter. Also check the reference for scrubbing: https://www.sciencedirect.com/science/article/abs/pii/S1053811913009117?via%3Dihub Coverage is low, meaning that we are missing tests.
synchon commented 2025-01-30 17:13:34 +00:00 (Migrated from github.com)

We are still missing the fd_threshold and the scrub parameter.

Just so that it's explicit:

  • Does fMRIPrepConfoundRemover's strategy take a key named scrub or does one pass it via the parameters?
  • From load_confounds docs: “scrub” regressors for Power et al.[[3]](https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#footcite-power2014) scrubbing approach. Associated parameter: scrub, fd_threshold, std_dvars_threshold. It treats them as "regressors" and when you pass "scrub" in the strategy, it does not return either framewise_displacement or std_dvars in the returned confounds dataframe.

We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of clean_img

I thought the columns should be in the returned dataframe?

  • If you are ok with how the std_dvars_threshold is currently implemented for fMRIPrepConfoundRemover, I can add fd_threshold as well. I'm not sure how scrub is supposed to be implemented?
> We are still missing the fd_threshold and the scrub parameter. Just so that it's explicit: - Does fMRIPrepConfoundRemover's `strategy` take a key named `scrub` or does one pass it via the parameters? - From `load_confounds` docs: `“scrub” regressors for Power et al.[[3]](https://nilearn.github.io/stable/modules/generated/nilearn.interfaces.fmriprep.load_confounds.html#footcite-power2014) scrubbing approach. Associated parameter: scrub, fd_threshold, std_dvars_threshold`. It treats them as "regressors" and when you pass `"scrub"` in the `strategy`, it does not return either `framewise_displacement` or `std_dvars` in the returned confounds dataframe. > We are good as long as dvars or any confounds used for the scrubbing appear in the confounds of clean_img I thought the columns should be in the returned dataframe? - If you are ok with how the `std_dvars_threshold` is currently implemented for fMRIPrepConfoundRemover, I can add `fd_threshold` as well. I'm not sure how `scrub` is supposed to be implemented?
synchon commented 2025-02-10 16:01:37 +00:00 (Migrated from github.com)

@fraimondo would really appreciate your feedback to the previous comment.

@fraimondo would really appreciate your feedback to the previous comment.
synchon commented 2025-02-11 16:27:11 +00:00 (Migrated from github.com)

I've updated the interface and also improved the logic by replicating nilearn's implementation, would appreciate a review @fraimondo @kaurao

I've updated the interface and also improved the logic by replicating nilearn's implementation, would appreciate a review @fraimondo @kaurao
fraimondo (Migrated from github.com) reviewed 2025-02-12 08:19:04 +00:00
fraimondo (Migrated from github.com) commented 2025-02-12 08:18:35 +00:00

Here you add the required variables for scrubbing to the df. Are they used later as regressors? Or they are not considered?

I fear that we might be regressing out this variables too.

Here you add the required variables for scrubbing to the df. Are they used later as regressors? Or they are not considered? I fear that we might be regressing out this variables too.
fraimondo (Migrated from github.com) commented 2025-02-12 08:18:57 +00:00

You can actually check the thresholds and which samples should be removed here.

You can actually check the thresholds and which samples should be removed here.
synchon (Migrated from github.com) reviewed 2025-02-12 08:29:51 +00:00
synchon (Migrated from github.com) commented 2025-02-12 08:29:51 +00:00

Motion outlier regressors are generated using the thresholds: github.com/nilearn/nilearn@7a8cd0ee71/nilearn/interfaces/fmriprep/load_confounds_components.py (L222) and then added to the df: github.com/nilearn/nilearn@7a8cd0ee71/nilearn/interfaces/fmriprep/load_confounds.py (L484) which then gets processed to generate the sample mask and the added motion outlier regressors are removed to return the proper confounds. This is leveraging nilearn's implementation so no code of ours should affect it.

Motion outlier regressors are generated using the thresholds: https://github.com/nilearn/nilearn/blob/7a8cd0ee7161c4ea5ce4c2df248f0113890e3070/nilearn/interfaces/fmriprep/load_confounds_components.py#L222 and then added to the df: https://github.com/nilearn/nilearn/blob/7a8cd0ee7161c4ea5ce4c2df248f0113890e3070/nilearn/interfaces/fmriprep/load_confounds.py#L484 which then gets processed to generate the sample mask and the added motion outlier regressors are removed to return the proper confounds. This is leveraging nilearn's implementation so no code of ours should affect it.
fraimondo (Migrated from github.com) reviewed 2025-02-13 08:34:23 +00:00
fraimondo (Migrated from github.com) commented 2025-02-13 08:34:23 +00:00

ok then.

ok then.
fraimondo (Migrated from github.com) approved these changes 2025-02-13 08:35:42 +00:00
fraimondo (Migrated from github.com) left a comment

Once CI is ready, we can merge

Once CI is ready, we can merge
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
juaml/junifer!421
No description provided.