Back to the main page.
Bug 1146 - ft_selectdata with no selected trials
| Status | CLOSED FIXED |
| Reported | 2011-11-12 14:54:00 +0100 |
| Modified | 2014-01-15 14:42:18 +0100 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Linux |
| Importance: | P3 major |
| Assigned to: | Jan-Mathijs Schoffelen |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | 1021 |
| See also: |
Gio Piantoni - 2011-11-12 14:54:41 +0100
Hi, I'm analyzing some data with reaction times in there (stored in the data.trialinfo field). I want to split the data, into two datasets, one with reaction times shorter than 500 ms and one longer. However, if there are NO trials with reaction times shorter than 500 ms, ft_selectdata selects ALL the trials. Example: % Create some data nchan = 10; ntrl = 20; data = []; data.label = cellfun(@(x) ['E' num2str(x)], num2cell(1:nchan), 'uni', 0); data.fsample = 100; data.trial = repmat({rand(nchan,100)}, 1, ntrl); data.time = repmat({1/data.fsample:1/data.fsample:1}, 1, ntrl); data.trialinfo(:,1) = 1:ntrl; % reaction times, for simplicity between 1 and 20 %-select some trials seltrl = data.trialinfo > 10; % or seltrl = find(data.trialinfo > 10); datasel = ft_selectdata(data, 'rpt', seltrl) % <- expected behavior %-select no trials seltrl = data.trialinfo < 1; % none selected datasel = ft_selectdata(data, 'rpt', seltrl) % <- unexpected behavior, no warning It selects all the trials without warning, which in my case does not make sense (it returns all the trials, with low and high reaction times). Possible fixes: I'm not sure if this is the expected behavior (WONTFIX) or a bug. There are two possible inputs: seltrl is logical or it is find(LOGICAL). Both show the unexpected behavior. If seltrl is find(LOGICAL), that is [], then ft_selectdata selrpt = ft_getopt(varargin, 'rpt', []); selectrpt = ~isempty(selrpt); does not compute any selection on the trials at all, and selectrpt = FALSE. This might be the expected behavior though. If seltrl is LOGICAL, all FALSE, then the relevant part is: if islogical(selrpt), selrpt = find(selrpt); elseif isempty(selrpt), warning('you request all repetitions to be thrown away'); end In this case, I'd change the elseif into if: if islogical(selrpt), selrpt = find(selrpt); end if isempty(selrpt), warning('you request all repetitions to be thrown away'); end So, at least it throws a warning. Even after the warning, it returns ALL the trials (because of private/selfromraw.m, if ~isempty(selrpt) ). At this point, it gets pretty complicated. My preference would be for an error, or a warning but the output has no trials. Please, let me know what the intended behavior is and if it makes sense to throw an error instead of a warning. Cheers, Gio
Jan-Mathijs Schoffelen - 2011-11-15 20:54:53 +0100
@ Craig, I found where I encountered your issue before... @ Gio: thanks for your input and the suggested fix. I think there should not be a fundamental objection against the " 'rpt', [] " returning no trials. My suggestion would be to change the default assignment (by the call to ft_getopt) into 'all', and to set the selectrpt-flag to true if ischar(selrpt) && ~strcmp(selrpt, 'all'). then we need implement this change also in seloverdim/selfromraw, and ensure that they behave correctly when the 'sel'-vector is empty. I made this suggestion to Craig, along with the request to give it a go implementing it... Could you team up and get it fixed? Thanks, JM
Craig Richter - 2011-11-24 01:01:09 +0100
working on it...
Craig Richter - 2011-12-13 13:33:23 +0100
(In reply to comment #1) JM, I'm not sure I understand the approach. If the default is all, how does the empty matrix, i.e. data = ft_selectdata(data','rpt',[]) cause the selection of none of the trials rather than all of them?
Jan-Mathijs Schoffelen - 2011-12-13 14:03:25 +0100
right now the code reads selrpt = ft_getopt(varargin, 'rpt', []); selectrpt = ~isempty(selrpt); In other words, selectrpt is set to TRUE only if the selrpt variable is not empty. Only when selectrpt is TRUE the subselection of trials is done. So currently there will never be a selection of NO trials if selrpt = empty. I suggested: selrpt = ft_getopt(varargin, 'rpt', 'all'); selectrpt = ~ischar(selrpt) || ~strcmp(selrpt,'all'); This causes selectrpt to be set to TRUE if selrpt is either empty, or is a vector with trial indices, i.e. in this case the trial selection will take place. If the code that is doing the actual trial selection knows how to deal with the empty selrpt (i.e. throwing away all trials) then we are set, correct?
Craig Richter - 2011-12-13 16:14:07 +0100
But if rpt is [], then ft_getopt sets selrpt to 'all', the default, rather than to [], so this is a problem.
Jan-Mathijs Schoffelen - 2011-12-13 16:56:33 +0100
I don't understand this. ft_getopt should return [] if there is a key-value pair stating " 'rpt', [] " If it's your purpose to select no trials at all, you should call ft_selectdata(data, 'rpt', []). Or am I missing something?
Craig Richter - 2011-12-13 17:06:00 +0100
Oddly, it is giving me 'all', when I specify []. It confused me too. When I give an empty input ([]) ft_getopt gives me the default value. I guess we need to change this, but the file is a mex.
Jan-Mathijs Schoffelen - 2011-12-13 17:24:28 +0100
confirmed. I don't know why this is the case, and what should be appropriate behavior. I'll check with the boss.
Jan-Mathijs Schoffelen - 2013-10-29 14:20:13 +0100
Working on ft_selectdata_new, I implemented the following behavior: default: cfg.trials = 'all'; cfg.channel = 'all'; cfg.latency = 'all'; cfg.frequency = 'all'; behavior in this case is to select all, i.e. do not select. If the user specifies any of these to be empty, nothing will be selected, i.e. the result will be an empty matrix. At the moment, this behavior is tested for frequency data and raw data in test_ft_selectdata. I will extend this test also to timelock data (not yet to source data, this pertains to a different bug), and if all works as expected I will change the status of this bug. Agreed?