fmriprep icon indicating copy to clipboard operation
fmriprep copied to clipboard

--skip-confounds flag (or similar)

Open effigies opened this issue 2 years ago • 9 comments

What would you like to see added in fMRIPrep?

Confounds can cause problems, and we produce so many that people will not use all of them. For example, I just found an issue in dvars: https://github.com/nipy/nipype/issues/3487

We have other situations where a degenerate mask may result in an array of shape (), which causes inscrutable numpy errors. Allowing people to disable confounds that they do not intend to use is going to be a better solution than fixing all confounds for all data repeatedly.

I suggest --skip-confounds that would allow users to specify dvars, fd, *compcor, etc. This can be added to 20.2.x, 21.0.x and the upcoming 22.0.0.

Not that we shouldn't fix bugs, but our release process can be slow at times, especially if bugs crop up high up the dependency chain.

Do you have any interest in helping implement the feature?

Yes

Additional information / screenshots

No response

effigies avatar Jul 07 '22 16:07 effigies

Alternately, we could also add confounds to --ignore...

effigies avatar Jul 07 '22 17:07 effigies

@mgxd @oesteban I'm going to start working on this. Any thoughts/objections?

effigies avatar Jul 11 '22 14:07 effigies

The overall idea seems very reasonable.

At a first sight, I'm inclined towards a language reusing --ignore, e.g., --ignore confounds-dvars confounds-fd confounds-{a,f,}compcor

Alternatively, we could be more lenient handling errors and whenever they happen write n/a for the full timeseries of the failing confound.

That would be more amenable to users and would not interact with the CLI. On the output and the reports, we would need to signal the absence of these confounders very clearly (more emphatically than the option of allowing the user specify --ignore/--skip mechanisms)

oesteban avatar Jul 11 '22 14:07 oesteban

Alternatively, we could be more lenient handling errors and whenever they happen write n/a for the full timeseries of the failing confound.

This would require every such interface to be adapted, which is what I'm trying to work around so that we can get this into LTS.

At a first sight, I'm inclined towards a language reusing --ignore, e.g., --ignore confounds-dvars confounds-fd confounds-{a,f,}compcor

This is fine with me.

effigies avatar Jul 11 '22 14:07 effigies

I would be okay with this, but just my 0.02 - I don't think it quite fits in --ignore, as to me that should be reserved for ignoring certain attributes / files of a BIDS dataset. Maybe a --skip flag would better reflect cases like this where whole steps of the workflow are ignored, but like Oscar said we would need to be very clear to signal this in the reports.

mgxd avatar Jul 11 '22 15:07 mgxd

Do we have other skips that this would fit with? Otherwise I think --skip-confounds will be clearer.

effigies avatar Jul 11 '22 15:07 effigies

--skip-bids-validation seems like a reasonable argument to deprecate and include in --skip.

mgxd avatar Jul 11 '22 15:07 mgxd

Okay. So will use --skip confounds-dvars, etc for LTS/21.0. For 22.0 we can deprecate --skip-bids-validation for --skip bids-validation.

effigies avatar Jul 11 '22 15:07 effigies

This has turned out to be way more painful than I had guessed. I think fixing dvars and re-releasing nipype is going to be the shorter path.

effigies avatar Jul 12 '22 00:07 effigies