Back to the main page.
Bug 3238 - improve support for partial artifact handling using NaNs
| Status | CLOSED FIXED |
| Reported | 2017-01-31 13:20:00 +0100 |
| Modified | 2021-09-16 14:52:08 +0200 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Mac OS |
| Importance: | P5 normal |
| Assigned to: | Jan-Mathijs Schoffelen |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: | http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3193http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2645http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3220http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=1661http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2818http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3337 |
Robert Oostenveld - 2017-01-31 13:20:42 +0100
as discussed at the M/EEG meeting. @Lorijn: I created an account with a random password for you, please reset it (that goes via email). TODO - identify known issues - create test script, starting from fabricated data - improve existing FT functions
Robert Oostenveld - 2017-01-31 13:55:15 +0100
i noticed in fieldtrip/private/preproc (where the filtering etc is done) this 6b5bd0e9 (Eelke Spaak 2013-11-04 13:43:22 +0000 130) fsample = 1./nanmean(diff(time)); which to me does not make sense: the time axis should never contain nans as far as I know. This traces back to https://github.com/fieldtrip/fieldtrip/commit/6b5bd0e9 which refers to bug 2361. There seems to be some unexpected use of nans, which we should look into.
Robert Oostenveld - 2017-01-31 14:00:03 +0100
There is also another section in private/preproc that pertains to nans, around line 284 if any(any(isnan(dat))) % filtering is not possible for at least a selection of the data ft_warning('data contains NaNs, no filtering or preprocessing applied'); else % here it does the filtering I detected a related change around here in github at https://github.com/fieldtrip/fieldtrip/commit/3c71793b but the actual insertion of that code seems to predate the use of github. History (and the associated reasoning) can therefore not easily be found. right now it simply skips the processing for an affected trial. I don't think that is good!
Robert Oostenveld - 2017-01-31 14:04:20 +0100
(In reply to Robert Oostenveld from comment #2) in the actual filtering function there is this section 376514a1 preproc/ft_preproc_bandpassfilter.m (Robert Oostenveld 2016-11-07 10:31:41 +0100 141) ft_warning('FieldTrip:dataContainsNaN', 'data contains NaN values'); which was inserted in 2016, well after the skip implemented in preproc. I think it is better to rely on the low-level handling in the ft_preproc_xxx functions than on skipping the processing altogether.
Robert Oostenveld - 2017-01-31 14:08:33 +0100
(In reply to Robert Oostenveld from comment #3) These ones do consider nans already mac011> grep -l isnan *.m ft_preproc_bandpassfilter.m ft_preproc_bandstopfilter.m ft_preproc_denoise.m ft_preproc_derivative.m ft_preproc_dftfilter.m ft_preproc_highpassfilter.m ft_preproc_hilbert.m ft_preproc_lowpassfilter.m ft_preproc_medianfilter.m ft_preproc_polyremoval.m ft_preproc_rereference.m ft_preproc_resample.m ft_preproc_slidingrange.m ft_preproc_smooth.m ft_preproc_standardize.m These ones do NOT consider nans at the moment. mac011> grep -L isnan *.m ft_preproc_baselinecorrect.m -> calls ft_preproc_polyremoval ft_preproc_detrend.m -> calls ft_preproc_polyremoval ft_preproc_padding.m -> bookkeeping only ft_preproc_rectify.m -> works on individual samples, so fine and these ones are for realtime processing, where nans don't apply: ft_preproc_online_downsample_apply.m ft_preproc_online_downsample_init.m ft_preproc_online_filter_apply.m ft_preproc_online_filter_init.m So the nan-aware processing seems to be present in the ft_preproc_xxx functions, the task is now to port that correctly to the higher-level functions so that ft-preprocessing does it correctly.
Robert Oostenveld - 2017-01-31 14:13:04 +0100
I made an initial test script with the name "failed_bug3238" (since it still fails at the moment). The test script is part of the master (i.e. release) branch of fieldtrip. mac011> git commit [master 4c281ea] ENH - created test script for http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3238, the test script is known to be failing right now 1 file changed, 158 insertions(+) create mode 100644 test/failed_bug3238.m To contribute, you should use the latest version of fieldtrip, which requires using git. To get started using git, please look at http://www.fieldtriptoolbox.org/development/git If you follow the instructions there, you will also learn how to create a "pull request", i.e. a proposal for code updates to be included in the release. I can review those and include them. @Lorijn, please give it a try and let me know whether you can make sense of it. I will still link some related bugs to this one (for reference), but won't work on the code for now.
Robert Oostenveld - 2017-01-31 14:28:29 +0100
searching through bugzilla, I encountered ft_interpolatenan. That is a FT function which I actually forgot about. I don't think it is relevant in light of the discussion of Monday (on baby EEG) but should be kept in mind. The function is used in http://www.fieldtriptoolbox.org/tutorial/tms-eeg. The short-lived pulse artefact in TMS-EEG can be represented as nans. I updated the documentation. mac011> git commit -a [master 0aba9a5] DOC - improved documentation, point to related functions. Some small cleanups to the code, nothing functional. 2 files changed, 49 insertions(+), 43 deletions(-)
Lorijn Zaadnoordijk - 2017-02-13 19:01:35 +0100
(In reply to Robert Oostenveld from comment #5) Hi Robert, I'm slowly getting used to both bugzilla and at least extracting code from git. I found the failing script that you referred to. It indeed failed for me too, and it indeed failed exactly where my own code is failing: Rereferencing when there are still NaNs in the trial information. It seems that my colleagues so far have used a hack around it (basically by removing the channels filled with NaNs entirely from the data-file and storing them elsewhere for bookkeeping), but for the method that we proposed (to fill a channel with NaNs for individual trials instead of an entire experiment) this would not work. This is because one ends up with a matrix of channels that needs to be send as an input (namely per trial) to the re-referencing step rather than a vector, and it seems to get confused by the empty spaces in that matrix for the channels that are missing in a trial. It's also probably not the cleanest way to solve the issue. I'm not entirely sure what the next step is. What could I do?
Jan-Mathijs Schoffelen - 2020-11-13 19:54:12 +0100
I have opened a PR on github: https://github.com/fieldtrip/fieldtrip/pull/1594/