Back to the main page.

Bug 1158 - Wrong channeighbstructmat in newer ft versions

Status CLOSED FIXED
Reported 2011-11-16 08:12:00 +0100
Modified 2013-03-20 12:17:50 +0100
Product: FieldTrip
Component: core
Version: unspecified
Hardware: Other
Operating System: Linux
Importance: P1 critical
Assigned to: Jörn M. Horschig
URL:
Tags:
Depends on:
Blocks:
See also:

Gregor Volberg - 2011-11-16 08:12:04 +0100

This potential bug occurred during the call of ft_freqstatistics with cfg.correctm = 'cluster'. Although the same neighbourhood-structure and the same overall cfg was used, older versions of ft (fieldtrip-20100810, fieldtrip-20101010) revealed a different clustering of significant electrodes than newer versions (fieldtrip-20111012, fieldtrip-20111028). In the newer versions, electrodes that were actually not neighbours were grouped into one cluster. A deeper look revealed that older and newer fieldtrip versions constructed different channeighbstructmat matrices from the same neighbours, possibly due to an irregular call of the spm function 'spm_bwlabel.m' in the newer version. During the call of ft_freqstatistics, a warning is issued by fieldtrip-20111012 saying that the spm8 toolbox is added to the path. (after computing the montecarlo statistics and before entering the cluster randomization; 'Warning: adding /loctmp/m-lib/fieldtrip-20111012/external/spm8 toolbox to your Matlab path'). Accordingly, removing the spm8 toobox from the fieldtrip-20111012 installation issues the following error: ??? Undefined function or method 'spm_bwlabel' for input arguments of type 'double'. Error in ==> findcluster at 95 [labelmat(spatdimlev, :, :), num] = spm_bwlabel(double(reshape(onoff(spatdimlev, :, :), nfreq, ntime)), 6); % the previous code contained a '4' for input Error in ==> clusterstat at 197 posclusobs = findcluster(reshape(postailobs, [cfg.dim,1]),channeighbstructmat,cfg.minnbchan); Error in ==> statistics_montecarlo at 319 [stat, cfg] = clusterstat(cfg, statrand, statobs,'issource',issource); Error in ==> ft_freqstatistics at 279 [stat, cfg] = statmethod(cfg, dat, cfg.design); An example output is provided below. fieldtrip-20101010 and fieldtrip-20111012 both identify two positive clusters and identify the same electrodes as candidates for clustering. However, fieldtrip-20111012 combines electrodes into clusters that are not actually neighbours. **** Example code cfg=[]; cfg.neighbours = neighbours; cfg.frequency = [5 8]; cfg.latency = [0 0.7]; cfg.avgovertime = 'yes'; cfg.avgoverfreq = 'yes'; cfg.method = 'montecarlo'; cfg.correctm = 'cluster'; cfg.statistic = 'depsamplesT'; cfg.tail = 0; cfg.correcttail = 'prob' cfg.alpha = 0.05; cfg.clusteralpha = 0.05; cfg.numrandomization = 1000; cfg.design = [[1:17 1:17]; [zeros(1,17)+1 zeros(1,17)+2]]; cfg.uvar = 1; cfg.ivar = 2; cfg.layout = '29channel.lay'; stat = ft_freqstatistics(cfg, ga_con, ga_incon); with ga_con = label: {29x1 cell} freq: [1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21] time: [1x401 double] dimord: 'subj_chan_freq_time' powspctrm: [4-D double] cfg: [1x1 struct] ga_incon = label: {29x1 cell} freq: [1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21] time: [1x401 double] dimord: 'subj_chan_freq_time' powspctrm: [4-D double] cfg: [1x1 struct] * Results with fieldtrip-20101010. The electrodes within each cluster are real neighbours within a 29 channel net. >> stat.label(stat.posclusterslabelmat ==1) ans = 'P3' 'P4' 'O2' 'T8' 'P8' 'Pz' 'CP1' 'FC6' 'CP5' 'CP6' >> stat.label(stat.posclusterslabelmat ==2) ans = 'Fp2' 'F3' 'Fz' 'FC1' * Results with fieldtrip-20111012. Fp2 and F3 are actually not neighbours of the othe relectrodes in cluster 1, and Fz is not a neighbour of the other electrodes in cluster 2. >> stat.label(stat.posclusterslabelmat ==1) ans = 'Fp2' 'F3' 'P3' 'Pz' 'FC1' 'CP1' 'FC6' 'CP6' >> stat.label(stat.posclusterslabelmat ==2) ans = 'P4' 'O2' 'T8' 'P8' 'Fz' 'CP5'


