Back to the main page.

Bug 3285 - how to keep information about the montage and/or referencing that is applied to data

Status CLOSED FIXED
Reported 2017-04-20 16:04:00 +0200
Modified 2019-08-10 12:42:13 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P5 normal
Assigned to: Arjen Stolk
URL:
Tags:
Depends on: 3287
Blocks:
See also:

Robert Oostenveld - 2017-04-20 16:04:46 +0200

see https://github.com/fieldtrip/fieldtrip/pull/393 for one of the changes by Arjen I had to make an emergency fix: [master b3f492c] emergency fix - commit d79037f in PR #393 broke quite some of the test scripts. I removed the section from ft_preprocessing where the montage was applied to the sensors. The interaction between the montage, reref and all sensor descriptions that might be present in the data was not clear. This requires a serious reconsideration on how it should work.


Robert Oostenveld - 2017-04-20 16:08:21 +0200

I opt for the following: - if cfg.montage is used, it is possible through cfg.updatesens to update the elec/grad/opto (all three of them). - if cfg.reref is used, cfg.updatesens is not relevant. If we want to make this relevant, then it should be done by making a cfg.montage on the fly. This makes ft_preproc_reref obsolete, and would involve changes in private/preproc, and also relate to cfg.implicitref and cfg.refchannel.


Arjen Stolk - 2017-04-24 04:07:29 +0200

Thanks for fixing this on the fly. Did you already implement those suggestions? I'm asking because all seems to work, i.e. test_pull393.m, with the minor fix proposed in PR 411 on top of your recent changes. One related issue that remains open is that the elec field is not updated with the channels specified in cfg.channel when calling ft_preprocessing. Accordingly, more information is stored in the elec field than necessary, resulting in redundant information (multiple copies of elec fields in one) when appending differently preprocessed iEEG data coming from the same recording (e.g. bipolar deriv on depths + CAR on grids). Also described here: https://github.com/fieldtrip/fieldtrip/pull/393


Arjen Stolk - 2017-04-24 04:09:27 +0200

Test code for both montage-based and standard-way rereferencing: load('SubjectUCI29_data.mat', 'data'); cfg = []; cfg.channel = ft_channelselection({'LPG*', 'LTG*'}, data.label); cfg.reref = 'yes'; cfg.refchannel = 'all'; reref_grids = ft_preprocessing(cfg, data); %FIXME: chansel not applied to elec struc depths = {'RAM*', 'RHH*', 'RTH*', 'ROC*', 'LAM*', 'LHH*', 'LTH*'}; for d = 1:numel(depths) cfg = []; cfg.channel = ft_channelselection(depths{d}, data.label); cfg.montage.labelold = cfg.channel; cfg.montage.labelnew = strcat(cfg.channel(1:end-1),'-',cfg.channel(2:end)); cfg.montage.tra = ... [1 -1 0 0 0 0 0 0 0 1 -1 0 0 0 0 0 0 0 1 -1 0 0 0 0 0 0 0 1 -1 0 0 0 0 0 0 0 1 -1 0 0 0 0 0 0 0 1 -1 0 0 0 0 0 0 0 1 -1]; reref_depths{d} = ft_preprocessing(cfg, data); %FIXME: chansel not applied to elec struc end cfg = []; cfg.appendsens = 'yes'; reref = ft_appenddata(cfg, reref_grids, reref_depths{:});


Robert Oostenveld - 2017-04-24 09:02:30 +0200

ft_preprocessing is working on the data, not on the sensor structure. That has never been the case in the last 15 years and I don't see why that should change. The only modification that is done is that the linear weighting of the channels is updated, such that the forward model (with ft_compute_leadfield) matches the data. The selection of channels for the forward model is made at the time when needed, i.e. in private/prepare_headmodel and ft_prepare_vol_sens.


Robert Oostenveld - 2017-04-24 11:24:34 +0200

mac011> git commit test/test_bug3285.m [bug3287-append 42131f1] added test script that was provided by Arjen in http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3285 1 file changed, 71 insertions(+) create mode 100644 test/test_bug3285.m note that this is in my personal bug3287-append branch, not yet in master.


Robert Oostenveld - 2017-04-25 14:55:12 +0200

appending of channel level data has been reimplemented. It is not clear to me why certain processing steps (such as montaging) need to be represented in the data. Principle 1: all processing is contained in the user's script, and in data.cfg.previous. The data itself does not describe what happened to it. Principle 2: the data should be self-explanatory and internally consistent.


Arjen Stolk - 2017-04-25 22:16:14 +0200

For completion, the issue in numbers: >> data.elec ans = chanpos: [152x3 double] chantype: {152x1 cell} chanunit: {152x1 cell} coordsys: 'tal' elecpos: [152x3 double] label: {152x1 cell} unit: 'mm' cfg: [1x1 struct] >> reref.elec ans = chanpos: [1209x3 double] chanposold: [1216x3 double] chantype: {1209x1 cell} chanunit: {1209x1 cell} coordsys: 'tal' elecpos: [1216x3 double] label: {1209x1 cell} labelold: {1216x1 cell} unit: 'mm' cfg: [1x1 struct] Violates the self-consistency aspect of principle 2. It's actually not going to break anything btw. There's just a lot of redundant and old electrode information in reref.elec, such as 'RAM1' (which was superseded by 'RAM1-RAM2', etc). It'd be cleaner if reref.elec had the same 145 channels as reref itself: reref = label: {145x1 cell} sampleinfo: [26x2 double] elec: [1x1 struct] trial: {1x26 cell} time: {1x26 cell} cfg: [1x1 struct]


