Back to the main page.

Bug 1120 - resolve inconsistency in reading balancing coefficients

Status CLOSED FIXED
Reported 2011-11-05 12:40:00 +0100
Modified 2013-01-16 14:19:47 +0100
Product: FieldTrip
Component: fileio
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P3 minor
Assigned to: Jan-Mathijs Schoffelen
URL:
Tags:
Depends on:
Blocks:
See also:

Robert Oostenveld - 2011-11-05 12:40:07 +0100

I noticed in ft_read_header the following if any(~cellfun(@isempty,strfind(coeftype, 'G1AR'))) try [alphaMEG,MEGlist,Refindex] = getCTFBalanceCoefs(orig, 'G3AR', 'T'); orig.BalanceCoefs.G3AR.alphaMEG = alphaMEG; orig.BalanceCoefs.G3AR.MEGlist = MEGlist; orig.BalanceCoefs.G3AR.Refindex = Refindex; catch % May not want a warning here if these are not commonly used. % Already get a (fprintf) warning from getCTFBalanceCoefs.m % warning('cannot read balancing coefficients for G3AR'); end end which is part of alonger section of coeftypes. It would make more sense to try and read all possible balancing coefficients ("for each coeftype try getCTFBalanceCoefs"). Furthermore, a possible bug is that the code mixes up the G1AR and G3AR types. This should be tested with some datasets at hand


Boris Reuderink - 2011-11-17 10:46:50 +0100

Changed the status of bugs without a specific owner to UNCONFIRMED. I'll try to replicate these bugs (potentially involving the submitter), and change confirmed bugs to NEW. Boris


Boris Reuderink - 2011-11-17 13:58:13 +0100

Roberts fragment is in fileio/ft_read_header.m:386.


Boris Reuderink - 2012-01-17 12:23:21 +0100

Robert, could you provide some additional information? I have no clue where to begin.


Robert Oostenveld - 2012-01-18 22:11:48 +0100

the sequence of try-catch statements for the different types should be avoided. Rather, it should just do what needs to be done. More something along the lines of for i=1:length(coeftype) thistype = coeftype{i}; read it with getCTFBalanceCoefs and store it end The potential mix up is in the inconsistent use of G1AR in the if_any_cellfun(...) and G1AR in the code immediately below. The potential bug is not serious, but could result in either G1AR or G1AR not being present in the structure (i.e. "G1AR is not detected, so G3AR is not read"). PS try-catch in general should be avoided as coding style to deal with known exceptions. It is lazy programming, can cause other problems to remain invisible, and moreover it is slow (setting up the exception handler takes time).


Boris Reuderink - 2012-03-28 10:46:42 +0200

Hmm. I am not sure why this is assigned to me; probably someone else can fix this more efficiently --- assigning this one to the joint dev.


Jan-Mathijs Schoffelen - 2012-09-19 16:01:55 +0200

Actually from getCTFBalanceCoefs it appears that G1AR does not exist as a balancing scheme; it's either G1BR, G2BR, G3BR, or G3AR. The BR's already have a try catch around it, updating the code and change G1AR into G3AR


Jan-Mathijs Schoffelen - 2012-09-19 16:05:13 +0200

bash-3.2$ svn diff ft_read_header.m Index: ft_read_header.m =================================================================== --- ft_read_header.m (revision 6475) +++ ft_read_header.m (working copy) @@ -446,7 +446,7 @@ warning('cannot read balancing coefficients for G3BR'); end end - if any(~cellfun(@isempty,strfind(coeftype, 'G1AR'))) + if any(~cellfun(@isempty,strfind(coeftype, 'G3AR'))) try [alphaMEG,MEGlist,Refindex] = getCTFBalanceCoefs(orig, 'G3AR', 'T'); orig.BalanceCoefs.G3AR.alphaMEG = alphaMEG; bash-3.2$ svn commit ft_read_header.m Sending ft_read_header.m Transmitting file data . Committed revision 6477.