Boris Reuderink - 2011-11-17 10:46:51 +0100

Changed the status of bugs without a specific owner to UNCONFIRMED. I'll try to replicate these bugs (potentially involving the submitter), and change confirmed bugs to NEW. Boris


Boris Reuderink - 2011-11-17 11:35:47 +0100

Dear Gregor Volberg, Thank you for this elaborate bug report. The warning that spm8 is added to the path was mainly meant to inform users that that path was changed by FieldTrip. This is done since it masks common MATLAB functions with spm8 variants, if I am not mistaken. Could you provide a file with data you used (ga_con and ga_incon), or alternatively, artificial data that displays the same problem? Best regards, Boris


Boris Reuderink - 2011-11-18 14:23:50 +0100

Sent FTP instructions through email.


Boris Reuderink - 2011-11-18 21:17:34 +0100

I received the files needed for reproduction through FTP.


Boris Reuderink - 2011-11-21 13:12:28 +0100

Created attachment 190 Script to reproduce bug. Data is stored on mentat shared drive.


Boris Reuderink - 2011-11-21 13:14:14 +0100

I have confirmed that the results indeed changed between the two MATLAB versions.


Boris Reuderink - 2011-11-23 11:23:57 +0100

(In reply to comment #6) Correction: Confirmed difference between SVN revision 1896 and the latest revision, not MATLAB versions.


Jörn M. Horschig - 2011-11-24 08:59:08 +0100

Update from the mailinglist: Hi Gregor and Jörn, I find a simliar problem in my data (bti148). 2 sets of channels that share no connections across the sets are included in one cluster. I am not sure, whether spm_bwlabel is the actual problem. However, I do not really understand why calling that function would be necessary. FT calls the function spm_bwlabel during findcluster.m, line 95. (findcluster.m is called by clusterstat.m). As far as I understand, spm_bwlabel is similar to bwlabel.m which searches for connected bins in a matrix. The input to spm_label is the vector 'onoff' (in may case :148x1 logical), which already contains information about neighboring channels. Searching for connected bins in this vector does seem odd to me. However, I supsect something else to be the reason for the wrong clustering, at least in my data. In line 166, ft_freqstatistics.m changes the order of the channel labels: chan = intersect(chan, varargin{i}.label); Original order: {'A68'; 'A58'; 'A148';...} Re-arranged in 'chan': {'A1';'A10';'A100'; 'A101'; 'A102';...} in line 193, ft_freqstatistics calls: cfg.channel = ft_channelselection(cfg.channel, chan); now cfg.channel contains the re-aranged order. On the basis of cfg.channel, the 'channeighbstructmat' is computed by makechanneighbstructmat(cfg) in clusterstat.m. The statistics on the data is computed without re-arranging the channels in statfun_depsamplesT.m ('statobs'). In findcluster.m, line 84 (called during clusterstat.m, l. 197), the matrices 'onoff' and 'selectmat' are multiplied. To my understanding, 'onoff' is based on the statistics that is computed with the originial channel order. 'selectmat' is based on the re-arranged order. If true, the multiplication of these matrices is wrong. Hope this helps, although I might have missed something obvious and might be totally wrong then. best, tobias


Jörn M. Horschig - 2011-11-24 09:00:13 +0100

I thought I'd add the two of you to the discussion, because this is pretty relevant to everyone who had used clusterstatistics the last months(or longer?)


Jörn M. Horschig - 2011-11-28 17:22:31 +0100

It's been the messed up channel order Tobias was talking about. I just still want to find out when this bug was introduced, in order to track back when results got wrong


Jörn M. Horschig - 2011-11-30 10:46:19 +0100

The bug was introduced in revision 3499, that's from May 2011 on


Jörn M. Horschig - 2012-08-23 14:02:13 +0200

bug closing time (http://www.youtube.com/watch?v=xGytDsqkQY8)


Eelke Spaak - 2013-03-20 12:17:50 +0100

This should be discussed in the FT meeting in 45 minutes. It seems the line of code related to chan when i ~= 1 introduced another bug, related to bug 2069.