The goal of this issue is partly to implement/support scrubbing of time series.
This issue depends on #1011
I had a look at the relevant code of sample_mask application for resolving #2777. Currently, the timeseries censoring is done in the base_masker module:
https://github.com/nilearn/nilearn/blob/4cdd8fe8aad085d21e5d8367ad5b94d8efbe5151/nilearn/input_data/base_masker.py#L71-L73
It is rather trivial if we are just moving sample_mask to the transform method. However, while looking into this, I found that the current test cases don't cover the situation when both sample_mask and confounds passed to the function. I am wondering if we should check this in the relevant function of base_masker, or should it be part of the signal.clean?
The benefit of keeping it in base_masker: easy to make the change.
The benefit of keeping it in signal.clean: allow scrubbing for general application.
Will need to add new test either way.
Are there any cases other than the nifti masker where scrubbing is needed within the current nilearn API?
WDYT? cc'd @pbellec @bthirion @NicolasGensollen
Thx for raising that point. This is not a strong opinion, but since signal.clean is part of the public API, it may be used directly by some people. So I would test the compatibility btw sample_mask and confound there.
Thanks @htwangtw ! I had a look and I think we can move sample_mask down to signal.clean without breaking things. We will lose a little bit computation-wise since we will resample (if needed...) and extract signals for all time instants instead of doing it on the masked image, but that shouldn't be noticeable.
I found that the current test cases don't cover the situation when both sample_mask and confounds passed to the function
Indeed. Sorry if this is a stupid question, but is sample_mask supposed to apply to both the niimg and the confounds or is the user responsible for providing confounds matching the shape of the "censored niimg"?
I would expect sample_mask to apply to both but this is not what is implemented currently, right?
Are there any cases other than the nifti masker where scrubbing is needed within the current nilearn API?
I don't know, but I also think it makes sense to implement this in signal.clean as this would allow scrubbing for general application as you said.
Indeed. Sorry if this is a stupid question, but is
sample_masksupposed to apply to both the niimg and the confounds or is the user responsible for providing confounds matching the shape of the "censored niimg"?
I would expectsample_maskto apply to both but this is not what is implemented currently, right?
With the current Nilearn Docs - there's not clear instruction at all! Exactly as you said, sample_mask should apply to both, but that's not implemented. I guess so few people use that argument so that never got found....
On the load_confounds side, @pbellec suggested to separate the scrubbing related API, as the output is very different from the rest. A normal output would be a matrix of confounds = (n_timepoints, n_regressors), but with scrubbing it returns a sample_mask = (n_timepoints - n_scrubbed, ) and confounds = (n_timepoints - n_scrubbed, n_regressors). However, we haven't closed that PR yet so technically we haven't settle on the solution.
The most sensible change I can thing of is something like this:
scrubbing = {False, "censor", "regression"} for example). For scrubbing, signal.clean accepts a sample_mask with size (n_timepoints - n_scrubbed, ) and confounds (n_timepoints, n_regressors)`.
sample_mask, and instead accept a pandas dataframe of size (n_timepoints - n_scrubbed, n_regressors), and the index of the data frame would be the input of sample_mask.The down side of this approach is that different input length can be confusing to user.
Let me know if this make sense and if it's a good idea to cover it with the scope of applying sample_mask on the fly.
Are there any cases other than the nifti masker where scrubbing is needed within the current nilearn API?
I don't know, but I also think it makes sense to implement this in
signal.cleanas this would allow scrubbing for general application as you said.
We already got some code to do so in load_confounds. Right now on the load_confounds side, we are finalising the strategies. Once that's done, I will start migrating the code and think of how to best integrate that with signal.clean
Thanks for the explanation @htwangtw !
I am not sure to fully understand what you propose, so I might just be rephrasing below what you said above...
I think that the option that would make the most sense would be to have signal.clean accept as inputs:
signal : shape (n_timepoints, n_features)confounds: shape (n_timepoints, n_regressors)sample_mask: either False (default value), or some indexing which will mask out n_scrubbed time points among the n_timepointsThis will output a "cleaned signal" with the same shape as the input signal if sample_mask was left to False, and with shape (n_timepoints - n_scrubbed, n_features) if sample_mask was provided.
Does this make sense or am I missing something?
I personally like the idea to apply scrubbing both to signal and confounds, so that confounds are of shape (n_timepoints, n_regressors) . This avoids creating a non-trivial dependency between confounds and sample_mask.
Best,
@NicolasGensollen Yes that's correct!
After a night of sleep would say it's definitely better, in a pure nilearn workflow, to have confounds of shape (n_timepoints, n_regressors) as @bthirion said.