Back to the main page.

Bug 2705 - ft_rejectvisual does not keep channels with cfg.keepchannel='yes'

Status CLOSED FIXED
Reported 2014-09-25 12:35:00 +0200
Modified 2015-02-11 10:40:25 +0100
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P5 normal
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also:

Jim Herring - 2014-09-25 12:35:44 +0200

When ft_rejectvisual is called using cfg.keepchannel = 'yes' and cfg.channel is not 'all', it is supposed to keep all channels but only use those channels that were selected for plotting. Unfortunately, only those channels that were selected are kept and the others removed.


Jim Herring - 2014-09-25 13:47:32 +0200

After a discussion with Stephen we agreed that cfg.channel is to indicate which channels are used in the summary view. All channels in the input should be available in the output. At this point cfg.keepchannel is used in the code to decide what to do with the channels that are marked as bad. If we wish this option to reflect what to do with channels that are not selected by cfg.channel, then we need to add another function that reflects what to do with marked channels as two decisions need to be made: what to do with unselected input channels and what to do with channels marked as 'bad'.


Jim Herring - 2014-09-26 13:17:36 +0200

I've changed the code so that cfg.keepchannel pertains to the input channels. I've added cfg.reject to specify what to do with the channels and trials marked for rejection (keep/remove/fill with nans). I've also updated the documentation. The behaviour of the function has not changed if the default configuration options are used. By default, input channels not selected with cfg.channel are not kept. Sending ft_rejectvisual.m Transmitting file data . Committed revision 9845.


Robert Oostenveld - 2014-09-27 11:33:24 +0200

I reviewed https://code.google.com/p/fieldtrip/source/diff?spec=svn9845&r=9845&format=side&path=/trunk/ft_rejectvisual.m but don't understand why the solution needs to be so difficult and requires another cfg option (cfg.reject). While working on the plane I also had to solve this, and this was enough mac011> diff ft_rejectvisual.m.mine ft_rejectvisual.m.r9840 166,167c166,168 < orgcfg = cfg; < tmpcfg = keepfields(cfg, {'trials'}); --- > orgcfg.latency = cfg.latency; > tmpcfg = []; > tmpcfg = keepfields(cfg, {'trials','channel'}); 171,175c172 < cfg = copyfields(orgcfg, cfg, {'channel', 'latency'}); < < % restore the original latency, it should not be 'all' < % restore the original channel selection, it is dealt with below < cfg.channel = orgcfg.channel; --- > cfg.latency = orgcfg.latency;% restore the original latency, it should not be 'all' The support was already there, it was just that the call to ft_selectdata made the channels disappear. I guess that ft_selectdata was added without thinking about the consequence here (in this function cfg.channel had a different meaning than in general) I have reverted to the original and used my solution instead. mac011> svn commit ft_rejectvisual.m Sending ft_rejectvisual.m Transmitting file data . Committed revision 9847. Jim and Stephen: please test whether my code works as you would expect. I looked at the preprocessing tutorial and there did not see cfg.reject being used.


Jim Herring - 2014-09-28 19:43:40 +0200

Hi Robert, The reason I added cfg.reject is because cfg.keepchannel also decided what would happen with channels that you actually want to remove (line 326 onwards). In your current implementation, if you have cfg.keepchannel ='yes', you cannot remove channels anymore, even if you mark them for rejection in the summary view. This is because input channels that are not selected with cfg.channel are treated as channels that are marked in the summary view. I do not believe this is what we want. Naturally I thought it should be possible to keep the input channels that were not specified with cfg.channel and still be able to reject channels. I therefore implemented cfg.reject to deal with the marked channels and have cfg.keepchannel only handle the input channels.


Robert Oostenveld - 2014-09-28 23:19:46 +0200

as discussed with Stephen on skype: ft_selectdata should be done at th eend of the function, cfg.keeptrials should be added, cfg.trials/cfg.channel and cfg.keeptrials/keepchannels should behave consistently I will implement this, but only after the natmeg workshop


Robert Oostenveld - 2014-10-07 11:13:09 +0200

this has been addressed in the following commits: mac011> svn commit private/rejectvisual_*.m ft_rejectvisual.m Sending ft_rejectvisual.m Sending private/rejectvisual_channel.m Sending private/rejectvisual_summary.m Sending private/rejectvisual_trial.m Transmitting file data .... Committed revision 9879. mac011> svn commit ft_rejectartifact.m Sending ft_rejectartifact.m Transmitting file data . Committed revision 9880. mac011> svn commit ft_rejectvisual.m Sending ft_rejectvisual.m Transmitting file data . Committed revision 9881.


Robert Oostenveld - 2015-02-11 10:40:25 +0100

Closed several bugs that were recently resolved. Please reopen if you are not happy with the resolution.