Back to the main page.

Bug 637 - ft_appenddata should be extended to also support appending timelock, freq and source data

Status CLOSED FIXED
Reported 2011-05-04 13:36:00 +0200
Modified 2013-10-26 18:15:56 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P1 enhancement
Assigned to: Jan-Mathijs Schoffelen
URL:
Tags:
Depends on:
Blocks: 1021
See also:

Robert Oostenveld - 2011-05-04 13:36:31 +0200

... and it should work for trials and channels, just like now for raw data


Jan-Mathijs Schoffelen - 2011-05-06 21:47:10 +0200

Please use the functionality in ft_selectdata for this. As it stands at the moment, ft_selectdata is doing the following: -appending data (raw, timelock, freq, source), not only in the trial dimension but also in the channel or freq (and possibly even time) dimension. -selecting trials,channels,frequencies,timepoints, roi? -averaging across channels, frequencies, timepoints. Should we split the function in 3 parts?


Robert Oostenveld - 2011-05-09 09:31:30 +0200

interesting... Users currently know of ft_appenddata for raw data. The name ft_selectdata does not suggest that one can append with it, nor that one can average with it. The three functionalities are implementation-efficiency wise logical to have in one function, but towards the end user combining the three does not make sense. Should we go for ft_appenddata ft_appendtimelock(cfg, timelock1, timelock2, ...) ft_appendfreq ft_appendsource and ft_averagetimelock(cfg, timelock1, timelock2, ...) ft_averagefreq ft_averagesource The underlying functionality can remain in ft_selectdata, the functions above would act as entry points for the end-user. What do you think?


Jan-Mathijs Schoffelen - 2011-05-09 09:36:36 +0200

good plan.


Eelke Spaak - 2011-05-11 09:28:46 +0200

I tried using ft_selectdata to append timelock and freq structures, but ran into several bugs. Fixing one quickly led to another, and another..., so before I destroy (or fix :) ) the entire ft_selectdata function, maybe we should look at it together, JM?


Jan-Mathijs Schoffelen - 2011-05-11 10:31:48 +0200

yes, please


Jan-Mathijs Schoffelen - 2011-05-11 10:35:57 +0200

i should add that the appenddata functionality in ft_selectdata is relatively crude and certainly need improvement!


Robert Oostenveld - 2011-05-11 16:03:10 +0200

A first implementation is finished, but not yet tested. Could any of you try it out? One aspect that requires careful consideration is dealing with sampleinfo and trialinfo: at the moment ft_appenddata explicitely deals with it, whereas I would expect for the three new functions that ft_selectdata takes care of it. manzana> svn commit ft_append*m Sending ft_appenddata.m Adding ft_appendfreq.m Adding ft_appendsource.m Adding ft_appendtimelock.m Transmitting file data .... Committed revision 3519.


Jan-Mathijs Schoffelen - 2011-05-15 20:39:55 +0200

tested for freq and timelock data using test data on home common (ctf151). did not yet check trialinfo handling etc.


Johanna - 2011-05-17 13:05:40 +0200

I tried ft_appendsource on some source data computed a few months ago: LCMV source data which were two different conditions, so the output is now 'rpt_pos' with rpt = 2, and dim is [2 27 22 22]. Had to modify call to ft_selectdata to include (varargin{:},'param',{'pow'}) (committed) Perhaps this should be a cfg option into ft_appendsource (i.e. which param to use)? since ft_selectdata says that it can only operate on one dimension at a time for source data. and later (line 76) Ndata does not exist, so I replaced with numel(varargin) (committed) Could it be possible to allow for varargin to either be a list ft_appendsource(cfg,source1,source2,..) % as it currently is OR ft_appendsource(cfg,source) % where source is array of source{1}, source{2}, etc?


Robert Oostenveld - 2011-05-17 13:47:34 +0200

> Could it be possible to allow for varargin to either be a list > ft_appendsource(cfg,source1,source2,..) % as it currently is > OR > ft_appendsource(cfg,source) % where source is array of source{1}, source{2}, > etc? it should be specified as the 1st option, not as the second. Inside ft_appendsource you want varargin{1} to be a struct and not a cell-array with structs.


Arjen Stolk - 2011-09-01 15:10:24 +0200

ft_appendtimelock does not yet work properly (or at all). I've tried to append two timelock structures each containing all trials (ft_timelockanalysis with keeptrials = yes). First it tries to check the data with ft_checkdata at line 59 and option 'hassampleinfo' set to 'ifmakessense'. This throws an error at the low level in the istrue function; 'could not determine whether "ifmakessense" should be interpreted as true or false' Removing this hassampleinfo check, throws another error at ft_selectdata (line 63 of ft_appendtimelock), where it tries to refer to the non-existent field 'avg'. Naturally, this is not present in a timelock structure. I'm not sure why this is needed in the first place. I think both these problems need fixing for ft_appendtimelock to work.


Arjen Stolk - 2011-09-01 16:23:13 +0200

(In reply to comment #11) > ft_appendtimelock does not yet work properly (or at all). > I've tried to append two timelock structures each containing all trials > (ft_timelockanalysis with keeptrials = yes). > First it tries to check the data with ft_checkdata at line 59 and option > 'hassampleinfo' set to 'ifmakessense'. This throws an error at the low level in > the istrue function; 'could not determine whether "ifmakessense" should be > interpreted as true or false' > Removing this hassampleinfo check, throws another error at ft_selectdata (line > 63 of ft_appendtimelock), where it tries to refer to the non-existent field > 'avg'. Naturally, this is not present in a timelock structure. I'm not sure why > this is needed in the first place. > I think both these problems need fixing for ft_appendtimelock to work. Okay, I figured out what complicated factor was causing all this trouble. Turned out I was trying to appendtimelock preproccessed data instead of timelocked data.... sorry... ft_appendtimelock works fine.


Jan-Mathijs Schoffelen - 2013-10-09 16:34:13 +0200

The functions ft_appendtimelock, ft_appendfreq and ft_appendsource (?) have been created. I suggest to close this bug, and create new bugs pertaining to buggy functionality of the specific functions, if needed.