Back to the main page.
Bug 2424 - ft_prepare_layout from sens rather than from elec/grad
| Status | CLOSED FIXED |
| Reported | 2013-12-20 10:53:00 +0100 |
| Modified | 2014-03-12 12:19:56 +0100 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P5 enhancement |
| Assigned to: | Jörn M. Horschig |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | 2314 |
| See also: |
Jörn M. Horschig - 2013-12-20 10:53:58 +0100
The component viewmode of the databrowser requires a layout to topographically plot the components. If a layout is not available, the function features a layout construction based on the elec- or grad-information and call ft_prepare_layout. However, there are a number of inconsistencies with other code in the databrowser code, and I think this can be improved by discussing ft_prepare_layout. First, I will paste the relevant databrowser code, and then I will indicate what might/could/should be done better together with things to discuss: warning('No layout specified - will try to construct one using sensor positions'); try tmpcfg = []; tmpcfg.layout = []; if isfield(data, 'grad') tmpcfg.grad = data.grad; elseif isfield(data, 'elec') tmpcfg.elec = data.elec; else error('cannot infer sensor type'); end cfg.layout = ft_prepare_layout(tmpcfg); catch cfg.layout = []; end % try First of all, the error will never be printed, as it is caught - I guess such things can happen ;) Most importantly, however, grad- and elec-structures can also be defined in a cfg, and elecfile and gradfile should also be sensible fields to be checked. There is an ft_fetch_sens function which should be used instead of having the if-statement. However, the drawback here is that ft_prepare_layout explicitly needs .grad .elec, .elecfile or .gradfile but cannot handle .sens. In the light of recent MEG/EEG studies it makes sense to make the distinction between elec and grad, but this is a whole other topic (and one that will cause a lot of headache in future...). As ft_prepare_layout does not dinstinguish between electrodes and gradiometers, would it make sense that ft_prepare_layout takes a .sens field rather than making the distinction between .elec and .grad? I cannot believe that it will ever make sense to mix electrodes and gradiometers in one single layout, so calling it .sens makes a lot of sens (haha!) to me.
Robert Oostenveld - 2013-12-20 11:09:13 +0100
(In reply to Jörn M. Horschig from comment #0) I agree that ft_fetch_sens should be used. And that the error makes no sense like this. However, we never have a field that is called ".sens" as a mixed eeg/meg sensor definition is not possible in a single sensor description. It is possible that data.grad and data.elec both exist, in that case one is picked arbitrarily. But I am also not sure whether I understand your statement on sens... Regarding where ft_fetch_sens gets called: would it not make sense (not sens) to do it like if hasdata cfg.layout = ft_prepare_layout(cfg, data) else cfg.layout = ft_prepare_layout(cfg) end or perhaps more safe tmpcfg = [] try, tmpcfg.elec = cfg.elec; end try, tmpcfg.grad = cfg.grad; end try, tmpcfg.elecfile = cfg.elecfile; end try, tmpcfg.gradfile = cfg.gradfile; end if hasdata cfg.layout = ft_prepare_layout(tmpcfg, data) else cfg.layout = ft_prepare_layout(tmpcfg) end to ensure that ft_prepare_layout does not mess with the other cfg options. inside ft_prepare_layout there is still no call to ft_fetch_sens, but there is the whole sequence of if-elseif-elseif for all possible cases.
Jörn M. Horschig - 2013-12-20 11:51:24 +0100
(In reply to Robert Oostenveld from comment #1) yes, I agree that we never had a .sens field, but the case of mixed EEG/MEG is exactly my point. Using a .sens field would also make very explicit that you can *not* create a layout for mixed MEG/EEG data but have to choose. Additionally, ft_prepare_layout does not need to know whether we are dealing with electrodes or with gradiometers. it just needs to be concerned with sensors and their positions. Right now, it might seem that you can put in both and get a mixed layout out. The statement that one is 'arbitrarily' picked makes things even worse, imho. For fixing the databrowser, I like the second code idea, I'll change that. And maybe ft_prepare_layout should then also be changed to use ft_fetch_sens
Robert Oostenveld - 2013-12-20 14:02:28 +0100
(In reply to Jörn M. Horschig from comment #2) you can hand-craft a mixed layout, but cannot have it made automatically. The 'arbitrary' refers to: if _you_ (i.e. user) want _me_ (i.e. ft_prepare_layout) to do something automatically without telling me what to do, I'll do my best, but you should not complain.
Jan-Mathijs Schoffelen - 2014-03-07 13:06:49 +0100
Assign to a named person, to keep it alive (and make someone feel responsible to fix it).