Back to the main page.
Bug 2418 - ft_senstype does not return neuromag306alt
Status | CLOSED FIXED |
Reported | 2013-12-10 14:25:00 +0100 |
Modified | 2014-02-24 10:56:31 +0100 |
Product: | FieldTrip |
Component: | core |
Version: | unspecified |
Hardware: | PC |
Operating System: | Mac OS |
Importance: | P3 normal |
Assigned to: | Robert Oostenveld |
URL: | |
Tags: | |
Depends on: | |
Blocks: | |
See also: |
Robert Oostenveld - 2013-12-10 14:25:10 +0100
On 9 Dec 2013, at 19:59, Raghavan Gopalakrishnan wrote: There seems to be a bug in ft_senstype. Even if my data.label has names without spaces, it always returns 'neuromag306', instead of 'neuromag306alt'. Because of that line 103 and 104 in ft_combineplanar returns empty matrices. Thanks, Raghavan
Robert Oostenveld - 2013-12-10 15:05:37 +0100
it can be reproduced with lab = ft_senslabel('neuromag306alt') lab = lab(:); ft_senstype(lab) which returns "neuromag306".
Robert Oostenveld - 2013-12-10 15:32:42 +0100
I made a test script that checks ft_senstype versus ft_senslabel. The neuromag306alt not being detected was due to a simple type. It also applied to neuromag122. I also fixed yokogawa9, and made some changes to eeg1005, eeg1010, eeg1020 and ext1020. mac001> svn commit Sending forward/ft_senslabel.m Sending forward/ft_senstype.m Adding test/test_bug2418.m Transmitting file data ... Committed revision 9009.
Gianpaolo Demarchi - 2014-01-03 15:28:47 +0100
Hi, despite running your example returns correctly now 'neuromag306alt', when I apply to my data (see attachment), I get wrong results (I've labels without spaces, i.e. 'alt'). Namely, >> ft_senstype(testdata.label) ans = neuromag306alt *but* >> ft_senstype(testdata) ans = neuromag306 and in ft_combineplanar you pass the data, not the label ... Sorry for reopening a bug, I should have noticed it before replying to the mailing list though ... Best, GP
Gianpaolo Demarchi - 2014-01-03 15:37:33 +0100
Created attachment 578 Some data to show the weird behavior of ft_senstype Shortened version of testdata, still shows the same ft_senstype behavior
Robert Oostenveld - 2014-01-07 10:57:51 +0100
(In reply to Gianpaolo Demarchi from comment #4) thanks for the test data. I can confirm your report with >> ft_senstype(testdata_short) ans = neuromag306 >> ft_senstype(testdata_short.label) ans = neuromag306alt >> ft_senstype(testdata_short.grad) ans = neuromag306 >> ft_senstype(testdata_short.grad.label) ans = neuromag306alt Case 1 and 3 are explained by testdata_short.grad having the type specified as neuromag306 (that was added incorrectly when it was created by you with the previous incorrect FT version). If I do >> testdata_short.grad = rmfield(testdata_short.grad, 'type'); then >> ft_senstype(testdata_short) ans = neuromag306alt >> ft_senstype(testdata_short.label) ans = neuromag306alt >> ft_senstype(testdata_short.grad) ans = neuromag306alt >> ft_senstype(testdata_short.grad.label) ans = neuromag306alt all types are correct. So the error is in the data structure (because FT was wrong when the data structure was made) and not in the present code any more.
Robert Oostenveld - 2014-01-07 11:04:21 +0100
(In reply to Robert Oostenveld from comment #5) I added this section to the test script. mac001> svn commit test_bug2418.m Sending test_bug2418.m Transmitting file data . Committed revision 9073.
Robert Oostenveld - 2014-01-07 17:53:30 +0100
I am presently processing a neuromag dataset and am running into the same inconsistency. So the present FT code apparently still is not internally consistent.
Robert Oostenveld - 2014-01-07 18:07:56 +0100
(In reply to Robert Oostenveld from comment #7) ohh... this neuromag306 versus neuromag306alt is a mess. Why do we have the two types in the code? I can understand that we need it for the template files (layout and neighbours), but the code would be much cleaner without it.
Gianpaolo Demarchi - 2014-01-07 23:03:07 +0100
(In reply to Robert Oostenveld from comment #8) Hi Robert, I didn't have the time to tinker with that stuff today, but as you said it's still weird. Namely, with a fresh fif file&fieldtrip: hdr = ft_read_header('19880122_CISN_131128_DHNETWORK_R01.fif') >> hdr.grad.label ans = 'MEG0113' 'MEG0112' ... so far so good, but >> ft_senstype(hdr) ans = neuromag306 mmmh .. this should be 'alt' >> ft_senstype(hdr.grad) ans = neuromag306 again ... >> ft_senstype(hdr.grad.label) ans = neuromag306alt This is finally right ... The question is from where the hell are we, uselessly, inheriting the two different styles ... Is it from the MNE toolbox? >> Mhdr= fiff_read_meas_info('19880122_CISN_131128_DHNETWORK_R01.fif'); >> Mhdr.ch_names ans = Columns 1 through 6 'MEG0113' 'MEG0112' 'MEG0111' 'MEG0122' 'MEG0123' 'MEG0121' etc ... >> Mhdr.chs(1,1).ch_name ans = MEG0113 Can't we just strip the spaces out, if found in the fiff file info (old filetype?!), and keep only one version, imho the one without spaces?! I know it's an arbitrary choice, and the good place to do it could be already when opening the file with ft_read_header, but with ft we/you take arbitrary choices everyday anyway ... What's the real need of having the two versions, even if present in the fiff pipeline, since also the combined channels are the same, spaces apart ... ?!
Robert Oostenveld - 2014-01-08 09:01:56 +0100
(In reply to Gianpaolo Demarchi from comment #9) fileio/private/mne2grad has at the end % determine the type of acquisition system if nAxGrad>0 grad.type = 'babysquid74'; elseif nPlaGrad>122 && nMag~=0 grad.type = 'neuromag306'; elseif nPlaGrad<=122 && nMag==0 grad.type = 'neuromag122'; else % do not specify type of acquisition system end which I added in revision 7435 (not too long ago). So irrespective of channel labels, there is the field "type=neuromag306". Subsequently, if FT looks at the grad, it sees type=neuromag306. If FT looks at only the labels, it decides it is neuromag306alt. Throughout the code I don't see any important use of neuromag306alt, it is only there to allow the channel list from ft_senslabel to have the space or not. I think that there should only be one type, and that it should work with or without spaces. Note about stripping the spaces: the spaces are there (or not) in the fif file. Some neuromag systems have been configured to have spaces, some do not have spaces. FT should not change the channel names, so stripping the spaces is not a good idea. But FT should simply not care whether there is a space or not.
Robert Oostenveld - 2014-01-08 17:18:18 +0100
I made some changes, it should now consistently be without the alt. Even if type=neuromag306alt is present in the data, it will be identified as neuromag306. mac001> svn commit forward/ test/ Sending forward/ft_senslabel.m Sending forward/ft_senstype.m Sending test/test_bug2418.m Transmitting file data ... Committed revision 9083.
Gianpaolo Demarchi - 2014-01-09 22:38:20 +0100
(In reply to Robert Oostenveld from comment #11) Great! Tested and works flawlessly now! Sorry for the late reply, uni email was kaputt ... Thanks again, G.