Arjen Stolk - 2017-04-26 02:16:10 +0200

(In reply to Arjen Stolk from comment #7) Actually have to take back that duplicate elecs in an elec structure won't break anything. Generating the layout, using Roemer's functionality, will cause lay.pos to have many overlapping electrodes. This, on its turn, crashes ft_multiplotTFR: Warning: (one of the) axis is/are not evenly spaced, but plots are made as if axis are linear > In ft_multiplotTFR (line 414) Error using uimage (line 60) The X and Y axis should be increasing. This means 'smart elec merging' is needed. But it cannot be done in ft_appendsens, but because ft_appendsens does not know anything about the functional data.


Arjen Stolk - 2017-04-26 02:32:41 +0200

Calling ft_preprocessing with cfg.montage does indeed replace old with new channels, e.g. in this case RAM1, RAM2 become RAM1-RAM2. Yet, the non-selected channels are also included. Not saying this is necessarily undesired, just that it lays the groundwork for the problem of duplicate and outdated channels later on after ft_appendsens. >> reref_depths{1}.elec.label ans = 'RAM1-RAM2' 'RAM2-RAM3' 'RAM3-RAM4' 'RAM4-RAM5' 'RAM5-RAM6' 'RAM6-RAM7' 'RAM7-RAM8' 'RTH1' 'RTH2' 'RTH3' 'RTH4' 'RTH5' 'RTH6' 'RTH7' 'RTH8' 'RHH1' 'RHH2' 'RHH3' 'RHH4' 'RHH5' 'RHH6' 'RHH7' 'RHH8' 'ROC1' 'ROC2' 'ROC3' 'ROC4' 'ROC5' 'ROC6' 'ROC7' 'ROC8' 'LAM1' 'LAM2' 'LAM3' 'LAM4' 'LAM5' 'LAM6' 'LAM7' 'LAM8' 'LTH1' 'LTH2' 'LTH3' 'LTH4' 'LTH5' 'LTH6' 'LTH7' 'LTH8' 'LHH1' 'LHH2' 'LHH3' 'LHH4' 'LHH5' 'LHH6' 'LHH7' 'LHH8' 'LPG1' 'LPG2' 'LPG3' 'LPG4' 'LPG5' 'LPG6' 'LPG7' 'LPG8' 'LPG9' 'LPG10' 'LPG11' 'LPG12' 'LPG13' 'LPG14' 'LPG15' 'LPG16' 'LPG17' 'LPG18' 'LPG19' 'LPG20' 'LPG21' 'LPG22' 'LPG23' 'LPG24' 'LPG25' 'LPG26' 'LPG27' 'LPG28' 'LPG29' 'LPG30' 'LPG31' 'LPG32' 'LPG33' 'LPG34' 'LPG35' 'LPG36' 'LPG37' 'LPG38' 'LPG39' 'LPG40' 'LPG41' 'LPG42' 'LPG43' 'LPG44' 'LPG45' 'LPG46' 'LPG47' 'LPG48' 'LPG49' 'LPG50' 'LPG51' 'LPG52' 'LPG53' 'LPG54' 'LPG55' 'LPG56' 'LPG57' 'LPG58' 'LPG59' 'LPG60' 'LPG61' 'LPG62' 'LPG63' 'LPG64' 'LTG1' 'LTG2' 'LTG3' 'LTG4' 'LTG5' 'LTG6' 'LTG7' 'LTG8' 'LTG9' 'LTG10' 'LTG11' 'LTG12' 'LTG13' 'LTG14' 'LTG15' 'LTG16' 'LTG17' 'LTG18' 'LTG19' 'LTG20' 'LTG21' 'LTG22' 'LTG23' 'LTG24' 'LTG25' 'LTG26' 'LTG27' 'LTG28' 'LTG29' 'LTG30' 'LTG31' 'LTG32'


Arjen Stolk - 2017-04-26 23:54:37 +0200

https://github.com/fieldtrip/fieldtrip/pull/415 copp-paste: ft_preprocessing creates a unit matrix on the fly in case cfg.reref = 'yes' (and in case there's a sens) ft_apply_montage keepunused = 'no' still had too many elecpos. the function is pretty oldskool and the most reliable way to ensure that keepunused = 'no' didn't still attach unused stuff was by using a call to ft_selectdata on the input right in the beginning (rather than sifting out fields at the end) test_preprocmontage test the different rereferencing options on data with and without a sens unclear: should ft_preprocessing, when it creates the unit matrix (in case cfg.reref) , also take into account cfg.implicitref if existent? physically, there's no elecpos to assign to it, but there may be a chanpos hypothetically (although no idea where to put it)


Arjen Stolk - 2017-05-13 09:17:52 +0200

This had been addressed in PR 415, 434, 435


Arjen Stolk - 2017-05-13 09:18:03 +0200

This had been addressed in PR 415, 434, 435


Robert Oostenveld - 2019-08-10 12:35:57 +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:42:13 +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.