Back to the main page.
Bug 2404 - ft_channelrepair returns a grad field which was not present before
| Status | ASSIGNED |
| Reported | 2013-12-03 10:29:00 +0100 |
| Modified | 2019-04-06 13:03:05 +0200 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P3 normal |
| Assigned to: | Robert Oostenveld |
| URL: | |
| Tags: | |
| Depends on: | 2405 |
| Blocks: | 2331 |
| See also: | http://bugzilla.fcdonders.nl/show_bug.cgi?id=2403 |
Jim Herring - 2013-12-03 10:29:47 +0100
After running ft_channelrepair on an EEG dataset using the 'nearest' method, the structure contains a grad field which was not present before. I think the problem occurs because in line 104-106 and 197-201: "% determine the type of data iseeg = ft_senstype(data, 'eeg'); ismeg = ft_senstype(data, 'meg');" the data is not recognized as being eeg data nor as being meg data. Further in the code (197-201) you can see that if iseeg is not 1, a grad field is added, regardless of the type: "if iseeg interp.elec = sens; else interp.grad = sens; end"
Robert Oostenveld - 2013-12-03 11:11:39 +0100
If I try the following test script data = []; data.label = {'1', '2', '3'}; data.time{1} = (1:1000)/1000; data.trial{1} = randn(3,1000); data.trial{1}(2,:) = 0; % broken channel cfg = []; cfg.channel = '2'; cfg.neighbours(1).label = '2'; cfg.neighbours(1).neighblabel = {'1', '3'}; fixed = ft_channelrepair(cfg, data); it fails with Error using ft_fetch_sens (line 179) no electrodes or gradiometers specified. Error in ft_channelrepair (line 112) sens = ft_fetch_sens(cfg, data); I am surprised that the function cannot simply compute the average over the neighbours any more. It now uses a distance-weighted average, which required the sens. Well. that is not something to fix now.
Robert Oostenveld - 2013-12-03 11:31:24 +0100
I have made some changes that might fix it for you mac001> svn commit ft_channelrepair.m test/test_bug2404.m Sending ft_channelrepair.m Adding test/test_bug2404.m Transmitting file data .. Committed revision 8942. However, the test script reveals some other problems. I will make a linked bug out of it.
Jim Herring - 2013-12-03 11:32:52 +0100
Created attachment 567 test script producing the problem
Jim Herring - 2013-12-03 11:33:14 +0100
Created attachment 568 sample data
Jim Herring - 2013-12-03 11:35:31 +0100
Ahh, I just added a script and sample of data reproducing the problem. I'll check if your fix solves the problem for the sample I just uploaded.
Matt Mollison - 2013-12-06 21:17:23 +0100
I downloaded fieldtrip-20131204 and ended up receiving a new version of ft_channelrepair. I have a workflow that passes data from ft_preprocessing to ft_channelrepair if a channel needed interpolating, but the new output of ft_channelrepair has changed and fields that I relied on are no longer included. To be specific, hdr, fsample, and cfg are missing, and I used fsample to determine the max frequency for subsequent filtering during artifact checking. I could probably work around it, but I thought I'd see whether this was done for a good reason or if it's an oversight/bug. Here is what I pass into the new ft_channelrepair: data = hdr: [1x1 struct] label: {128x1 cell} fsample: 250 trial: {1x1736 cell} time: {1x1736 cell} trialinfo: [1736x20 double] sampleinfo: [1736x2 double] cfg: [1x1 struct] and here is what I get out: data = label: {128x1 cell} time: {1x1736 cell} trial: {1x1736 cell} sampleinfo: [1736x2 double] trialinfo: [1736x20 double] elec: [1x1 struct] As I said earlier, there are missing fields, which I can see were deleted using this URL (lines 193 and 340) https://code.google.com/p/fieldtrip/source/diff?spec=svn8985&r=8942&format=side&path=/trunk/ft_channelrepair.m&old_path=/trunk/ft_channelrepair.m&old=8384 Maybe they (or at least fsample, as data.cfg and data.hdr do not look like they were included in the previous version) should be added at line 336 with the other fields (e.g., trialinfo)?
Matt Mollison - 2013-12-06 21:32:33 +0100
(In reply to comment #6) Oh sorry, there is a cfg field when it actually returns to the parent function. In my prior post I was investigating the fields at a break point at the end of ft_channelrepair where data = interp;
Robert Oostenveld - 2013-12-09 11:40:10 +0100
(In reply to comment #6) Hi Matt Please see the documentation in utilities/ft_datatype_raw: % Required fields: % - time, trial, label % % Optional fields: % - sampleinfo, trialinfo, grad, elec, hdr, cfg % % Deprecated fields: % - fsample % % Obsoleted fields: % - offset We are trying to move away from having fsample in the raw data, as it is redundant with the time axis and potentially conflicting (i.e. which one is correct if they differ?). There are some FT functions that still use it, hence it is still added by ft_checkdata at the start of the FT function. But in the output of FT functions it preferably should not be present. hdr should only be present immediately after reading it, not after other steps on the data. It is also largely redundant, as the raw data should describe itself. FT functions never have used data.hdr. The risk in hdr is that labels might be renamed (with an EEG montage), channel subsets are made, balancing is applied to MEG gradiometers, data is resampled. All of those steps would invalidate the information in hdr, that is why it should not be in the output. We have had (and probably still have) functions that work like this function data = dosomething(cfg, data) data.field = updatedfield; but that is a risky programming style, since there might be other fields in the data that now get copied to the output without properly being updated. The safer style of FT functions is function datanew = dosomething(cfg, datanew) datanew.field = updatedfield; datanew.something1 = dataold.something1; % explicitly knowing that it still is valid datanew.something2 = dataold.something2; % explicitly knowing that it still is valid datanew.something3 = dataold.something3; % explicitly knowing that it still is valid I suspect that ft_channelrepair changed from the first (leaving stuff in place, only updating fields) to the new style (copying fields according to explicit design). That, linked with the ft_datatype_raw documentation, results in fewer output fields being present than there were in the past.
Jörn M. Horschig - 2013-12-17 14:30:42 +0100
Hey guys, you bugged ft_channelrepair in revision 8942 by not updating the label-field appropriately if missing channels are added in the end http://code.google.com/p/fieldtrip/source/detail?spec=svn9038&r=8942 I fixed that svn ci ft_channelrepair.m -m "bugfix - correct labels added after rev. 8942 juggled things around" Sending ft_channelrepair.m Transmitting file data . Committed revision 9039. I'll keep an eye on whether the order of the labels is still in accordance with the data ;)