Back to the main page.
Bug 2354 - averaging of avg/rpt fails for ERFs in ft_topoplot/topoplot_common
| Status | CLOSED FIXED |
| Reported | 2013-10-31 15:48:00 +0100 |
| Modified | 2014-05-16 21:13:31 +0200 |
| Product: | FieldTrip |
| Component: | plotting |
| Version: | unspecified |
| Hardware: | All |
| Operating System: | All |
| Importance: | P3 minor |
| Assigned to: | Jan-Mathijs Schoffelen |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Philipp Ruhnau - 2013-10-31 15:48:07 +0100
Dear fieldtrip developers, i encountered a bug when plotting topographies using ft_topoplotER. i think it might also apply to other functions. I just updated fieldtrip to yesterday's version, so i'm confident it hasn't been fixed yet. what happens is the following (see also example script/data) 1) i have a data-structure containing ERFs of individual subjects, thus a struct with the field 'data.individual'. (data are from a neuromag system) 2) i want to plot some topographies of a specific time window and i'm indicating cfg.parameter = 'individual'; 3) the function reports computing an average over trials but then crashes printing an error: 'Reference to non-existent field 'individual'.' Error in topoplot_common (line 522) dat = data.(cfg.parameter); Error in ft_topoplotER (line 169) cfg = topoplot_common(cfg, varargin{:}); The problem is that topoplot_common averages across the rpt/subj dimension (line 279 ff), for that purpose ft_timelockanalysis is applied which (sensibly) changes the 'individual' to an 'avg' field, yet cfg.parameter (to extract the data later, line 522) remains unchanged. I would suggest either to change the parameter at that point, or to follow the tf-logic (line 303) and add data.(cfg.parameter) = data.avg; to line 286. this does solve the problem. I used to use a different workaround and average across subjects by hand before plotting (see also the example script), but this is rather annoying and an easy fix in my view. Best Philipp
Philipp Ruhnau - 2013-10-31 15:48:54 +0100
Created attachment 548 test script
Jan-Mathijs Schoffelen - 2013-10-31 15:58:46 +0100
Dear Philipp, Thanks for posting. I see that the attachment to this bug already contains some data. That's great! We'll look into it a.s.a.p. JM
Jan-Mathijs Schoffelen - 2013-11-01 12:38:44 +0100
Thanks Philipp, I could reproduce the problem with your data and have fixed it in today's version. This also affected ft_singleplotER and ft_multiplotER. The change is in revision 8685
Philipp Ruhnau - 2013-11-01 13:46:32 +0100
cool, thanks p (In reply to comment #3)
Robert Oostenveld - 2013-11-02 15:33:41 +0100
Hi Jan-Mathijs, Your change in http://code.google.com/p/fieldtrip/source/detail?r=8685 causes a number of regression test scripts to fail. For reference; test_bug1245 AND test_tutorial_clusterpermutationtimelock are two of them. The change entails that after data = ft_timelockanalysis(tmpcfg, data) you do data.(cfg.parameter) = data.avg data = rmfield(data, 'avg'); but in the test scripts I found that cfg.parameter='avg', i.e. you rename avg to avg and subsequently remove it! I fixed one case in http://code.google.com/p/fieldtrip/source/detail?r=8704, extending the test script a bit and am about to commit two more changes, as I just figured out the pattern of problems introduced in commit 8685. The reason for me reopening this bug is that it ain't fully clear to me how your change is supposed to work. I guess it relies on ft_timelockanalysis averaging whatever parameter is useful, and returning this in avg. In Philipp's case the parameter of interest is "individual", but it might also be "trial" (as a matrix). It might be that, rather than using ft_timelockanalysis with its limited parameter handling, ft_selectdata is the better choice with cfg.avgoverxxx=yes. cheers Robert
Robert Oostenveld - 2013-11-02 15:37:20 +0100
(In reply to comment #5) roboos@mentat001> svn commit Sending ft_singleplotER.m Sending private/topoplot_common.m Transmitting file data .. Committed revision 8708.
Jan-Mathijs Schoffelen - 2013-11-03 12:00:25 +0100
(In reply to comment #5) Too rash, indeed. I am aware of the fact that nowadays, it would be better to use ft_selectdata. For a quick fix for the time being I opted for Philipp's suggestion. Did not think it through long enough, apparently. Note, that for the plotting functions ft_selectdata should probably also replace the code that is actually selecting the data to be plotted. Also, once we open that can of worms, we should probably also consider to use ft_selectdata for prepare_freq_matrices, and prepare_timefreq_data.
Philipp Ruhnau - 2013-11-04 09:46:52 +0100
sorry to add something again, although i have the impression the potential changes go further than this bug. i think my initial idea should work if one does not remove the 'avg' field (i think JM did this for memory reasons? but this is just the average so it shouldn't take that much.) alternatively one could create a tmpdata struct and take the necessary fields from there (which is actually done for tf data). and second, i'm not sure if ft_selectdata works. in my case i'm rather sure it does not. remember here it is about the individual field. there is no avgoversubj option (and the avgoverrpt option does not work (at least not in my first few attempts). i'm always getting errors (i think the function assumes a cell array, as for trials). best p (In reply to comment #7)
Jan-Mathijs Schoffelen - 2013-11-04 09:55:37 +0100
Dear Philipp, Thanks for feedback. It's on our long-standing to do list to make ft_selectdata better suited to generate output that meets the user's expectation. At some point in the future this may be the case, so that we can start using this to be used as a more intermediate-level function, rather than using the high-level ft_timelockanalysis function. For now, I think that Robert added some lines in the code, that only throws away the avg field, when the cfg.parameter is not avg (which makes sense). So, the bottom line is that I think the code still works for you, correct?
Philipp Ruhnau - 2013-11-04 10:07:17 +0100
ah okay, now i see the 'if', didn't know where to look... i wasn't sure if you were still adding some new things to solve this (also cause it's still open). thanks again p (In reply to comment #9)
Jan-Mathijs Schoffelen - 2013-11-24 12:14:23 +0100
The issue at hand seems to be solved for now. When working on ft_selectdata related bugs, we will revisit the issue that this function should replace the calls to ft_timelockanalysis, ft_freqdescriptives etc, if possible