Back to the main page.
Bug 1704 - Crash when preparing layout for Neuromag
| Status | ASSIGNED |
| Reported | 2012-09-07 16:49:00 +0200 |
| Modified | 2012-10-24 09:48:29 +0200 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P3 critical |
| Assigned to: | Eelke Spaak |
| URL: | |
| Tags: | |
| Depends on: | 1757 |
| Blocks: | |
| See also: |
Vladimir Litvak - 2012-09-07 16:49:52 +0200
Created attachment 306 bug cfg Hi, I get: ??? Index exceeds matrix dimensions. Error in ==> ft_prepare_layout>sens2lay at 788 mindist = mindist(1:round(numel(label)/4)); Error in ==> ft_prepare_layout at 285 lay = sens2lay(cfg.grad, cfg.rotate, cfg.projection, cfg.style); Error in ==> spm_eeg_project3D at 25 lay = ft_prepare_layout(cfg); The problematic cfg is attached. This happens for all Neuromag data. Please fix as this prevents Neuromag data from being converted properly in SPM. Thanks, Vladimir
Eelke Spaak - 2012-09-21 13:24:19 +0200
Hi Vladimir, Unfortunately, it is impossible to automatically generate a layout based on the channel positions in your cfg.grad structure, since Neuromag306 data has several channels that occupy identical positions. The only way to properly plot these is to hand-craft a layout that either displaces channels (i.e. show the magnetometer and gradiometers at the same coordinates in a triplet) or shows a subset of channels. You can specify pre-generated layouts by using one of: cfg.layout = 'neuromag306all.lay' cfg.layout = 'neuromag306mag.lay' cfg.layout = 'neuromag306cmb.lay' cfg.layout = 'neuromag306planar.lay' In revision 6510, I have added an explicit check for the presence of many overlapping channels in ft_prepare_layout, throwing a more informative error message. Marking this as fixed for now, feel free to post additional comments though.
Vladimir Litvak - 2012-09-21 17:07:29 +0200
Hi Eelke, We need that to work for SPM and it's not a problem if you output the same positions. We have some code that takes care of that by slightly moving them apart. If you want I can send you the code to put in FT, but pre-specified layouts is definitely a bad solution for us. Maybe you can just make output identical position and display a warning (that was the previous behavior anyway). Vladimir
Vladimir Litvak - 2012-09-21 17:08:53 +0200
(In reply to comment #2) In any case we need to make it work quickly as right now it breaks SPM conversion for Neuromag data.
Eelke Spaak - 2012-09-23 08:09:45 +0200
Assigning this to Robert, since I think it needs an executive decision on how to deal with this.
Robert Oostenveld - 2012-09-23 10:06:31 +0200
(In reply to comment #2) Hi Vladimir, I agree that template layouts are not good as default. Can you give us the code for moving the channels that lie on top of each other slightly apart? Is it a random movement or will the triplets (in case of neuromag306) or pairs (in the case of neuromag122) move consistently for different locations over the head? Is it neuromag specific? The yokogawa404 system is also a difficult one to deal with with its mixed sensors. Robert PS in the mean time you might want to do in the high level code try ft_prepare_layout catch read a template end
Vladimir Litvak - 2012-09-23 12:13:37 +0200
(In reply to comment #5) Hi Robert, Here is the code. It is applied to 2D locations. Give credit to Laurence Hunt if you use it. It doesn't have to be random. It just needs to be consistent, like whatever is first in the list always moves one way. Vladimir function xy = shiftxy(xy,mindist) x = xy(1,:); y = xy(2,:); l=1; i=1; %filler mindist = mindist/0.999; % limits the number of loops while (~isempty(i) && l<50) xdiff = repmat(x,length(x),1) - repmat(x',1,length(x)); ydiff = repmat(y,length(y),1) - repmat(y',1,length(y)); xydist= sqrt(xdiff.^2 + ydiff.^2); %euclidean distance between all sensor pairs [i,j] = find(xydist<mindist*0.999); rm=(i<=j); i(rm)=[]; j(rm)=[]; %only look at i>j for m = 1:length(i); if (xydist(i(m),j(m)) == 0) diffvec = [mindist./sqrt(2) mindist./sqrt(2)]; else xydiff = [xdiff(i(m),j(m)) ydiff(i(m),j(m))]; diffvec = xydiff.*mindist./xydist(i(m),j(m)) - xydiff; end x(i(m)) = x(i(m)) - diffvec(1)/2; y(i(m)) = y(i(m)) - diffvec(2)/2; x(j(m)) = x(j(m)) + diffvec(1)/2; y(j(m)) = y(j(m)) + diffvec(2)/2; end l = l+1; end xy = [x; y];
Robert Oostenveld - 2012-09-23 13:07:04 +0200
re-assigning to Eelke.
Eelke Spaak - 2012-09-24 14:24:27 +0200
I will incorporate the code, seems like a good fix. Vladimir, could you tell me what value you (typically) use for parameter 'mindist'?
Eelke Spaak - 2012-09-24 14:36:19 +0200
Committed fix as rev 6519. The default now is to shift to a mindist of 0.2 if many channels overlap, which I found to give visually nice results in a multiplot. There is a warning thrown when the shifting is needed.
Vladimir Litvak - 2012-09-24 15:10:38 +0200
(In reply to comment #9) OK that's better. Could you just also make this optional (on by default) so that if we turn that off we'll still get identical locations? Also when shifting is explicitly turned off I'd prefer if there is no warning as warnings make some of our users nervous. The reason is that I realized that we use layouts in two ways. One is multiplots and then shifting is OK but the other is topoplots (or generating scalp x time images) and then we need the original locations and it's not problem that they overlap. That's actually true for FT as well isn't it? Vladimir
Eelke Spaak - 2012-09-25 09:43:59 +0200
No problem, I just (r6529) added the configuration option cfg.overlap to ft_prepare_layout. From the new reference doc: % cfg.overlap string, how to deal with overlapping channels when % layout is constructed from a sensor configuration % structure (can be 'shift' (shift the positions in 2D % space to remove the overlap (default)), 'keep' (don't shift, % retain the overlap), 'no' (throw error when overlap is % present)) Best, Eelke
Vladimir Litvak - 2012-09-25 11:42:40 +0200
(In reply to comment #11) OK the option seems to work as needed, but something is still not right. It looks like there is a mismatch between labels and positions. Try this code on a neuromag header: sel = strmatch('megmag', hdr.chantype); cfg = []; cfg.grad = hdr.grad; cfg.overlap = 'keep'; lay = ft_prepare_layout(cfg); [sel1, sel2] = match_str(hdr.label(sel), lay.label); figure;plot(lay.pos(sel2, 1), lay.pos(sel2, 2), '.') Vladimir
Vladimir Litvak - 2012-09-27 12:45:01 +0200
Hi Eelke, This problem looks like a bug in ft_read_header rather than ft_prepare layout as grad.chanpos is already wrong. I'll try to look where it comes from and let you know what I find. Vladimir
Vladimir Litvak - 2012-09-27 13:06:40 +0200
(In reply to comment #13) OK, fixed.
Eelke Spaak - 2012-09-27 13:46:32 +0200
Great, thanks!
Vladimir Litvak - 2012-09-27 14:58:05 +0200
Created attachment 330 weird bti layout weird bti layout
Vladimir Litvak - 2012-09-27 15:00:58 +0200
(In reply to comment #16) More problems I'm afraid. There was a problem with BTi and CTF conversion because of my fix (or more precisely because the FT code played withe the input sens struct and didn't change it back and I wasn't aware of that). I fixed it now. However the layout for BTi looks weird now. I attached an example from SPM. I don't have time to investigate this further now but perhaps you can look. Vladimir
Robert Oostenveld - 2012-10-02 11:15:03 +0200
I get this problem >> grad = ft_read_sens('/home/common/matlab/fieldtrip/data/Subject01.ds'); ??? Error using ==> cell.unique at 47 Input must be a cell array of strings. Error in ==> match_str at 63 [dum1, dum2, c] = unique([a; b]); Error in ==> channelposition at 284 [sel1, sel2] = match_str(sens.label, lab); Error in ==> ft_datatype_sens at 99 [chanpos, chanori, chanlab] = channelposition(sens, 'channel', 'all'); Error in ==> ft_read_header at 1707 hdr.grad = ft_datatype_sens(hdr.grad); Error in ==> ft_read_sens at 157 hdr = ft_read_header(filename, 'headerformat', fileformat); and the last change to channelposition seems related to this bug: % $Id: channelposition.m 6564 2012-09-27 12:52:32Z vlalit $ The problem is on line 284 in channelposition where [sel1, sel2] = match_str(sens.label, lab); and lab contains some empty elements (i.e. [])
Robert Oostenveld - 2012-10-02 11:17:03 +0200
(In reply to comment #18) traced it further back to line 114, where lab(sel) = sens.label; but sel is not complete, i.e. some of the elements are missing (and hence remain empty.
Johanna - 2012-10-02 11:20:55 +0200
see also bug 1757
Robert Oostenveld - 2012-10-02 11:23:52 +0200
(In reply to comment #19) I did the following to go back to the version just prior to the last channelposition change manzana> svn update -r 6563 and the problem persists. So it was introduced earlier. With manzana> svn update -r 6558 I don't have the problem, whereas with manzana> svn update -r 6559 I do have the problem. That means that http://code.google.com/p/fieldtrip/source/detail?r=6559 introduced it. See http://code.google.com/p/fieldtrip/source/diff?spec=svn6559&r=6559&format=side&path=/trunk/fileio/private/channelposition.m for the diff. It is precisely the lines of code on which it now fails.
Robert Oostenveld - 2012-10-02 11:32:30 +0200
I suspect that we will have to revisit this problem in the future, as the implemented solution does not seem to be very robust. But for the moment (and for me) it now works, so I will close it again.
Vladimir Litvak - 2012-10-02 11:41:23 +0200
(In reply to comment #22) Hi Robert, I think what I did is quite benign and sensible, it just might have revealed other bugs that were previously hidden. So definitely don't revert my fix. The way things were before, nothing assured that the order of grad.chanpos was the same as the order of grad.label and it indeed wasn't for Neuromag. Vladimir
Robert Oostenveld - 2012-10-02 17:45:55 +0200
(In reply to comment #23) Yes, I believe you are right. The lack of robustness that I feel to apply here more pertains to the whole chain of events and code that results in the layout, than the specific last section of code.