Back to the main page.
Bug 1181 - ft_preproc_detrend and ft_preproc_polyremoval should be merged
| Status | CLOSED FIXED |
| 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 |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| 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.