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