Back to the main page.

Bug 2673 - ft_rejectvisual plots all channels

Status CLOSED FIXED
Reported 2014-08-12 13:10:00 +0200
Modified 2018-11-22 16:51:26 +0100
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P5 normal
Assigned to: Jan-Mathijs Schoffelen
URL:
Tags:
Depends on:
Blocks:
See also: http://bugzilla.fcdonders.nl/show_bug.cgi?id=2625http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3459

- 2014-08-12 13:10:25 +0200

Hi guys, ft_rejectvisual (in latest FT-version used at DCCN: '/home/common/matlab/fieldtrip'), plots all channels instead of only the MEG channels. This is happening despite the following cfg I'm using: cfg = []; cfg.layout = 'CTF275.lay'; cfg.channel = 'MEG'; data_rejvis = ft_rejectvisual(cfg,data_denoise); Best, Erik te Woerd


Diego Lozano Soldevilla - 2014-08-12 13:40:26 +0200

(In reply to e.tewoerd from comment #0) Fixed Transmitting file data . Committed revision 9772.


Nir Ofir - 2018-09-13 10:38:08 +0200

Created attachment 871 Example data structure


Nir Ofir - 2018-09-13 10:41:50 +0200

Hi, I tried using cfg to select trials and/or channels with ft_rejectvisual, but the function disregards it. The trial selection seems to have no effect, while selecting channels means that subplot are still creadted but have no traces in them. I'm using the latest FT from the FTP (2018/09/12). I attached example data, and here are the lines I used to call ft_rejectvisual: cfg = []; cfg.trials = 1; cfg.channel = 1; cfg.method = 'trial'; cfg.preproc.detrend = 'yes'; ft_rejectvisual(cfg, dat) Thanks, Nir Ofir


Jan-Mathijs Schoffelen - 2018-11-05 13:32:42 +0100

If you want this to work, how would you suggest it to be fixed? I think that what is missing from the function is a call to ft_selectdata (which is typically used early in a fieldtrip function to select channels and trials). Just look in ft_timelockanalysis or ft_freqnalysis to get inspired. Please file a pull request with a suggested fix to ft_rejectvisual on the FT repository on github. Also, note that the people who are working on improvements of the code, don't do this because their boss wants them to do it, it's on a voluntary basis, so that means that filing something as a bug does not automatically mean that it will be fixed for you.


Jan-Mathijs Schoffelen - 2018-11-05 13:34:15 +0100

*** Bug 3459 has been marked as a duplicate of this bug. ***


Diego Lozano Soldevilla - 2018-11-06 09:27:10 +0100

(In reply to Jan-Mathijs Schoffelen from comment #4) I added selectdata early in ft_rejectvisual https://github.com/fieldtrip/fieldtrip/pull/868 Let me know what do you think


Jan-Mathijs Schoffelen - 2018-11-06 09:32:29 +0100

Good stuff Diego! Thanks for doing Nir's work :).


Diego Lozano Soldevilla - 2018-11-06 09:51:24 +0100

(In reply to Jan-Mathijs Schoffelen from comment #7) my pleasure!


Nir Ofir - 2018-11-12 16:26:46 +0100

(In reply to Diego Lozano Soldevilla from comment #8) Thanks for doing my work :) Next time I would do it myself. Anyway, the previous correction doesn't work as intended: the functions called by ft_rejectvisual (rejectvisual_trial, rejectvisual_channel and rejectvisual_summary) do their own sort of data selection (which I'll elaborate in a second). The outcome is that if I were to use cfg.trials starting not at 1 (say 10:20), the function would crash after the interactive part, because it will look for the non-existing trials 10:20. This happens because the specific functions (trial, channel and summary) are built such that instead of only showing the chosen trials, they mark them as BAD, and then allow the user to revert that decision, like this: (this is the beginning of rejectvisual_trial, rejectvisual_channel and rejectvisual_summary) ntrl = length(data.trial); if isequal(cfg.trials, 'all') % support specification like 'all' cfg.trials = 1:ntrl; end trlsel = false(1,ntrl); trlsel(cfg.trials) = true; If I use cfg.trials=10:20, the last line of the snippet is going to ADD new elements to trlsel instead of just changing their values. This trlsel ends up as used in line 406 in ft_rejectvisual: data = ft_selectdata(tmpcfg, data); This call of ft_selectdata attempts selecting trials that were removed from data at lines 207-211 in the correction of ft_rejectvisual. Because of the way ft_rejectvisual was written, I suggest the following: 1. Revert the correction for now. 2. Give me a week or two to push a new correction that will not cause this issue and will not be so "patchy" (having data selection twice in the function). I would like to hear your thoughts now, and sorry for wasting your time. Best, Nir


Diego Lozano Soldevilla - 2018-11-12 16:55:59 +0100

Hi Nir, Thank your for your work and sorry for introducing my NO-FIX. Your proposal sounds good to me. I failed to see that rejectvisual_trial, rejectvisual_channel and rejectvisual_summary do their own sort of data selection: my bad


Jan-Mathijs Schoffelen - 2018-11-22 16:51:26 +0100

I have updated rejectvisual_summary/trial/channel to not do a second round of selection as per the cfg specification. I will push this in a minute to the repository on github.