Back to the main page.
Bug 1078 - all functions that use cfg.elec, cfg.elecfile or get it out out of the data should be consistent
| Status | CLOSED FIXED |
| Reported | 2011-10-26 15:41:00 +0200 |
| Modified | 2012-08-23 14:02:01 +0200 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Mac OS |
| Importance: | P1 normal |
| Assigned to: | Jörn M. Horschig |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Robert Oostenveld - 2011-10-26 15:41:23 +0200
They should all do something like % get the gradiometer or electrode definition if isfield(cfg, 'gradfile') fprintf('reading gradiometers from file ''%s''\n', cfg.gradfile); sens = read_sens(cfg.gradfile); elseif isfield(cfg, 'grad') fprintf('using gradiometers specified in the configuration\n'); sens = cfg.grad; elseif isfield(data, 'grad') fprintf('using gradiometers specified in the data\n'); sens = data.grad; elseif isfield(cfg, 'elecfile') fprintf('reading electrodes from file ''%s''\n', cfg.elecfile); sens = read_sens(cfg.elecfile); elseif isfield(cfg, 'elec') fprintf('using electrodes specified in the configuration\n'); sens = cfg.elec; elseif isfield(data, 'elec') fprintf('using electrodes specified in the data\n'); sens = data.elec; else error('no electrodes or gradiometers specified'); end and they should check for potential confusion of the user in case multiple elecs or grads are available
Robert Oostenveld - 2011-10-26 15:44:53 +0200
I suggest that if there is no confusion, a fprintf is used, and if there is potentially a confusion that a warning is used instead of the fprintf. It is not needed to be specific in the warning. Perhaps something like this would work num = 0; num = num + isfield(cfg, 'gradfile'); num = num + isfield(cfg, 'elecfile'); ... and then if num>1 display = @warning else display = @fprintf; end and then use display('reading gradiometers from file ''%s''\n', cfg.gradfile);
Jörn M. Horschig - 2011-11-09 14:53:45 +0100
prepare_headmodel is doing the same - should I put these if-clauses in a new (private) function? That way, we have one central location at which we can implement new changes, and the handling will be consistent across fieldtrip
Robert Oostenveld - 2011-11-09 17:10:46 +0100
(In reply to comment #2) > prepare_headmodel is doing the same - should I put these if-clauses in a new > (private) function? That way, we have one central location at which we can > implement new changes, and the handling will be consistent across fieldtrip like a elec = get_electrode(cfg, data)? and similar for grad? Sounds like a plan.
Jörn M. Horschig - 2011-11-22 13:45:36 +0100
How about something similar to ft_read_sens just for (cfg, data)? The function would try to retrieve the sens-structure based on if-else, and could then consistently this function to obtain sens in all functions that need one from (cfg, data)? Another remark to this bug: ft_channelrepair is doing strange stuff to obtain sens (if iseeg, sens=data.elec, elseif ismeg, sens=data.grad, else error)
Robert Oostenveld - 2011-11-23 13:45:35 +0100
(In reply to comment #4) Good idea to make an ft_fetch_sens, just like ft_fetch_header and ft_fetch_data. It would be slightly different to those two, because ft_fetch_sens can also get it from the cfg, whereas the other two cannot. Note that this function should not be exposed to regular users.
Jörn M. Horschig - 2011-12-02 10:50:49 +0100
sanity checks: - when input data iseeg, only allow elec definitions - when input data ismeg, only allow grad definitions - when grad and elec are defined in some way simultaneously and it's unclear whether data ismeg or iseeg, throw an error - when grads (or elecs) can be defined in various ways, throw a warning - when the input contains .pnt or .chanpos, consider that it is already a sens (issens, see ft_prepare_neighbours) prioritization: grad-/elecfile > cfg > data > layout > issens function comitted, but not used, yet. Adding ft_fetch_sens.m Transmitting file data . Committed revision 4889.
Jörn M. Horschig - 2012-01-25 12:44:39 +0100
I did a similar thing for headmodel, ft_fetch_vol, see bug 1285 116 $ svn ci -m "bugfix-#1078 #1090- implemented consistent ways to retrieve sens definitions, see ft_fetch_sens" Sending compat/ft_headmodelplot.m Sending compat/ft_prepare_bemmodel.m Sending ft_dipolefitting.m Sending ft_dipolesimulation.m Sending ft_electroderealign.m Sending ft_headmodelplot.m Sending ft_megplanar.m Sending ft_megrealign.m Sending ft_neighbourplot.m Sending ft_prepare_bemmodel.m Sending ft_prepare_headmodel.m Sending ft_prepare_leadfield.m Sending ft_prepare_neighbours.m Sending ft_scalpcurrentdensity.m Sending ft_sensorrealign.m Sending ft_sourceanalysis.m Sending private/prepare_headmodel.m Sending test/test_ft_megplanar.m Sending utilities/ft_fetch_sens.m Transmitting file data ................... Committed revision 5174.
Robert Oostenveld - 2012-07-02 22:53:01 +0200
I have just updated all high-level fieldtrip function documentation. It had gotten unclear to the users hjow to specify electrode positions, gradiometer definitions and volume conduction models. Rather than explaining the options, the documentation pointed to cfg-parsing-helper functions which are in fieldtrip/private and hence not accessible to the end-user. The documentation that is now present for the end-user states % The volume conduction model of the head should be specified as % cfg.vol = structure with volume conduction model, see FT_PREPARE_HEADMODEL % cfg.hdmfile = name of file containing the volume conduction model, see FT_READ_VOL and % The EEG or MEG sensor positions can be present in the data or can be specified as % cfg.elec = structure with electrode positions, see FT_DATATYPE_SENS % cfg.grad = structure with gradiometer definition, see FT_DATATYPE_SENS % cfg.elecfile = name of file containing the electrode positions, see FT_READ_SENS % cfg.gradfile = name of file containing the gradiometer definition, see FT_READ_SENS