Back to the main page.

Bug 3295 - ft_appenddata does not only select the channels present in all data structures anymore

Status CLOSED FIXED
Reported 2017-05-11 14:44:00 +0200
Modified 2019-08-10 12:40:58 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: All
Operating System: All
Importance: P5 critical
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also:

Thomas Hartmann - 2017-05-11 14:44:44 +0200

Created attachment 839 demo script for the bug hi, the previous behavior of ft_appenddata was that if you would append two or more data structures over the rpt dimension and these data structures have different channels, only those channels present in all the data structures and their data would end up in the result. this behavior has changed dramatically leading to inconsistent data structures. in the attached script, i simulate some data with 10 channels. i then create a version of that data with 7 and 6 channels, both taken out of the original data structure. the two new structures only share 3 of their channels. in the next step, ft_appenddata is used to combine the three data structures. i would expect the result to have all trials containing only the info of those 3 channels, common to all the structures. but instead, the label field of the result contains all 10 original channels while the 150 trials contain the full set of channels present in the data structure they originate from. best, thomas


Arjen Stolk - 2017-05-11 17:57:05 +0200

Hi Thomas, working on a new version of ft_appendsens as we speak.


Arjen Stolk - 2017-05-11 20:33:22 +0200

Hi Thomas, Which version of ft are you using? When I used the latest: K>> size(data_few_1.trial{1}) ans = 7 200 K>> size(data_few_1.label) ans = 7 1 and K>> size(data_few_2.trial{1}) ans = 6 200 K>> size(data_few_2.label) ans = 6 1


Thomas Hartmann - 2017-05-12 09:42:28 +0200

(In reply to Arjen Stolk from comment #2) hi arjen, thanks for taking care of this. i am using the latest ft version (commit: c2f875cf6059bf19f202762d6a52248db29f0628). and concerning your comment: the two data_few_X structures are fine. they result from two calls to ft_selectdata which works perfectly. the problem becomes apparent when you look at data_final, created in line 51 using ft_appenddata: >> data_final data_final = struct with fields: label: {10×1 cell} trial: {1×150 cell} time: {1×150 cell} cfg: [1×1 struct] >> size(data_final.trial{1}) ans = 10.0000e+000 200.0000e+000 >> size(data_final.trial{51}) ans = 7.0000e+000 200.0000e+000 >> size(data_final.trial{101}) ans = 6.0000e+000 200.0000e+000 best, thomas


Arjen Stolk - 2017-05-13 02:37:47 +0200

Hi Thomas, I was able to replicate the issue. It seems append_common is doing its work properly, finding the 3 channels that overlap across your 3 datasets: line 123 in ft_appenddata: data = append_common(cfg, dummy{:}); data = label: {3x1 cell} data.label ans = 'chan_04' 'chan_05' 'chan_06' However, at lines 155 to 169, this information is not taken into account. As a result, data_final has 10 channel labels but unequally-sized trial matrices depending on the appended data (e.g. 10 rows, 7 rows, and 6 rows). I'm cc'ing Robert, to see where he thinks the fix should be implemented. That is, either in ft_appenddata or in append_common, in case the issue replicates with timelock and freq data.


Thomas Hartmann - 2017-05-19 09:26:13 +0200

(In reply to Arjen Stolk from comment #4) any updates?


Robert Oostenveld - 2017-05-19 10:36:43 +0200

I thought that it only pertained to the default behavior of the function, but actually this is a blatant bug in ft_appenddata. with ft_appenddata(cfg, data_all, data_few_1, data_few_2); it returns a data structure that is internally not consistent.


Robert Oostenveld - 2017-05-19 11:13:16 +0200

[bug3295-appenddata b7d0183] fixed bug in ft_appenddata when appending data with non-identical but overlapping channels. See http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3295 and thanks to Thomas for reporting. 2 files changed, 117 insertions(+), 26 deletions(-) create mode 100644 test/test_bug3295.m


Robert Oostenveld - 2017-05-19 11:16:45 +0200

I made changes to the code, see https://github.com/fieldtrip/fieldtrip/pull/441


Arjen Stolk - 2017-05-30 01:46:05 +0200

Thanks


Robert Oostenveld - 2019-08-10 12:34:47 +0200

This closes a whole series of bugs that have been resolved (either FIXED/WONTFIX/INVALID) for quite some time. If you disagree, please file a new issue on https://github.com/fieldtrip/fieldtrip/issues.


Robert Oostenveld - 2019-08-10 12:40:58 +0200

This closes a whole series of bugs that have been resolved (either FIXED/WONTFIX/INVALID) for quite some time. If you disagree, please file a new issue on https://github.com/fieldtrip/fieldtrip/issues.