Back to the main page.

Bug 2539 - unnecesary data check ft_topoplotIC

Status CLOSED FIXED
Reported 2014-04-15 21:38:00 +0200
Modified 2017-10-05 09:36:40 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: All
Operating System: All
Importance: P5 normal
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also: http://bugzilla.fcdonders.nl/show_bug.cgi?id=2518http://bugzilla.fcdonders.nl/show_bug.cgi?id=2559http://bugzilla.fcdonders.nl/show_bug.cgi?id=1887

rené scheeringa - 2014-04-15 21:38:56 +0200

Whenever I use ft_topoplotIC fieldtrip performs a check on the data and checks if the .time field is present. I never save this, because I only need to save the mixing, unmixing and topolabel fields for the rest of my anlyses. Saving all the trial data and the time fields only takes unnecesary disk space. The bug is easy to 'fix' by introducing a fake .time field, but it would be cleaner if the ft_topolotIC only checks for the precense of the mixing weights and the topolabels. More is not needed for plotting the IC topographies. Error notification: ??? Reference to non-existent field 'time'. Error in ==> ft_datatype_raw at 143 data.fsample = 1/mean(diff(data.time{1})); Error in ==> ft_datatype_comp at 122 rawdata = ft_datatype_raw(rawdata, 'version', rawversion, 'hassampleinfo', hassampleinfo, 'hastrialinfo', hastrialinfo); Error in ==> ft_checkdata at 222 data = ft_datatype_comp(data, 'hassampleinfo', hassampleinfo); Error in ==> topoplot_common at 80 data = ft_checkdata(data, 'datatype', {'timelock', 'freq', 'comp'}); Error in ==> ft_topoplotTFR at 185 [cfg] = topoplot_common(cfg, varargin{:}); Error in ==> ft_topoplotIC at 148 ft_topoplotTFR(cfg, varargin{:});


Robert Oostenveld - 2014-04-15 22:22:54 +0200

fair point. We discussed this actually 2 weeks ago in our FT meeting. See http://fieldtrip.fcdonders.nl/development/meeting/20140402 Here are the relevant notes. support for component data in source reconstruction functions: 1. dipolefitting and mne should in principle be doable. 2. yet, the functions may become confused because of the mixed representation of the component data (in a freq/tlck structure). 3. identify which functions should support this, but currently don't do it well. 4. ensure that the data is unambiguous, i.e. only keep topo/topolabel and remove anything else. More general: whenever a function needs only the component topographies, it should remove the other fields as soon as possible. This is followed up in detail in bug 2518. However, I think that the changes I implemented for bug 2518 might have caused an error to slip in, since datatype=comp in the past always meant raw-comp, whereas it now can also be timelock-comp or freq-comp. The error below suggests that the code expects raw-comp, but that the data is different. Can you share the data and the cfg you are using?


rené scheeringa - 2014-04-15 22:56:57 +0200

Created attachment 612 ICA mixing and unmixing weights for one subject Attached are the mixing and unmixing weights for a single subject. They were computed more than two weeks ago, so before the fieldtrip meeting. I ran another ICA today with Floris de Vries and encountered what I think are some other unnesecary dependencies if you run it now. I can look into that tomorrow. Ik commented out the adding of the weigths.time field that 'solves' the problem. %weights.time{1}=nan; sz=size(weights.topo); cfg=[]; cfg.component = 1:16; cfg.layout = '/home/language/rensch/layers/data/EEG/scripts/MR64.lay'; cfg.zlim = 'maxabs'; cfg.marker = 'on'; figure(11);ft_topoplotIC(cfg, weights);


Robert Oostenveld - 2014-04-16 17:05:13 +0200

thanks, I added the data to /home/common/matlab/fieldtrip/data/test/bug2539.mat


Robert Oostenveld - 2014-04-16 17:23:03 +0200

*** Bug 2520 has been marked as a duplicate of this bug. ***


Robert Oostenveld - 2014-04-16 17:31:50 +0200

I was able to reproduce the problem. It was due to weights = topo: [63x63 double] unmixing: [63x63 double] topolabel: {63x1 cell} being recognised as comp, and subsequently assuming that it was also raw (which it was not). I have changed ft_datatype such that it distinguishes between comp comp+raw comp+timelock comp+freq in line with the requested functionality enhancements for bug 2518. mac011> svn commit utilities/ft_datatype*m test/test_bug2539.m Adding test/test_bug2539.m Sending utilities/ft_datatype.m Sending utilities/ft_datatype_comp.m Transmitting file data ... Committed revision 9430.


Robert Oostenveld - 2014-04-17 09:12:11 +0200

there is a regression error in the script test_bug1887. The output on the last line has a topi, whereas it is not supposed to have one.


Robert Oostenveld - 2014-04-17 09:12:22 +0200

(In reply to Robert Oostenveld from comment #6) topi=topo


Robert Oostenveld - 2014-07-16 12:18:04 +0200

the check on timelock/comp/freq/raw is now implemented correctly


Robert Oostenveld - 2015-02-11 10:40:34 +0100

Closed several bugs that were recently resolved. Please reopen if you are not happy with the resolution.


Robert Oostenveld - 2017-10-05 09:36:40 +0200

the recent update of the plotting functions caused this test script to fail. It was assuming that a "comp" structure would also have a label field, which it only has in case it is comp+raw, comp+timelock or comp+freq. I have updated the code to deal with this. mac011> git commit -a [bug2539 aa87f38] FIX - improve handling of component structure without timeloock/freq/raw data, see http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2539 4 files changed, 23 insertions(+), 33 deletions(-)