Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Confound model enhancement #1586

Merged
merged 57 commits into from
May 4, 2019
Merged

ENH: Confound model enhancement #1586

merged 57 commits into from
May 4, 2019

Conversation

oesteban
Copy link
Member

Replaces #1487 after my mistake forcing a push.

rciric and others added 30 commits January 25, 2019 23:40
[MAINT] Repin dependencies
@oesteban oesteban changed the title [WIP] ENH: Confound model enhancement ENH: Confound model enhancement Apr 26, 2019
oesteban added a commit to oesteban/niworkflows that referenced this pull request Apr 29, 2019
This PR updates the fMRIPrep report specification to include derivatives
generated in the context of nipreps/fmriprep#1586
Copy link
Member Author

@oesteban oesteban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 . Tagging @effigies for a quick look if time permits

@@ -173,7 +173,7 @@ motion-correction parameters estimated by fMRIPrep;
if present, ``non_steady_state_outlier_XX`` columns indicate non-steady state volumes with a single
``1`` value and ``0`` elsewhere (*i.e.*, there is one ``non_steady_state_outlier_XX`` column per
outlier/volume);
six noise components are calculated using :abbr:`CompCor (Component Based Noise Correction)`,
additional noise components are calculated using :abbr:`CompCor (Component Based Noise Correction)`,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rciric I feel we will need to expand this and elaborate a bit more about what components were dropped, how they are denoted and why they were dropped. I'd be fine to do that on another PR (with a branch called docs/<whatever> we would be saving a lot of CircleCI cycles).

signals were expanded with the inclusion of temporal derivatives and
quadratic terms for each [@confounds_satterthwaite_2013].
Frames that exceeded a threshold of {fd} mm FD or {dv} standardised DVARS
were classified as motion outliers.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
were classified as motion outliers.
were annotated as motion outliers.

maybe better?

the aforementioned mask and the union of CSF and WM masks calculated
in T1w space, after their projection to the native space of each
functional run (using the inverse BOLD-to-T1w transformation).
functional run (using the inverse BOLD-to-T1w transformation). Components
are also calculated separately within the WM and CSF masks.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in my previous comment (i.e. to-do in another PR), we should update the boilerplate to mention the component selection strategy. If that is described in Satterthwaite 2013, it'd be pertinent to also add the reference here.

@oesteban oesteban requested review from effigies and jdkent May 3, 2019 14:03
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a pretty quick look through and only saw one thing.

fmriprep/workflows/bold/confounds.py Outdated Show resolved Hide resolved
@oesteban
Copy link
Member Author

oesteban commented May 4, 2019

Good job @rciric!

@oesteban oesteban merged commit d4786c3 into nipreps:master May 4, 2019
@oesteban oesteban deleted the pr/1487 branch May 4, 2019 16:23
@effigies effigies mentioned this pull request May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants