Back to the main page.

Bug 2148 - fixdimord changes dimord chancmb to chan though labelcmb is there

Status CLOSED FIXED
Reported 2013-04-29 15:12:00 +0200
Modified 2014-03-12 12:20:43 +0100
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P3 normal
Assigned to: Jörn M. Horschig
URL:
Tags:
Depends on: 2197
Blocks:
See also:

Jörn M. Horschig - 2013-04-29 15:12:46 +0200

my data: data = labelcmb: {104x2 cell} dimord: 'chancmb_freq' wpli_debiasedspctrm: [104x60 double] freq: [1x60 double] cfg: [1x1 struct] I thought I had to fix the dimord already after ft_connectivityanalysis, cause it was 'chan_freq' although 'labelcmb' was present, but ft_checkdata (ie fixdimord) always wants to revert to 'chan'. The only way to make e.g. ft_connectivitplot work atm is to use: ft_checkdata(data, 'cmbrepresentation', 'full'); This will make dimord chan_chan_freq and convert labelcmb to label. I couldn't find good documentation on chancmb on the wiki, so I am not 100% sure about this (whereas I am definitely sure that currently it's nonsense and output of ft_connectivityanalysis is incompatible with ft_connectivityplot) btw: same holds e.g. for granger as well, so it's not specific to wpli


Jan-Mathijs Schoffelen - 2013-04-29 15:17:31 +0200

as far as I know ft_connectivityplot only works if the bivariate data has a dimord of chan_chan_freq


Jörn M. Horschig - 2013-04-29 15:52:40 +0200

then the error message in ft_connectivityplot should be fixed: Error using ft_connectivityplot (line 99) the data should have a dimord of chan_chan_freq or chancmb_freq


Jan-Mathijs Schoffelen - 2013-04-29 16:34:24 +0200

Ow, first we should probably check whether my claim was true (as in only support for chan_chan_xxx)


Jörn M. Horschig - 2013-04-29 16:58:36 +0200

good plan :)


Jörn M. Horschig - 2013-05-02 11:02:40 +0200

in ft_connectivityplot: if strcmp(data.dimord, 'chan_chan_freq') % that's ok elseif strcmp(data.dimord, 'chancmb_freq') % convert into 'chan_chan_freq' data = ft_checkdata(data, 'cmbrepresentation', 'full'); else error('the data should have a dimord of %s or %s', 'chan_chan_freq', 'chancmb_freq'); end However, the problem maintains that chancmb_freq gets changed to chan_freq, thus the chancmb_freq to chan_chan_freq conversion is never executed


Jan-Mathijs Schoffelen - 2013-05-09 08:15:21 +0200

can you upload the data please?


Jörn M. Horschig - 2013-05-13 11:07:13 +0200

cfg = []; cfg.ntrials = 500; cfg.triallength = 1; cfg.fsample = 200; cfg.nsignal = 3; cfg.method = 'ar'; cfg.params(:,:,1) = [ 0.8 0 0; 0 0.9 0.5; 0.4 0 0.5]; cfg.params(:,:,2) = [-0.5 0 0; 0 -0.8 0; 0 0 -0.2]; cfg.noisecov = [0.3 0 0; 0 1 0; 0 0 0.2]; data = ft_connectivitysimulation(cfg); % freqanalysis cfgf = []; cfgf.method = 'mtmfft'; cfgf.output = 'fourier'; cfgf.tapsmofrq = 2; freq = ft_freqanalysis(cfgf, data); % connectivityanalysis cfgc = []; cfgc.channelcmb = {freq.label{1} freq.label{2}; freq.label{1} freq.label{3}}; cfgc.method = 'coh'; c1 = ft_connectivityanalysis(cfgc, freq);


Jörn M. Horschig - 2013-05-13 11:08:06 +0200

K>> c1 c1 = labelcmb: {2x2 cell} dimord: 'chan_freq' cohspctrm: [2x101 double] freq: [1x101 double] dof: 1500 cfg: [1x1 struct] K>> ft_connectivityplot([], c1) ??? Error using ==> ft_connectivityplot at 99 the data should have a dimord of chan_chan_freq or chancmb_freq


Jörn M. Horschig - 2013-06-05 10:53:01 +0200

please tell me if I am wrong with any of this: fix 1: ft_checkdata(data, 'cmbrepresentation', 'sparse') currently returns data.dimord = 'chan_XXX', but it should be 'chancmb_XXX'. new problem 1: dimord 'chancmb' is not supported by ft_selectdata_old which is called subsequently by ft_connectivityanalysis: data = ft_selectdata(data, 'avgoverrpt', 'yes');


Jörn M. Horschig - 2013-06-12 17:20:01 +0200

fixed http://code.google.com/p/fieldtrip/source/detail?r=8250 http://code.google.com/p/fieldtrip/source/detail?r=8252 but see bug 2197


Jan-Mathijs Schoffelen - 2013-06-13 21:05:16 +0200

In ft_sourceanalysis an explicit check is done on the dimord of the input frequency domain data. 'chancmb_freq' is not one of them, causing a crash...


Jan-Mathijs Schoffelen - 2013-06-13 21:11:36 +0200

prepare_freq_matrices also does not work anymore


Jan-Mathijs Schoffelen - 2013-06-13 21:32:44 +0200

updated ft_sourceanalysis and private/prepare_freq_matrices to support chancmb_XXX in dimord. revision 8261 Please check whether other functions are affected.


Jörn M. Horschig - 2013-06-14 08:38:41 +0200

what structure did you put into ft_sourceanalysis? When I compute a freq-structure with cfg.output = 'powandcsd' I get a freq.label and a freq.labelcmb field, if I use cfg.output='fourier' I only get a .label field. Both worked fine for me. So I wonder what magic you are doing with FieldTrip that I don't know, yet :)


Jan-Mathijs Schoffelen - 2013-06-14 09:11:15 +0200

it's not about how the numeric data are represented, it's about the dimord. There was an explicit check w.r.t it, e.g. if strcmp(freq.dimord, 'chan_freq') elseif strcmp( etcetc 'chancmb_freq' was not one of them.


Jörn M. Horschig - 2013-06-14 09:20:35 +0200

I never get a chamcmb dimord, that's what I meant


Jan-Mathijs Schoffelen - 2013-06-14 11:01:29 +0200

I do: tmp = ft_checkdata(freq, 'cmbrepresentation', 'fullfast'); tmp = ft_checkdata(tmp, 'cmbrepresentation', 'sparse'); tmp = ft_checkdata(tmp, 'cmbrepresentation', 'sparsewithpow'); ft_sourceanalysis(cfg, tmp);


Jörn M. Horschig - 2013-06-14 11:07:21 +0200

huh, strange magic you are doing I'll extend the test script of this bug with these things and will test a bunch of functions that take freq as input


Jörn M. Horschig - 2013-06-25 09:04:57 +0200

I found: Warning: unsupported dimord "chancmb_freq_time" for interactive plotting > In trunk/private/topoplot_common at 835 and therefore propose to transform to chan_chan inside topoplot_common. Tomorrow, I'll look into more of these kind of things


Jörn M. Horschig - 2013-07-03 10:30:29 +0200

is there a reason why chan_chan is not supported in ft_sourceanalysis but chancmb_ is?


Jan-Mathijs Schoffelen - 2013-07-13 21:12:06 +0200

it's all about history. chan_chan would actually be the more natural way to format the input. It all boils down to update prepare_freq_matrices with my own local copy (which performs exactly the same, but which is based on a sequence of ft_selectdata calls). Now I speak of it, I am not sure whether the chan_chan is explicitly supported, but the expected behavior would be to just do some bookkeeping on the data and leave it at that.


Jörn M. Horschig - 2013-07-16 09:17:51 +0200

(In reply to comment #21) "but the expected behavior would be to just do some bookkeeping on the data and leave it at that." I don't get that, what bookkeeping? Leave it at what?


Jan-Mathijs Schoffelen - 2013-07-16 09:23:41 +0200

sorry for being a bit cryptic. the output of the function prepare_freq_matrices is a square cross-spectral density matrix (plus some additional stuff if one wants to do 'classical' dics), which has a dimord of chan_chan. If the input contains a chan_chan_freq_time matrix, the only thing that needs to be done is the selection of the frequency and time of interest (if there's anything to choose) and the selection of channels that are not present in the list of required channels (e.g. remove EOG channels if present)


Jörn M. Horschig - 2013-07-16 10:00:57 +0200

ah okidoki. and what about your own local copy of prepare_freq_matrices, would it make sense to share it or should I pimp the function accordingly?


Jan-Mathijs Schoffelen - 2013-07-16 10:39:28 +0200

Created attachment 495 JM's version of prepare_freq_matrices


Jan-Mathijs Schoffelen - 2013-07-16 10:40:10 +0200

looks pretty old, and I am not sure whether it works fully ok. sounds like a testscript is necessary ;-).


Jörn M. Horschig - 2013-07-16 12:43:10 +0200

hm, by looking at it I get the impression that that is a really old version, isn't it? Does it work? But I am super confused by the current version as well... it needs both crsspctrm and powspctrm for constructing Cf, why is crsspctrm alone not sufficient? Can I replace this: % put the power on the diagonal Cf(find(eye(Nchans))) = freq.powspctrm(powspctrmindx, fbin); By the respective lines from your version? Maybe it might be best if we can have a look together tomorrow (or any other day)?


Jörn M. Horschig - 2013-07-16 13:48:04 +0200

hm, after some thorough thinking I decided to keep the current version if the freq-structure has crsspctrm and powscptrm and take your version if the freq-structure just has crsspctrm but no powspctrm. Additionally, there is a third case for fourierspctrm. this is definitely the safest option for now, but I'm super confused why this distinction needs to be made in the first place... svn ci private/prepare_freq_matrices.m utilities/ft_checkdata.m ft_sourceanalysis.m test/test_bug2148.m -m "bugfix #2148 - extended support for chancmb/laelcmb" Sending ft_sourceanalysis.m Sending private/prepare_freq_matrices.m Sending test/test_bug2148.m Sending utilities/ft_checkdata.m Transmitting file data .... Committed revision 8319.


Jörn M. Horschig - 2013-07-24 12:04:57 +0200

as discussed on monday, we keep it at that for the time being