Back to the main page.
Bug 3064 - rollback_provenance in ft_multiplotER breaks trial selection
| Status | CLOSED FIXED |
| Reported | 2016-02-05 11:57:00 +0100 |
| Modified | 2019-08-10 12:42:05 +0200 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P5 critical |
| Assigned to: | |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | 2978 |
| See also: |
Eelke Spaak - 2016-02-05 11:57:45 +0100
ft_multiplotER lines 320-324: tmpcfg = keepfields(cfg, 'channel'); tmpvar = varargin{1}; [varargin{:}] = ft_selectdata(tmpcfg, varargin{:}); % restore the provenance information [cfg, varargin{:}] = rollback_provenance(cfg, varargin{:}); If cfg contains cfg.trials = xxx before the call to rollback_provenance, this will no longer be there (set to 'all') after this call. Hence, trial selection further down will not happen. A consequence of this is that trial plotting from rejectvisual_summary is broken; it always plots the trial-average (while still displaying 'Trial X' in the title! dangerous bug!). I guess a quick and dirty patch (needed for the KCL workshop in which rejectvisual_summary features (preprocessing_erp tutorial)) would be to remember a possible cfg.trials setting and assign it again to cfg.trials after the rollback_provenance call. But how to fix it more properly? Should ft_selectdata here be in charge of doing a subselection of trials? In that case, the trial selection code in lines 352-363 can be removed. But since that is done by ft_timelockanalysis, I'm not sure how that affects other aspects of ft_multiplotER.
Eelke Spaak - 2016-02-05 12:10:35 +0100
(In reply to Eelke Spaak from comment #0) And, more generally, why is rollback_provenance touching cfg.trials at all..?
Robert Oostenveld - 2016-02-05 12:15:06 +0100
initially it indeed sounds the best to have ft_selectdata do both channels and trials, rather than having local code or rather than doing it in ft_timelockanalysis. The reason for it touching cfg.trials is that it is used both in the called function (select data) as in the caller function (multiplot). The interpretation here is the same, but imagine function 1 with cfg.method calling function 2 that also has (another) cfg.method... There should be examples in the code of similar situations like this.
Julian Keil - 2017-08-15 11:00:24 +0200
Created attachment 845 Hacked ft_multiplotER-function I (crudely) hacked the function to accept the trialselection.
Julian Keil - 2017-08-15 11:02:07 +0200
Dear Eelke, Dear Robert, since the problem is still unresolved, I hacked the ft-multiplotER-function (see attachment) to accept the cfg.trial-selection. Specifically, I included "trials" field in the keepselection (line
Julian Keil - 2017-08-15 11:06:42 +0200
(sorry, hit the wrong button while typing). Specifically, I included "trials" field in the keepselection (line 317 of the hacked function), I kept the old trial selection (line 350), temporarily switched the trial selection off (line 351) and transferred the old trial selection to the updated "varargin" (line 368). I'm sure there are more elegant ways, but this works for me right now. Cheers, Julian
Robert Oostenveld - 2017-08-15 12:29:06 +0200
(In reply to Julian Keil from comment #5) Hi Julian, Could you send it as pull request on github? A detailed tutorial is here http://www.fieldtriptoolbox.org/development/git
Robert Oostenveld - 2017-08-16 16:43:07 +0200
Julien made the PR at https://github.com/fieldtrip/fieldtrip/pull/492 I made an additional change and test script and merged it all under https://github.com/fieldtrip/fieldtrip/pull/493
Robert Oostenveld - 2019-08-10 12:35:50 +0200
This closes a whole series of bugs that have been resolved (either FIXED/WONTFIX/INVALID) for quite some time. If you disagree, please file a new issue on https://github.com/fieldtrip/fieldtrip/issues.