Back to the main page.

Bug 656 - ft_timelockanalysis: output should have a dimord consistent with its fields

Status ASSIGNED
Reported 2011-05-11 15:25:00 +0200
Modified 2017-09-15 10:03:58 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P1 enhancement
Assigned to: Johanna
URL:
Tags:
Depends on:
Blocks:
See also: http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3346

Johanna - 2011-05-11 15:25:38 +0200

Based on discussion at the FT meeting today and with JM after... Suggestion for ft_timelockanalysis: cfg.keeptrials: either 'yes' or 'no' cfg.output: either 'cov' or 'trials' timelock output will then contain only one of these: (a) trials (dimord 'rpt_chan_time') (b) avg (dimord 'chan_time') (c) covariance unaveraged (dimord 'rpt_chan_chan') (d) covariance averaged (dimord 'chan_chan') (a) and (b) would still have .time field. (c) and (d) may have a 'toilim' field instead (and no .time field) which may specify start/end points of covariance time window Does this require creation of a new type of data, that is either of dimord 'chan_chan' or 'rpt_chan_chan'? This general data type could consist of anything: timelock cov, freq csd, or any other connectivity metric supported/created in ft_connectivityanalysis? Or would ft_datatype_timelock just allow these four different types as valid structure?


Johanna - 2011-06-29 12:11:30 +0200

I'm working on a new implementation of ft_timelockanalysis, which is fixing the confusion of 'latency' with 'covariancewindow' meaning different things, especially in the context of the vartrllength in bug 624, as well as addressing the main suggestion of this bug. I will ask for the team to look it over before committing of course.


Johanna - 2011-06-29 16:45:44 +0200

I've finished a new version, added as attachment to this bug. One question came up: in the case of cfg.output='cov', and cfg.vartrllength=2 and cfg.keeptrials='no', then should the mean covariance take into account a weighting based on the possibility of unequal trial lengths? For example, instead of the current: covsig = squeeze(nansum(covsig, 1)) / sum(numcovsigsamples); use instead: covsig = squeeze(nansum(covsig.*repmat((numcovsigsmaples/max(numcovsigsamples))',[1 size(covsig,2) size(covsig,3)]), 1)) / sum(numcovsigsamples); (and similarly for if cfg.removemean='yes'). Is this valid to do this weighting? The bigger-picture question is if this version is ok, then it affects all other functions which take in a timelock structure and whether they want a 'cov' or 'avg' or 'trial' field.


Johanna - 2011-06-29 16:46:24 +0200

Created attachment 83 ft_timelockanalysis_new


Johanna - 2011-07-06 15:23:53 +0200

From discussion today in FT meeting and with JM, todo: 1) error if cfg.latency isfield (checkconfig) 2) use instead cfg.avglatency and cfg.covlatency 3) if neither of (2) is specified, use maxperlength 4) to be consistent with freqanalysis, cfg.output can be any of 'avg', 'cov' or 'avgandcov'. 5) new option for sliding timewindow covariance compuatation: call recursively if output='cov', and if cfg.toi and cfg.timwin specified. However, toi and timwin not allowed if output is 'avg' or 'avgandcov'. If used, then output structure has a .time field equal to .toi. 6) make sure vartrllength=0 crashes only after check if required latency is present for all trials (maybe already there) 7) make sure that, for cov computation, if a trial does not have all time points within covlatency, that it is not used, rather than using partial trials. Could be done, but then keep track of number of time points used 8) new output field of .usetrial 9) if trialinfo isfield, then save trialinfo(usetrial)


Robert Oostenveld - 2011-07-06 15:57:26 +0200

(In reply to comment #4) > 8) new output field of .usetrial I don't understand why you'd want this. We also don't have it for all other functions that have cfg.trials as selection. For interpreting the average, you don't have to know which trials went into its computation. And if you want to know what was done to get the results that were represented in the output structure, you can look in timelock.cfg and previous.... This seems the same as the discussion we had this afternoon on whether we should store the covlatency window in the output (which we also don't do). > 9) if trialinfo isfield, then save trialinfo(usetrial) but only if keeptrials=yes.


Johanna - 2011-07-20 16:04:11 +0200

(In reply to comment #4) > 1) error if cfg.latency isfield (checkconfig) In further discussion (for sake of consistency across all functions to have a .latency input) the names will now be cfg.latency and cfg.covlatency > 3) if neither of (2) is specified, use maxperlength maxperlength will be for .latency but minperlength for .covlatency [Since point (7) says to ignore trials for cov-computation that don't have all time points in the window. This is consistent with current implementation where a trial is thrown out for covariance computation if does not have all points inside specified time window.] > 6) make sure vartrllength=0 crashes only after check if required latency is > present for all trials (maybe already there) deprecate vartrllength. for 'avg', behaviour will be like vartrllength=2, for 'cov', it will be like vartrllength=1. > 8) new output field of .usetrial agree with Robert's comment: timelock.cfg.trials is sufficient > 9) if trialinfo isfield, then save trialinfo(usetrial) agree with Robert's comment: only if keeptrials='yes'


Johanna - 2011-07-21 15:22:57 +0200

Created attachment 105 ft_timelockanalysis_new


Johanna - 2011-07-21 15:23:41 +0200

Created attachment 106 test_ft_timelockanalysis.m


Johanna - 2011-07-21 15:25:55 +0200

Attached is the new version and test_* script. What should be the next step? Have the development team test it out for awhile first? commit it to SVN as *_new.m for now?