Back to the main page.
Bug 2954 - Bug when reading a subset of channels from an EDF file
| Status | CLOSED FIXED |
| Reported | 2015-08-27 13:39:00 +0200 |
| Modified | 2016-06-14 16:14:54 +0200 |
| Product: | FieldTrip |
| Component: | fileio |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P5 normal |
| Assigned to: | Robert Oostenveld |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: | http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2887 |
Vladimir Litvak - 2015-08-27 13:39:34 +0200
I have a complaint from an SPM user who gets an error when she tries to convert an EDF file when selecting a subset of channels. This comes down to line 345 in read_edf.m chanindx=[1:nchans]; %JD This line overrides the user-specified channel set. It seems this line is there for a reason, but then the problem should be fixed elsewhere. I think there was a part in ft_read_data before that selected a subset of channels in case the low-level functions didn't do it, but I don't see that now. Vladimir
Robert Oostenveld - 2015-08-27 15:28:56 +0200
Hi Vladimir, I am not sure whether the bug you report is a proper error in the software, or more of a user error. It happens if variableFs is true, which means that not all channels can be read in at the same time. Only channels with the same sampling rate can be read in one go, since the output of ft_read_data is a nchan*nsamples matrix. See also bug 2887 and http://www.fieldtriptoolbox.org/faq/how_can_i_read_all_channels_from_an_edf_file_that_contains_multiple_sampling_rates I don't recall all details, but the way that such an EDF file is to be read is by specifying chanindx both to ft_read_header and to ft_read_data. This ensures that hdr.Fs and hdr.nSamples applies to the selected channels that you would be reading with ft_read_data. The edf2fieldtrip function should also explain it.
Vladimir Litvak - 2015-08-27 15:35:21 +0200
(In reply to Robert Oostenveld from comment #1) Hi Robert, The problem is that with the code as it is now any user-specified channel selection will be ignored. I can tell the users to only select channels with the same sampling rate, but that won't make a difference. If there is any variability in the sampling rates, all channels are always selected (not sure why). This creates an error in SPM which expects to get a matrix of certain size from ft_read_data but gets a matrix of a different size. I'd suggest to either do exactly what the user specified or give an error, but not override the user specification with something else. Vladimir
Robert Oostenveld - 2015-08-27 16:35:53 +0200
(In reply to Vladimir Litvak from comment #2) I made a test script and now see the problem. That line from JD should simply not be there. This fixes it. mac011> svn commit test/test_bug2954.m fileio/private/read_edf.m Sending fileio/private/read_edf.m Adding test/test_bug2954.m Transmitting file data .. Committed revision 10638.
Robert Oostenveld - 2015-08-27 17:34:04 +0200
according to Joe (by email) the lien should be there. To be further discussed...
Joseph Dien - 2015-08-28 20:53:28 +0200
The line did indeed need to be there. The code was a bit messy due to the parallel use of chanindx and hdr.orig.chansel to specify what channels were to be selected. I rewrote the code to more clearly handle the contingencies of manual user selection of channels via chanindx and automatic read_edf selection of channels in the presence of heterogeneous sampling rates. I've also added documentation to the header to explain exactly how read_edf is behaving with respect to channel selection. I've also fixed my bugs in ft_read_data with respect to handling presence versus absence of hdr and chanindx contingencies. Sorry about the problems. The function is a bit of a kludge of a kludge. Might have been best to just rewrite it from the ground up but I think it should work properly now. Please let me know if you see any further issues. Joe
Robert Oostenveld - 2015-10-17 10:45:57 +0200
The test script was not working properly (since the first line was commented out). I fixed it, and extended it with an EDF file with variable sampling. Previously it only had a fixed sampling rate EDF file as test case. I also added a test case for edf2fieldtrip. mac011> svn commit `cat out` Sending edf2fieldtrip.m Sending fileio/ft_read_header.m Sending fileio/private/read_edf.m Sending ft_databrowser.m Sending test/dccnpath.m Sending test/test_bug2954.m Transmitting file data ...... Committed revision 10783. See also https://github.com/fieldtrip/fieldtrip/commit/cebdeed32a2154e446cfc40361c16c9b2195a6e5