include highest order in _cosine_drift function

[jrasero]

Hi,

I don't know how relevant this might be, but given that the _cosine_drift function was originally copied from nipy, it seems that nipype is currently exhibiting the same issue - if it is an issue at all - as the one described here https://github.com/nipy/nipy/issues/448, which has to do with the no inclusion of the highest order in the cosine basis expansion.

It all comes down to this part of the _cosine_drift function in https://github.com/nipy/nipype/blob/master/nipype/algorithms/confounds.py#L1523

 for k in range(1, order):
        cdrift[:, k - 1] = nfct * np.cos((np.pi / len_tim) * (n_times + 0.5) * k)

where the last order in the range is not included.

This exact issue seemed to be solved in nilearn after a pull request https://github.com/nilearn/nistats/pull/301

So I don't what are your thoughts on this, given that it directly affects things like CompCor, which fmriprep uses for example.

[satra]

@jrasero - the last column is set as the constant vector (corresponding to k=0) right after the for loop, so i believe that function is doing what it is supposed to. i.e. for order N it's going up to N-1 starting at 0 stored in the columns as [1, ..., N-1, 0] (https://en.wikipedia.org/wiki/Discrete_cosine_transform#DCT-II)

[jrasero]

Yeah I know, but in the way this function is written right know, the frequency corresponding to the order variable is not included, because range yields numbers from 1 to order -1. Then, the function documentation isn't totally true when it says: "Create a cosine drift matrix with periods greater or equals to period_cut", right?

That means, for example, that when doing a high-pass filtering by including the cosine terms created by this function, we are preserving the cutoff frequency too.

I mean, it's a minor thing, but as heavy user of nipype and nilearn, I was just curious to know if at some point it would be interesting to have both giving the same thing.