Back to the main page.
Bug 3143 - ft_prepare_neighbours error with unexpected channel unit
| Status | CLOSED FIXED |
| Reported | 2016-06-13 14:29:00 +0200 |
| Modified | 2016-11-29 09:01:50 +0100 |
| Product: | FieldTrip |
| Component: | core |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Mac OS |
| Importance: | P5 normal |
| Assigned to: | Jan-Mathijs Schoffelen |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
nno - 2016-06-13 14:29:02 +0200
Recent versions of FieldTrip crash on my machine (OSX 11.10, GNU Octave 4.0.0 and Matlab 2015b) running the following code: cfg=struct() cfg.layout= '4D248.lay'; cfg.method= 'triangulation' ft_prepare_neighbours(cfg) which result in the following output (with error): Using the 2-D layout to determine the sensor position reading layout from file 4D248.lay the call to "ft_prepare_layout" took 0 seconds and required the additional allocation of an estimated 0 MB Error using ft_datatype_sens (line 282) unexpected channel unit "ù" in channel 117unexpected channel unit "nknown" in channel Error in ft_fetch_sens (line 199) sens = ft_datatype_sens(sens); Error in ft_prepare_neighbours (line 154) sens = ft_fetch_sens(cfg); Error detected initially through travis [1] as part of the CoSMoMVPA test suite; the tests passed fine around June 5, 2016, which suggests that the error was introduced after that date. [1] https://travis-ci.org/CoSMoMVPA/CoSMoMVPA/jobs/136188710
nno - 2016-06-13 14:40:22 +0200
Using "git bisect' on these commands, I got: dc689a0676bc4affa84da329a629a204f68d04e8 is the first bad commit commit dc689a0676bc4affa84da329a629a204f68d04e8 Author: Robert Oostenveld <r.oostenveld@gmail.com> Date: Wed Jun 1 17:51:33 2016 +0200 ENH - added the "best" configuration to the test script, and added visualisation of the result
Jan-Mathijs Schoffelen - 2016-06-13 14:50:30 +0200
sorry about this, Nick. I think that the issue you experience is caused by Robert's recent changes to ft_datatype_sens, which now explicitly checks the physical units associated with a particular channel. Long story short, ft_datatype_sens expects this to be well-defined, and not 'unknown', which is the case for the 'COMNT' and 'SCALE' labels in the sens variable that made it into ft_datatype_sens. The reason why these labels are present is due to ft_fetch_sens creating a 'dummy' sens structure from a layout (from a lay file) which annoyingly has these COMNT and SCALE channels represented as separate 'channels'. I think that the best way to fix this would be in ft_fetch_sens, i.e. around line 176 (if haslayout...)
nno - 2016-06-13 15:11:04 +0200
Actually the mistake seems to be in another commit - my previous message wss based on Octave, which also resulted in an error, but a different one. With Matlab I ran the following git bisect start git bisect bad b7828199978717810037e15c902e0b2315e3b3ea git bisect good 943d2bd6ba88b401987c76d312c0ce7b74db9afe git bisect run /Applications/MATLAB_R2015a.app/bin/matlab -r "try,cfg=struct();cfg.layout='4D248.lay';cfg.method='triangulation';ft_prepare_neighbours(cfg),catch,exit(1);end;exit(0)"; with the following output: 1881242752009456130ef9b2c89b9667b24b9b2e is the first bad commit commit 1881242752009456130ef9b2c89b9667b24b9b2e Author: Robert Oostenveld <r.oostenveld@gmail.com> Date: Fri Jun 10 09:08:42 2016 +0200 Use labelOLD instead of labelORG, idem for chanpos, chanori, chantype and chanunit. This is more consistent with old/new in ft_apply_montage. The previous montages (containing "org") should still work. :040000 040000 1ea356b04558c8a8834d59d6f2c0e0d8f7bcaf99 e42ad6fa74d29c406fc8d616318b632584b3ccef M forward :040000 040000 3e16686b261545c18555a6e7a5330192150799ef 894591a3e1f16aaa64b6509cb1703fb2eebe1b3b M test :040000 040000 911a835291f2a928bb0ccacc4f6f7ac1bf08e950 79d8bb9bf26f5bb9eb1637d95a0f9c4a86ca3418 M utilities bisect run success
Jan-Mathijs Schoffelen - 2016-06-13 15:13:44 +0200
Yeah, that looks more like it. It was a commit that gave me trouble last week as well. I addressed this in pull request 173 for my purpose, but based on my diagnosis of what's going on in your case I think that your problem needs to be solved elsewhere.
Jan-Mathijs Schoffelen - 2016-06-13 15:18:03 +0200
I will do this in a minute.
Jan-Mathijs Schoffelen - 2016-06-13 15:21:56 +0200
the just merged pull request 178 should fix the issue. Could you verify and close this bug if it works again?
nno - 2016-06-13 15:44:27 +0200
Dear Jan-Matthijs, thanks for looking into this and updating the code. I just updated my code to commit 9ad7ed369a9e02f4c2a3c4e085aa167a1183812a (which includes your pull request #178) and the original code now runs fine. However, I do get an error now with a neuromag306 layout. In particular, the following code: cfg=struct(); cfg.layout='neuromag306planar.lay'; cfg.method='triangulation'; ft_prepare_neighbours(cfg); now gives the following error Using the 2-D layout to determine the sensor position reading layout from file neuromag306planar.lay the call to "ft_prepare_layout" took 0 seconds and required the additional allocation of an estimated 0 MB Reference to non-existent field 'tra'. Error in ft_datatype_sens (line 290) coil = find(abs(sens.tra(i,:))~=0); Error in ft_fetch_sens (line 203) sens = ft_datatype_sens(sens); Error in ft_prepare_neighbours (line 154) sens = ft_fetch_sens(cfg); Using git-bisect again as follows: git bisect start git bisect bad 9ad7ed369a9e02f4c2a3c4e085aa167a1183812a git bisect good 943d2bd6ba88b401987c76d312c0ce7b74db9afe git bisect run /Applications/MATLAB_R2015a.app/bin/matlab -r "try,cfg=struct();cfg.layout='neuromag306planar.lay';cfg.method='triangulation';ft_prepare_neighbours(cfg),catch,exit(1);end;exit(0)"; points to the same commit as in the earlier report: 1881242752009456130ef9b2c89b9667b24b9b2e is the first bad commit commit 1881242752009456130ef9b2c89b9667b24b9b2e Author: Robert Oostenveld <r.oostenveld@gmail.com> Date: Fri Jun 10 09:08:42 2016 +0200 Use labelOLD instead of labelORG, idem for chanpos, chanori, chantype and chanunit. This is more consistent with old/new in ft_apply_montage. The previous montages (containing "org") should still work. :040000 040000 1ea356b04558c8a8834d59d6f2c0e0d8f7bcaf99 e42ad6fa74d29c406fc8d616318b632584b3ccef M forward :040000 040000 3e16686b261545c18555a6e7a5330192150799ef 894591a3e1f16aaa64b6509cb1703fb2eebe1b3b M test :040000 040000 911a835291f2a928bb0ccacc4f6f7ac1bf08e950 79d8bb9bf26f5bb9eb1637d95a0f9c4a86ca3418 M utilities
nno - 2016-06-13 16:00:00 +0200
To help in debugging: I have made a separate branch [1] of CoSMoMVPA that enabled full regression testing for all 26 layouts known by (supported in) CoSMoMVPA. Since this functionality uses FieldTrip layouts, it may help in assessing to what extent changes in FieldTrip affect its behaviour with respect to dealing with layouts. Indeed, currently Travis reports a failing test [4] To run this test, run tests/test_meeg_chan_neighbors.m using either MOxUnit [3] or Matlab's xUnit framework (recently removed from the Matlab file exchange) [1] https://github.com/nno/CoSMoMVPA/tree/_tst/fieldtrip_full_chan_test [2] https://travis-ci.org/nno/CoSMoMVPA/builds/137248715 [3] http://github.com/MOxUnit/MOxUnit
Robert Oostenveld - 2016-06-14 16:49:21 +0200
(In reply to nno from comment #8) Replicating the small snippet >> cfg=struct(); cfg.layout='neuromag306planar.lay'; cfg.method='triangulation'; ft_prepare_neighbours(cfg); I get --- Reference to non-existent field 'tra'. Error in ft_datatype_sens (line 290) coil = find(abs(sens.tra(i,:))~=0); Error in ft_fetch_sens (line 203) sens = ft_datatype_sens(sens); Error in ft_prepare_neighbours (line 154) sens = ft_fetch_sens(cfg); 290 coil = find(abs(sens.tra(i,:))~=0); K>> sens sens = struct with fields: balance: [1×1 struct] chanpos: [204×3 double] chantype: {204×1 cell} chanunit: {204×1 cell} label: {204×1 cell} type: 'neuromag306' unit: 'dm' --- This is not a valid sensor definition (according to the help of FT_DATATYPE_SENS), as it only contains chanpos, but no coilpos/elecpos/optopos. It is clear that coilpos/elecpos/optopos cannot be retrieved from a 2-D layout file. The issue is that at the end of ft_fetch_sens it does this % ensure that the sensor description is up-to-date sens = ft_datatype_sens(sens); which then fails. We can circumvent that step by this % ensure that the sensor description is up-to-date if (hasgradfile + hascfggrad + hasdatagrad + ... haselecfile + hascfgelec + hasdataelec + ... hasoptofile + hascfgopto + hasdataopto) sens = ft_datatype_sens(sens); end which allows the ft_prepare_neighbours call to run through. I will push the following to the master branch. mac011> git commit -a [master 1f8c36a] FIX - don't do consistency check on sensor definition that was created from a layout, see http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3143#c8 2 files changed, 30 insertions(+), 21 deletions(-)
Jan-Mathijs Schoffelen - 2016-06-14 16:59:50 +0200
owja, dat kan natuurlijk ook :o)