Back to the main page.

Bug 1181 - ft_preproc_detrend and ft_preproc_polyremoval should be merged

Reported 2011-11-23 11:05:00 +0100
Modified 2012-03-14 10:00:40 +0100
Product: FieldTrip
Component: preproc
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P2 normal
Assigned to: Eelke Spaak
Depends on:
See also:

Eelke Spaak - 2011-11-23 11:05:08 +0100

Since they basically perform the exact same transformation. Also, note that lines 60-64 of polyremoval read: % estimate the contribution of the basis functions %a = dat(:,begsample:endsample)/x(:,begsample:endsample); <-this leads to %numerical issues, even in simple examples invxcov = inv(x(:,begsample:endsample)*x(:,begsample:endsample)'); a = dat(:,begsample:endsample)*x(:,begsample:endsample)'*invxcov; And lines 61-62 of detrend read: % estimate the polynomial trend using General Linear Modeling, where dat=beta*x+noise beta = dat(:,begsample:endsample)/x(:,begsample:endsample); Oops :)

Jan-Mathijs Schoffelen - 2011-11-23 11:38:42 +0100

Yes, I made the change to polyremoval at some point. Forgot about detrend.

Boris Reuderink - 2011-11-28 14:40:16 +0100

Since the comment in polyremoval suggests numerical instability I have marked this bug with normal severity. And, since this bug de-duplicates code and thus potential for errors, I have made it high-priority. Defect by developer -> changed status to NEW.

Jan-Mathijs Schoffelen - 2011-12-07 13:54:01 +0100

solution: ft_preproc_detrend and ft_preproc_baselinecorrection should become a wrapper around ft_preproc_polyremoval, specifying the correct order of the polynome to be fitted. this doesn't need any changes to higher level ft-code

Eelke Spaak - 2011-12-09 12:25:38 +0100

I think ft_preproc_baselinecorrect should remain separate, since it supports using just a specified interval to compute the mean, and subtract that mean from all data. ft_polyremoval only supports a full demeaning in the strictest sense, i.e. compute the mean of all the data. By the way, actually the name "demean" of the cfg.demean option in preprocessing is sort of misleading, don't you think? Since it is actually a baseline correction that you can do with it, if you specify a baselinewindow other than the complete trial. Shall I rename it to "cfg.baselinecorrect" or so?

Eelke Spaak - 2011-12-09 12:34:54 +0100

Oops, I was mistaken; polyremoval *does* support estimating the model on a subselection of data. The remark about the name "demean" still stands though.

Eelke Spaak - 2011-12-09 12:53:19 +0100

bash-3.2$ svn commit Sending ft_preprocessing.m Sending preproc/ft_preproc_baselinecorrect.m Sending preproc/ft_preproc_detrend.m Sending preproc/ft_preproc_polyremoval.m Transmitting file data .... Committed revision 4962.

Eelke Spaak - 2012-03-14 10:00:40 +0100

closing several bugs that have been RESOLVED for a long time